Repository navigation
Conversation
Dial returned a non-nil response both when the server rejected the handshake with a non-101 status and when a 101 response failed validation, so callers checking resp != nil treated protocol violations as HTTP errors. Dial now wraps ErrBadStatus only for a non-101 status, and documents that the response and its first 1024 body bytes are available in that case.
Wrapping ErrBadStatus with %w inserted "websocket: bad status: " into the Dial error string, and public projects match on the existing text. An unexported error type now returns the previous text and unwraps to ErrBadStatus, so errors.Is still holds.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dialnow wraps a new sentinel error,ErrBadStatus, when the server answers the opening handshake with a status other than 101 Switching Protocols. Aftererrors.Is(err, websocket.ErrBadStatus), the returned response is non-nil andresp.Bodyholds up to the first 1024 bytes of the response body.Before this change, the only signal was a non-nil response, as advised in #424. But
Dialalso returns a non-nil response when a 101 response fails validation: an invalidConnection,UpgradeorSec-WebSocket-Acceptheader, an unexpected subprotocol or extension, or a body that is not writable. A caller that treats every non-nil response as an HTTP rejection reports those protocol failures as rejections.ErrBadStatusis not wrapped in those cases, or when no response arrives.The error text is unchanged on purpose, for example
failed to WebSocket dial: expected handshake response status code 101 but got 403, because public projects match on it. An unexported error type returns that text and unwraps toErrBadStatus.On js/wasm,
Dialnever wrapsErrBadStatus, because the browser does not expose the handshake response.New tests cover a 403 with a 1025-byte body, a 101 with an invalid
Sec-WebSocket-Accept, and a refused connection. The 403 test fails on master with only the variable declared. Lint, race tests and the WASM test pass.Refs #424, #528
Decision log
ErrBadStatusin golang.org/x/net/websocket.Unwrapinstead of anIsmethod, soErrBadStatusis in the error chain that the docs describe as wrapped.%wprefix (websocket: bad status: expected ...), because it changes text that public projects match on.*http.Response, because its body is the same reader as the returned response's body, so one read empties both.resp.StatusCode != 101check only, because it gives no condition thaterrors.Iscan test after a caller wraps the error.Dialstill closes the body.