Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support WebSocket sub-protocols on Android #5810

Closed
mikemintz opened this issue Feb 8, 2016 · 3 comments
Closed

Support WebSocket sub-protocols on Android #5810

mikemintz opened this issue Feb 8, 2016 · 3 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@mikemintz
Copy link

I rely on the protocols argument in the WebSocket constructor to negotiate the client/server protocol. It looks like it's supported in react-native iOS, but Android appears to be ignoring it.

See WebSocketModule.java line 66
@facebook-github-bot
Copy link
Contributor

Hey mikemintz, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@mikemintz mikemintz mentioned this issue Feb 8, 2016
Open
@mikemintz mikemintz mentioned this issue Feb 24, 2016
Closed
@sodik82
Copy link

sodik82 commented Feb 25, 2016

+1: I am using STOMP.js over WS and it is working in iOS but not in Android. Most likely the same issue.

@zxcpoiu
Copy link
Contributor

zxcpoiu commented Feb 29, 2016

the WebSocketModule.java line 66 says:

// ignoring protocols, since OKHttp overrides them.

related code in okhttp: WebSocketCall.java line 71.

I tried to hard-coded in WebSocketModule.java line 87 with builder.addHeader("Sec-WebSocket-Protocol", "myprotocol");, and it DID add the header Sec-WebSocket-Protocol on the WebSocket handshake. (It did NOT get overridden by OKHttp.)

I tested it on react-native@0.20

Does that mean we can add back the support of websocket sub protocols now?

zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 1, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
0b54084 
…acebook#5810
@zxcpoiu zxcpoiu mentioned this issue Mar 1, 2016
Closed
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 10, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
37cee34 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 11, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
b8d866b 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 11, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
9717e60 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 11, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
fb80688 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 16, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
d687c4a 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Mar 23, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
1c551e9 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Apr 5, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
d5bd550 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Apr 6, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
6842da5 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Apr 6, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
286e633 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Apr 6, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
6999fe2 
…acebook#5810
zxcpoiu added a commit to zxcpoiu/react-native that referenced this issue Apr 11, 2016
@zxcpoiu
[WebSocket][Android] Support WebSocket sub-protocols on Android fixes f…
f88ca4c 
…acebook#5810
@ghost ghost closed this as completed in 914f33c Apr 11, 2016
zebulgar pushed a commit to nightingale/react-native that referenced this issue Jun 18, 2016
@zxcpoiu @zebulgar
make protocols argument work fixes facebook#5810 fix facebook#6137
4f037f4 
Summary:Hey there and thanks for submitting a pull request! Please have a look at the following checklist so that others have enough information to review your pull request:

**motivation**

WebSocket spec supports [Sec-WebSocket-Protocol](https://tools.ietf.org/html/rfc6455#section-11.3.4) as a standard way for negotiate a sub protocol between client and server.

* ios WebSocket implementation supports it.
* android WebSocket implementation ignores this header, leave a comment syas: "OkHttp will overrides it", so it did not implement.
* after some test, OkHttp doesn't override the header we add.

**Test plan (required)**

1. run and react-native app on android
2. at the main page, invoke: `var ws = new WebSocket('ws://example.ws-service.fakedomain.com', 'my-sub-protocol');`
3. see the header if it send the correct header, ex, use ngrep: `sudo ngrep -t -Wbyline -deth0 host example.ws-service.fakedomain.com and port 80`

you should see the WebSocket initial GET handshake includes header:
`Sec-WebSocke
Closes facebook#6223

Differential Revision: D3162822

fb-gh-sync-id: a00f1c0f3e1c24ad6aa234329cbb2abad7664264
fbshipit-source-id: a00f1c0f3e1c24ad6aa234329cbb2abad7664264
drewtunes pushed a commit to Tredsite/react-native that referenced this issue Aug 11, 2016
@zxcpoiu @drewtunes
make protocols argument work fixes facebook#5810 fix facebook#6137
3872a9f 
Summary:Hey there and thanks for submitting a pull request! Please have a look at the following checklist so that others have enough information to review your pull request:

**motivation**

WebSocket spec supports [Sec-WebSocket-Protocol](https://tools.ietf.org/html/rfc6455#section-11.3.4) as a standard way for negotiate a sub protocol between client and server.

* ios WebSocket implementation supports it.
* android WebSocket implementation ignores this header, leave a comment syas: "OkHttp will overrides it", so it did not implement.
* after some test, OkHttp doesn't override the header we add.

**Test plan (required)**

1. run and react-native app on android
2. at the main page, invoke: `var ws = new WebSocket('ws://example.ws-service.fakedomain.com', 'my-sub-protocol');`
3. see the header if it send the correct header, ex, use ngrep: `sudo ngrep -t -Wbyline -deth0 host example.ws-service.fakedomain.com and port 80`

you should see the WebSocket initial GET handshake includes header:
`Sec-WebSocke
Closes facebook#6223

Differential Revision: D3162822

fb-gh-sync-id: a00f1c0f3e1c24ad6aa234329cbb2abad7664264
fbshipit-source-id: a00f1c0f3e1c24ad6aa234329cbb2abad7664264
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mikemintz @christopherdro @zxcpoiu @sodik82 @facebook-github-bot @react-native-bot