[Android][Websockets] - Origin header check shouldn't be case sensitive#27827
Closed
brunobar79 wants to merge 3 commits into
Closed
[Android][Websockets] - Origin header check shouldn't be case sensitive#27827brunobar79 wants to merge 3 commits into
brunobar79 wants to merge 3 commits into
Conversation
facebook-github-bot
added
the
CLA Signed
brunobar79
changed the title
brunobar79
changed the title
cpojer
approved these changes
Jan 27, 2020
Contributor
cpojer
left a comment
cpojer
left a comment
There was a problem hiding this comment.
This makes sense, thank you!
facebook-github-bot
approved these changes
Jan 27, 2020
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Collaborator
|
This pull request was successfully merged by @brunobar79 in aeaf286. When will my fix make it into a release? | Upcoming Releases |
osdnk
pushed a commit
to osdnk/react-native
that referenced
this pull request
Mar 9, 2020
Summary: Based on [this](https://stackoverflow.com/a/5259004), header names are not case sensitive. That means that it's valid to pass a header `Origin` or `origin`. With the current implementation. on Android only, if you pass `Origin`, it will get overwritten by the default origin. This made me waste a lot of time debugging a problem while trying to connect to a websockets server that required an `origin` header and my implementation was working fine in iOS but not on Android, so I'm suggest changing the logic a little bit to take that into account. ## Changelog [Android] [Fixed] - Support for case insensitive "Origin" headers for Websockets Pull Request resolved: facebook#27827 Test Plan: Here's a screenshot of that shows the issue before the fix (`Origin` header set, but getting overridden)  The fix is not that easy to test since it requires a public websocket server that checks for a custom Origin header, but I think the code changes are very small and clear. Differential Revision: D19578860 Pulled By: cpojer fbshipit-source-id: d854e887d1b9e8e54da662b2da2ebe08ce65fdbc
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
4 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
Based on this, header names are not case sensitive.
That means that it's valid to pass a header
Originororigin.With the current implementation. on Android only, if you pass
Origin, it will get overwritten by the default origin.This made me waste a lot of time debugging a problem while trying to connect to a websockets server that required an
originheader and my implementation was working fine in iOS but not on Android, so I'm suggest changing the logic a little bit to take that into account.Changelog
[Android] [Fixed] - Support for case insensitive "Origin" headers for Websockets
Test Plan
Here's a screenshot of that shows the issue before the fix (
Originheader set, but getting overridden)The fix is not that easy to test since it requires a public websocket server that checks for a custom Origin header, but I think the code changes are very small and clear.