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.

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

Removes Sec-WebSocket-Origin From Websocket HS #9137

Closed
wants to merge 1 commit into from

Conversation

davydotcom
Copy link

@davydotcom davydotcom commented May 8, 2019

Sec-WebSocket-Origin is a Server to Client handshake not a Client to Server handshake header per the websocket RFC specification.

This removes the header from the specification. This has been changed because some websocket proxy servers are very picky. There exists other areas that need adjusted for example this client should send "Connection: Upgrade" but instead sends "connection: upgrade"... Some Proxy WS Clients are not adhering properly to case insensitivity specifications and require the proper casing for the keys and values being sent.

Fixes Issue #9134

Sec-WebSocket-Origin is a Server to Client handshake not a Client to Server handshake header per the websocket RFC specification. This Resolves Issue netty#9134
@netty-bot
Copy link

Can one of the admins verify this patch?

@normanmaurer
Copy link
Member

@davydotcom can you point me to the relevant RFC section and add a unit-test ?

@davydotcom
Copy link
Author

@amizurov
Copy link
Sponsor Contributor

amizurov commented May 9, 2019

@davydotcom, @normanmaurer Hi guys. The Sec-WebSocket-Origin header for old protocol versions (https://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-07, https://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-10). In the latest version https://tools.ietf.org/html/rfc6455 instead of Sec-WebSocket-Origin used Origin header. My suggestion is to replace it and not to delete it at all.

@davydotcom
Copy link
Author

davydotcom commented May 9, 2019 via email

@amizurov
Copy link
Sponsor Contributor

amizurov commented May 9, 2019

@davydotcom I suggest doing this for consistency with previous versions of the protocol (WebSocketClientHandshaker00, 07, 08). Even if you decide not to do this, please fix the documentation WebSocketServerHandshaker13#newHandshakeResponse also.

@normanmaurer
Copy link
Member

@netty-bot test this please.

@normanmaurer
Copy link
Member

@davydotcom please fix the unit tests and let me know once done:

https://ci.netty.io/job/netty-centos6-java8-prb/1029/

@normanmaurer
Copy link
Member

@davydotcom ping.. .also please sign our ICLA: https://netty.io/s/icla

@normanmaurer
Copy link
Member

Closing this due no response from original reporter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants