Remove Content-Length header from proxy inspector response#42590
Closed
kmagiera wants to merge 1 commit into
Closed
Remove Content-Length header from proxy inspector response#42590kmagiera wants to merge 1 commit into
kmagiera wants to merge 1 commit into
Conversation
This change removes Content-Length header from proxy inspector response. This header was resulting in the response being cropped some times, because of erroneously calculated langth. The Content-Length value represents the number of bytes in the response while string.length was used to calculate the value, which gives the number of characters in a string instead of its size in bytes. Specifically, if there are some UTF characters in the string that occupy more than byte, there would be a mismatch in this size which resulted in the response being cropped. This could happen for example when the simulator name contains such character.
facebook-github-bot
added
CLA Signed
Contributor
|
Thanks @kmagiera, appreciate the fix! |
Contributor
|
@motiz88 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
This was referenced Mar 1, 2024
kmagiera
added a commit
to software-mansion/radon-ide
that referenced
this pull request
Mar 19, 2024
With Expo 50, we no longer can you same metro runner as they've landed several changes that just breaks with the way we'd set up packager. This PR makes it so that we use similar path to starting packager with `expo start`. Here is what's changed: 1. When project has app.json, we use new expo start way of launching the packager 2. We extracted a number of metro related code to separate lib file such that it can be used both by old metro config and by expo setup 3. We blacklisted more jsx-transform modules, as they started conflicting with different setups 4. We're removing – (hyphen) char from default device names, as they started to cause issues with debugger (see facebook/react-native#42590) 5. Expo metro setup now has a new expo_start script that uses some internal method overrides in order to inject code that updates metro config 6. The expo setup is configured as CI such that it doesn't print too much formatted output 7. We also had to override package that tests for a busy port, because it didn't handle port nr 0 well 8. Finally we made small adjustments in runtime code in order to load lazily some internal RN modules 9. Also some small adjustments were made in the code controlling metro process such that it handles metro crashes and outputs some important messages better
Apple0717
added a commit
to Apple0717/radon
that referenced
this pull request
Nov 30, 2024
With Expo 50, we no longer can you same metro runner as they've landed several changes that just breaks with the way we'd set up packager. This PR makes it so that we use similar path to starting packager with `expo start`. Here is what's changed: 1. When project has app.json, we use new expo start way of launching the packager 2. We extracted a number of metro related code to separate lib file such that it can be used both by old metro config and by expo setup 3. We blacklisted more jsx-transform modules, as they started conflicting with different setups 4. We're removing – (hyphen) char from default device names, as they started to cause issues with debugger (see facebook/react-native#42590) 5. Expo metro setup now has a new expo_start script that uses some internal method overrides in order to inject code that updates metro config 6. The expo setup is configured as CI such that it doesn't print too much formatted output 7. We also had to override package that tests for a busy port, because it didn't handle port nr 0 well 8. Finally we made small adjustments in runtime code in order to load lazily some internal RN modules 9. Also some small adjustments were made in the code controlling metro process such that it handles metro crashes and outputs some important messages better
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This change removes Content-Length header from proxy inspector response.
The presence of this header was resulting in the response being cropped under some circumstances because of erroneously calculated length.
The
Content-Lengthheader value represents the number of bytes in the response. In the code,string.lengthwas used to calculate that value, but in JavaScript it gives the number of characters in a string instead of its size in bytes. Specifically, if there are some UTF characters in the string that occupy more than byte, there would be a mismatch in this size. This mismatch resulted in the response being cropped.The easiest way to reproduce this problem is to set the simulator name to contain a two-byte UTF character.
This change works according to the HTTP spec, which states that when Content-Length is not present, the end of the response stream indicates the end of the response. Since in the code
response.end(data)is use, it terminates the stream and hence there is no need to provide the length in the header.Changelog:
[GENERAL] [FIXED] - fix issue with debugger not working when device name contain two-byte UTF characters
Test Plan: