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

Send full response for unsupported websocket versions #3999

Closed
wants to merge 1 commit into from

Conversation

jroper
Copy link
Contributor

@jroper jroper commented Jul 17, 2015

When the WebSocketServerHandshakerFactory sent a response if the websocket verison was not expected, it had three problems, first it didn't send a LastHttpContent, leaving any upstream handlers that were tracking responses thinking the response wasn't finished, second it didn't flush, so the response was never actually sent, and third, if it did somehow manage to flush, it didn't send a content length, leaving the client waiting to receive a close delimited body.

@netkins
Copy link

netkins commented Jul 17, 2015

AUTOMATED MESSAGE for ADMINS:
Please verify and accept the pull request.
To accept, say @netkins accept
To build again, say @netkins build
To whitelist the author, say @netkins whitelist

@normanmaurer
Copy link
Member

@netkins accept

@@ -0,0 +1,51 @@
/*
* Copyright 2013 The Netty Project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2015

@normanmaurer
Copy link
Member

@jroper thanks! Could you please address my two comments and adjust the commit message to match our commit message template:

https://github.com/netty/netty/wiki/Writing-a-commit-message

After this I will cherry-pick. Thanks!

@normanmaurer normanmaurer self-assigned this Jul 17, 2015
@normanmaurer normanmaurer added this to the 4.0.30.Final milestone Jul 17, 2015
Motivation:

WebSocketServerHandshakerFactory.sendUnsupportedVersionResponse does not
send a LastHttpContent, nor does it flush, and it doesn't send a content
length.

Modifications:

Changed sendUnsupportedVersionResponse to send FullHttpResponse, to
writeAndFlush, and to set a content length of 0. Also added a test for
this method.

Result:

Upstream handlers will be able to determine the end of the response, the
response will actually get written to the client, and the client will be
able to determine the end of the response.
@jroper
Copy link
Contributor Author

jroper commented Jul 17, 2015

@normanmaurer done. By the way, I don't have any need for this change to be back ported, I found the problems when I copied the code to create the response (I couldn't use the method because I didn't want to write it immediately to the channel), so I thought I'd submit this fix.

@normanmaurer
Copy link
Member

@jroper thanks!

Cherry-picked into 4.0 (71ca935), 4.1 (b958263) and master (84f5996)

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

Successfully merging this pull request may close these issues.

None yet

3 participants