Skip to content

Don't defer WebSocket opening #1348

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

Closed
slandelle opened this issue Feb 7, 2017 · 0 comments
Closed

Don't defer WebSocket opening #1348

slandelle opened this issue Feb 7, 2017 · 0 comments
Assignees
Labels
Milestone

Comments

@slandelle
Copy link
Contributor

We currently buffer WebSocket opening until first LastHttpContent reception with the UpgradeCallback .
This doesn't make sense, and forces us to buffer any frame that might be sent along with the upgrade response.

WebSocketHandler and WebSocketUpgradeHandler need serious clean up:

  • Drop UpgradeHandler that's never used as an abstraction
  • Perform upgrade/abort as soon as response is received
  • Ignore LastHttpContent
  • No need to buffer any frame
@slandelle slandelle added this to the 2.1.0 milestone Feb 7, 2017
@slandelle slandelle self-assigned this Feb 7, 2017
slandelle added a commit that referenced this issue Feb 7, 2017
Motivation:

We currently buffer WebSocket opening until first LastHttpContent
reception with the UpgradeCallback.
This doesn't make sense, and forces us to buffer any frame that might
be sent along with the upgrade response.

Modifications:

* Drop UpgradeHandler that's never used as an abstraction
* Perform upgrade/abort as soon as response is received
* Ignore LastHttpContent
* No need to buffer any frame

Result:

More simple code
slandelle added a commit that referenced this issue Feb 7, 2017
Motivation:

We currently buffer WebSocket opening until first LastHttpContent
reception with the UpgradeCallback.
This doesn't make sense, and forces us to buffer any frame that might
be sent along with the upgrade response.

Modifications:

* Drop UpgradeHandler that's never used as an abstraction
* Perform upgrade/abort as soon as response is received
* Ignore LastHttpContent
* No need to buffer any frame

Result:

More simple code
slandelle added a commit that referenced this issue Mar 8, 2017
… Handler, close #1364

Motivation:

Installing the WebSocket handler can trigger a read, so if there's a
pending frame that was send along the HTTP upgrade response, we could
crash because the handler is not properly configured yet.

This is a regression caused by #1348.

Modifications:

Buffer frames until WebSocket is fully open.

Result:

No more crash when frames are sent along the HTTP upgrade response.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant