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

Issue #6642 - WebSocket handling of Connection: upgrade,close. #6657

Merged

Conversation

lachlan-roberts
Copy link
Contributor

Closes #6642

add tests for WebSocket Connection Headers.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@joakime
Copy link
Contributor

joakime commented Aug 24, 2021

Here's one websocket lib making the same discovery we just did with the "close" token.

warmcat/libwebsockets#1435 (comment)

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Copy link
Contributor Author

@joakime @gregw I have added back the test case for Connection: close, Upgrade and have added some code in HttpGenerator which prevents the connection from being closed.

@joakime
Copy link
Contributor

joakime commented Aug 24, 2021

Should we offer a configuration for websocket to satisfy behaviors of bad websocket clients?

What if the naive websocket client sends a normal Connection: upgrade, but the upgrade doesn't happen.
We respond with a normal HTTP response, and now the connection is kept open on the server side.

I wonder if we should have a configuration that can force a Connection: close on the response of bad upgrades to help poor quality websocket clients?
Is this even a concern?

@joakime
Copy link
Contributor

joakime commented Aug 24, 2021

Looks like org.eclipse.jetty.http.HttpGeneratorServerTest.testResponseUpgrade is failing due to this change.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@gregw gregw changed the title Issue #6642 - add tests for WebSocket Connection Headers. Issue #6642 - WebSocket handling of Connection: upgrade,close. Aug 25, 2021
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
…nnection

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
… response.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts merged commit 0a78b98 into jetty-10.0.x Aug 31, 2021
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-6642-WebSocketConnectionHeaders branch August 31, 2021 04:25
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.

WebSocket handling of Connection: upgrade,close.
3 participants