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

Protocol errors should send a close frame and shutdown the connection #38

Open
bluetech opened this issue Jul 10, 2018 · 6 comments
Open

Comments

@bluetech
Copy link
Contributor

The RFC defines a "Fail the WebSocket Connection" procedure:

   Certain algorithms and specifications require an endpoint to _Fail
   the WebSocket Connection_.  To do so, the client MUST _Close the
   WebSocket Connection_, and MAY report the problem to the user (which
   would be especially useful for developers) in an appropriate manner.
   Similarly, to do so, the server MUST _Close the WebSocket
   Connection_, and SHOULD log the problem.

Grepping the RFC for "Fail the WebSocket Connection" brings up the following cases:

  • If a nonzero value [for RSV1-3] is received and none of the negotiated extensions defines the meaning of such a nonzero value

  • If an unknown opcode is received

  • When an endpoint is to interpret a byte stream as UTF-8 but finds that the byte stream is not, in fact, a valid UTF-8 stream

I've ignored errors in the handshake phase since these should be naturally handled correctly I think.

Currently tungstenite does not follow the RFC; when one of these conditions happen the error is propagated to the caller but the WebSocket is still usable. Beyond just conformance I think making these errors unrecoverable makes sense, since something is obviously wrong. Continuing like nothing happened may turn up garbage.

@agalakhov
Copy link
Member

This effectively means that the caller must drop WebSocket if an error was returned.

@bluetech
Copy link
Contributor Author

bluetech commented Jul 10, 2018

I was thinking that the WebSocket would change its state to CLOSING, upon which all future operations should fail-fast.

@agalakhov
Copy link
Member

It doesn't, it just reports an error to you. A natural user's reaction to an error is dropping a connection. Probably I should initiate CLOSING earlier.

@bluetech
Copy link
Contributor Author

Sorry for the confusion - what I mean in the previous comment is what I think should happen in case one of the conditions occur. I think that when an unrecoverable error occurs, the WebSocket should transition to CLOSING and all furthers operations on the WebSocket should fail immediately (when doing anything). This is how the WebSocket API in browsers works, I think (link, see INVALID_STATE_ERR).

@agalakhov
Copy link
Member

Thank you, will be implemented.

@najamelan
Copy link
Contributor

A natural user's reaction to an error is dropping a connection.

I don't understand what would make it "natural", but it sure would save users a lot of time if this assumption would be documented.

Note that dropping the connection and doing a close handshake are mutually exclusive, because if the other end is trying to acknowledge the close frame, they will get ConnectionReset if it's dropped.

psmit pushed a commit to psmit/tungstenite-rs that referenced this issue Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants