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

Bugfix #9673: Origin header is always sent from WebSocket client #9692

Merged
merged 2 commits into from Nov 27, 2019
Merged

Bugfix #9673: Origin header is always sent from WebSocket client #9692

merged 2 commits into from Nov 27, 2019

Conversation

ursaj
Copy link
Contributor

@ursaj ursaj commented Oct 18, 2019

Motivation:

Those who need 'Origin' or 'Sec-WebSocket-Origin' headers should provide them explicitly, like it is stated in WebSocket specs.

E.g. through custom headers:

HttpHeaders customHeaders = new DefaultHttpHeaders()
    .add(HttpHeaderNames.ORIGIN, "http://localhost:8080");
new WebSocketClientProtocolHandler(
    new URI("ws://localhost:1234/test"), WebSocketVersion.V13, subprotocol, 
    allowExtensions, customHeaders, maxFramePayloadLength, handshakeTimeoutMillis)

Modification:

  • Remove enforced origin headers.
  • Update tests

Result:

Fixes #9673: Origin header is always sent from WebSocket client

@netty-bot
Copy link

Can one of the admins verify this patch?

@normanmaurer
Copy link
Member

/cc @trustin @vietj WDYT ?

@ursaj
Copy link
Contributor Author

ursaj commented Nov 3, 2019

@normanmaurer, @trustin, @vietj, @slandelle
hi, guys. 2 weeks have passed from the last update...
could we proceed with this PR? or should I re-open a new one, like it happened with #8308?

@normanmaurer
Copy link
Member

@slandelle @vietj any idea ?

@ursaj
Copy link
Contributor Author

ursaj commented Nov 17, 2019

hi, guys. yet another 2 weeks have passed...

@normanmaurer
Copy link
Member

@ursaj can you link me to the details in the spec ?

@ursaj
Copy link
Contributor Author

ursaj commented Nov 19, 2019

https://tools.ietf.org/html/rfc6455

The |Origin| header field [RFC6454] is used to protect against
unauthorized cross-origin use of a WebSocket server by scripts using
the WebSocket API in a web browser. The server is informed of the
script origin generating the WebSocket connection request. If the
server does not wish to accept connections from this origin, it can
choose to reject the connection by sending an appropriate HTTP error
code. This header field is sent by browser clients; for non-browser
clients, this header field may be sent if it makes sense in the
context of those clients.

@ursaj
Copy link
Contributor Author

ursaj commented Nov 19, 2019 •
edited

The same is valid for draft versions of WebSocket protocol, e.g.:
https://tools.ietf.org/id/draft-ietf-hybi-thewebsocketprotocol-06.html#rfc.section.1.3

The |Sec-WebSocket-Origin| header is used to protect against unauthorized cross-origin use of a WebSocket server by scripts using the |WebSocket| API in a Web browser. The server is informed of the script origin generating the WebSocket connection request. If the server does not wish to accept connections from this origin, it can choose to abort the connection. This header is sent by browser clients, for non-browser clients this header may be sent if it makes sense in the context of those clients.

@normanmaurer
Copy link
Member

/cc @amizurov

@normanmaurer normanmaurer merged commit 19a4633 into netty:4.1 Nov 27, 2019
@normanmaurer
Copy link
Member

@ursaj sorry for the slow turn around

@normanmaurer normanmaurer added this to the 4.1.44.Final milestone Nov 27, 2019
normanmaurer pushed a commit that referenced this pull request Nov 27, 2019
@ursaj @normanmaurer
Bugfix #9673: Origin header is always sent from WebSocket client (#9692)
f48d9fa 
Those who need 'Origin' or 'Sec-WebSocket-Origin' headers should provide them explicitly, like it is stated in WebSocket specs.

E.g. through custom headers:

    HttpHeaders customHeaders = new DefaultHttpHeaders()
        .add(HttpHeaderNames.ORIGIN, "http://localhost:8080");
    new WebSocketClientProtocolHandler(
        new URI("ws://localhost:1234/test"), WebSocketVersion.V13, subprotocol,
        allowExtensions, customHeaders, maxFramePayloadLength, handshakeTimeoutMillis)

* Remove enforced origin headers.
* Update tests

Fixes #9673: Origin header is always sent from WebSocket client
@ursaj ursaj deleted the bugfix-9673-invalid-origin-header branch November 27, 2019 08:41
@ursaj
Copy link
Contributor Author

ursaj commented Nov 27, 2019

obstacles (slow turn around) are not so important, when job is done =)

@normanmaurer
Copy link
Member

Sorry I need to revert this as it needs more thoughts.

normanmaurer added a commit that referenced this pull request Dec 18, 2019
@normanmaurer
Revert "Bugfix #9673: Origin header is always sent from WebSocket cli…
d24dfb1 
…ent (#9692)"

This reverts commit f48d9fa as this needs more thoughts.
normanmaurer added a commit that referenced this pull request Dec 18, 2019
@normanmaurer
Revert "Bugfix #9673: Origin header is always sent from WebSocket cli…
607cf80 
…ent (#9692)"

This reverts commit f48d9fa as it needs more thoughts
ihanyong pushed a commit to ihanyong/netty that referenced this pull request Jul 31, 2020
@normanmaurer @ihanyong
Revert "Bugfix netty#9673: Origin header is always sent from WebSocke…
9ff697a 
…t client (netty#9692)"

This reverts commit f48d9fa as it needs more thoughts
@amizurov amizurov mentioned this pull request Oct 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slandelle slandelle Awaiting requested review from slandelle

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.1.45.Final
Development

Successfully merging this pull request may close these issues.

Origin header is always sent from WebSocket client
3 participants
@ursaj @netty-bot @normanmaurer