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

Correctly support websocket close codes #8975

Merged
merged 2 commits into from
Apr 1, 2020

Conversation

RX14
Copy link
Member

@RX14 RX14 commented Mar 30, 2020

The first two bytes of the message in a WebSocket Close frame is anumeric code. Make sure to always encode this correctly, to avoid being in violation of the websocket spec. Also automatically decode this for clients.

This is a breaking change since on_close now takes two arguments: close code and message, but this change is unable to be made in a backwards compatible way.

Closes #6469

@RX14 RX14 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:feature breaking-change topic:stdlib:networking labels Mar 30, 2020
@RX14
Copy link
Member Author

RX14 commented Mar 30, 2020

Added HTTP::WebSocket::CloseCodes and refactored the ugly case statement

@Sija
Copy link
Contributor

Sija commented Mar 30, 2020

Just for the reference, here it is a list of error codes from IANA -> https://www.iana.org/assignments/websocket/websocket.xhtml#close-code-number

@RX14
Copy link
Member Author

RX14 commented Mar 31, 2020

This is essentially a bug in websockets right now, so I'd really like if this could make it into 0.34.0 because it's affecting a number of people I know. Changes to accept an enum could be made later. If the constant styling is unacceptable to the core team I'm happy to change it, but it'd be nice to merge this as-is right now...

@RX14 RX14 added this to the 0.34.0 milestone Apr 1, 2020
The first two bytes of the message in a WebSocket Close frame is a
numeric code. Make sure to always encode this correctly, to avoid being
in violation of the websocket spec. Also automatically decode this for
clients. This is a breaking change since on_close now takes two
arguments: close code and message, but this change was unable to be made
in a backwards compatible way.

Closes crystal-lang#6469
Closes crystal-lang#7483
@RX14 RX14 force-pushed the feature/websocket-close-codes branch from d2c35ec to 46723ce Compare April 1, 2020 11:00
@RX14
Copy link
Member Author

RX14 commented Apr 1, 2020

Squashed and added the extra close codes from the registry. I'll merge this once CI passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:feature topic:stdlib:networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebSocket close frame messages aren't being parsed
6 participants