From 7bd7405b0d681d8a7bc13a8c75f02b25e37a744a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 28 Oct 2026 10:27:26 +0000 Subject: [PATCH 01/12] test(accept): pin Accept rejection responses Accept's rejection responses had no test that checked the status, headers and body together. Pinning them for every rejection before the 101 lets a change to how the response is written prove that the default output stays the same. --- accept_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/accept_test.go b/accept_test.go index 92dbfcc7..cbc2216f 100644 --- a/accept_test.go +++ b/accept_test.go @@ -207,6 +207,155 @@ func TestAccept(t *testing.T) { }) } +func newHandshakeRequest() *http.Request { + r := httptest.NewRequest("GET", "/", nil) + r.Header.Set("Connection", "Upgrade") + r.Header.Set("Upgrade", "websocket") + r.Header.Set("Sec-WebSocket-Version", "13") + r.Header.Set("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==") + return r +} + +// acceptRejectionTests covers every rejection before the 101 response. The +// http.ResponseWriter is an httptest.ResponseRecorder, which does not +// implement http.Hijacker. +var acceptRejectionTests = []struct { + name string + modify func(r *http.Request) + opts AcceptOptions + wantStatus int + wantErr string + // wantHeader is the response header when the handler runs. + wantHeader http.Header +}{ + { + name: "http1.0", + modify: func(r *http.Request) { + r.Proto, r.ProtoMajor, r.ProtoMinor = "HTTP/1.0", 1, 0 + }, + wantStatus: http.StatusUpgradeRequired, + wantErr: `WebSocket protocol violation: handshake request must be at least HTTP/1.1: "HTTP/1.0"`, + wantHeader: http.Header{}, + }, + { + name: "noConnectionUpgrade", + modify: func(r *http.Request) { r.Header.Set("Connection", "keep-alive") }, + wantStatus: http.StatusUpgradeRequired, + wantErr: `WebSocket protocol violation: Connection header "keep-alive" does not contain Upgrade`, + wantHeader: http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}, + }, + { + name: "noUpgradeWebSocket", + modify: func(r *http.Request) { r.Header.Set("Upgrade", "h2c") }, + wantStatus: http.StatusUpgradeRequired, + wantErr: `WebSocket protocol violation: Upgrade header "h2c" does not contain websocket`, + wantHeader: http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}, + }, + { + name: "methodNotGet", + modify: func(r *http.Request) { r.Method = "POST" }, + wantStatus: http.StatusMethodNotAllowed, + wantErr: `WebSocket protocol violation: handshake request method is not GET but "POST"`, + wantHeader: http.Header{}, + }, + { + name: "unsupportedVersion", + modify: func(r *http.Request) { r.Header.Set("Sec-WebSocket-Version", "8") }, + wantStatus: http.StatusBadRequest, + wantErr: `unsupported WebSocket protocol version (only 13 is supported): "8"`, + wantHeader: http.Header{"Sec-Websocket-Version": {"13"}}, + }, + { + name: "missingKey", + modify: func(r *http.Request) { r.Header.Del("Sec-WebSocket-Key") }, + wantStatus: http.StatusBadRequest, + wantErr: "WebSocket protocol violation: missing Sec-WebSocket-Key", + wantHeader: http.Header{}, + }, + { + name: "multipleKeys", + modify: func(r *http.Request) { r.Header.Add("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==") }, + wantStatus: http.StatusBadRequest, + wantErr: "WebSocket protocol violation: multiple Sec-WebSocket-Key headers", + wantHeader: http.Header{}, + }, + { + name: "invalidKey", + modify: func(r *http.Request) { r.Header.Set("Sec-WebSocket-Key", "notbase64") }, + wantStatus: http.StatusBadRequest, + wantErr: `WebSocket protocol violation: invalid Sec-WebSocket-Key "notbase64", must be a 16 byte base64 encoded string`, + wantHeader: http.Header{}, + }, + { + name: "unauthorizedOrigin", + modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, + wantStatus: http.StatusForbidden, + wantErr: `request Origin "other.com" is not authorized for Host "example.com"`, + wantHeader: http.Header{}, + }, + { + // The pattern error is logged and not sent to the client. + name: "badOriginPattern", + modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, + opts: AcceptOptions{OriginPatterns: []string{"["}}, + wantStatus: http.StatusForbidden, + wantErr: "Forbidden", + wantHeader: http.Header{}, + }, + { + name: "noHijacker", + wantStatus: http.StatusNotImplemented, + wantErr: "http.ResponseWriter does not implement http.Hijacker", + wantHeader: http.Header{}, + }, +} + +// TestAcceptRejectionDefault pins the response Accept writes without an +// ErrorHandler. +func TestAcceptRejectionDefault(t *testing.T) { + t.Parallel() + + plain := func(h http.Header) http.Header { + h.Set("Content-Type", "text/plain; charset=utf-8") + h.Set("X-Content-Type-Options", "nosniff") + return h + } + want := map[string]struct { + header http.Header + body string + }{ + "http1.0": {plain(http.Header{}), `WebSocket protocol violation: handshake request must be at least HTTP/1.1: "HTTP/1.0"` + "\n"}, + "noConnectionUpgrade": {plain(http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}), `WebSocket protocol violation: Connection header "keep-alive" does not contain Upgrade` + "\n"}, + "noUpgradeWebSocket": {plain(http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}), `WebSocket protocol violation: Upgrade header "h2c" does not contain websocket` + "\n"}, + "methodNotGet": {plain(http.Header{}), `WebSocket protocol violation: handshake request method is not GET but "POST"` + "\n"}, + "unsupportedVersion": {plain(http.Header{"Sec-Websocket-Version": {"13"}}), `unsupported WebSocket protocol version (only 13 is supported): "8"` + "\n"}, + "missingKey": {plain(http.Header{}), "WebSocket protocol violation: missing Sec-WebSocket-Key\n"}, + "multipleKeys": {plain(http.Header{}), "WebSocket protocol violation: multiple Sec-WebSocket-Key headers\n"}, + "invalidKey": {plain(http.Header{}), `WebSocket protocol violation: invalid Sec-WebSocket-Key "notbase64", must be a 16 byte base64 encoded string` + "\n"}, + "unauthorizedOrigin": {plain(http.Header{}), `request Origin "other.com" is not authorized for Host "example.com"` + "\n"}, + "badOriginPattern": {plain(http.Header{}), "Forbidden\n"}, + "noHijacker": {plain(http.Header{}), "Not Implemented\n"}, + } + + for _, tc := range acceptRejectionTests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + r := newHandshakeRequest() + if tc.modify != nil { + tc.modify(r) + } + w := httptest.NewRecorder() + + _, err := Accept(w, r, &tc.opts) + assert.Equal(t, "err", "failed to accept WebSocket connection: "+tc.wantErr, err.Error()) + assert.Equal(t, "status", tc.wantStatus, w.Code) + assert.Equal(t, "header", want[tc.name].header, w.Header()) + assert.Equal(t, "body", want[tc.name].body, w.Body.String()) + }) + } +} + func Test_verifyClientHandshake(t *testing.T) { t.Parallel() From 0eca8fed1f9c551b6ba77403c4e66a3463959927 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 28 Oct 2026 10:27:37 +0000 Subject: [PATCH 02/12] feat: add AcceptOptions.ErrorHandler Accept always wrote a plain text http.Error response when it rejected a handshake, so servers with a structured error format had to write a second response, which net/http appends to Accept's body and logs as a superfluous WriteHeader call. ErrorHandler lets the caller write that response instead, with the same signature as gorilla's Upgrader.Error; nil keeps the previous output. Refs #545 --- accept.go | 33 ++++++++++++++++--- accept_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++ ws_js.go | 1 + 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/accept.go b/accept.go index 25c1be7d..1d6a442b 100644 --- a/accept.go +++ b/accept.go @@ -79,6 +79,16 @@ type AcceptOptions struct { // Unlike OnPingReceived, this callback does not return a value because a pong frame // is a response to a ping and does not trigger any further frame transmission. OnPongReceived func(ctx context.Context, payload []byte) + + // ErrorHandler writes the HTTP response when Accept rejects the handshake + // request. status is the HTTP status Accept chose, w.Header() holds the + // headers Accept set for the rejection, and Accept returns an error + // wrapping err. ErrorHandler must write a response; otherwise net/http + // sends 200 OK with an empty body. If nil, Accept writes a plain text + // response with http.Error. ErrorHandler is not called when hijacking the + // connection fails, because Accept has already written 101 Switching + // Protocols. + ErrorHandler func(w http.ResponseWriter, r *http.Request, status int, err error) } func (opts *AcceptOptions) cloneWithDefaults() *AcceptOptions { @@ -95,7 +105,9 @@ func (opts *AcceptOptions) cloneWithDefaults() *AcceptOptions { // Accept will not allow cross origin requests by default. // See the InsecureSkipVerify and OriginPatterns options to allow cross origin requests. // -// Accept will write a response to w on all errors. +// Accept writes a response to w on all errors, through +// AcceptOptions.ErrorHandler when it rejects the handshake request. Do not +// write to w after Accept returns an error. // // Note that using the http.Request Context after Accept returns may lead to // unexpected behavior (see http.Hijacker). @@ -106,13 +118,14 @@ func Accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (*Conn, func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Conn, err error) { defer errd.Wrap(&err, "failed to accept WebSocket connection") + opts = opts.cloneWithDefaults() + errCode, err := verifyClientRequest(w, r) if err != nil { - http.Error(w, err.Error(), errCode) + opts.writeRejection(w, r, errCode, err, err.Error()) return nil, err } - opts = opts.cloneWithDefaults() if !opts.InsecureSkipVerify { err = authenticateOrigin(r, opts.OriginPatterns) if err != nil { @@ -120,7 +133,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con log.Printf("websocket: %v", err) err = errors.New(http.StatusText(http.StatusForbidden)) } - http.Error(w, err.Error(), http.StatusForbidden) + opts.writeRejection(w, r, http.StatusForbidden, err, err.Error()) return nil, err } } @@ -128,7 +141,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con hj, ok := hijacker(w) if !ok { err = errors.New("http.ResponseWriter does not implement http.Hijacker") - http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) + opts.writeRejection(w, r, http.StatusNotImplemented, err, http.StatusText(http.StatusNotImplemented)) return nil, err } @@ -181,6 +194,16 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con }), nil } +// writeRejection writes the response for a handshake request that Accept +// rejects. body is the http.Error text used when ErrorHandler is nil. +func (opts *AcceptOptions) writeRejection(w http.ResponseWriter, r *http.Request, status int, err error, body string) { + if opts.ErrorHandler != nil { + opts.ErrorHandler(w, r, status, err) + return + } + http.Error(w, body, status) +} + func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ error) { if !r.ProtoAtLeast(1, 1) { return http.StatusUpgradeRequired, fmt.Errorf("WebSocket protocol violation: handshake request must be at least HTTP/1.1: %q", r.Proto) diff --git a/accept_test.go b/accept_test.go index cbc2216f..83b3f2d0 100644 --- a/accept_test.go +++ b/accept_test.go @@ -5,6 +5,7 @@ package websocket import ( "bufio" "errors" + "io" "net" "net/http" "net/http/httptest" @@ -310,6 +311,93 @@ var acceptRejectionTests = []struct { }, } +func TestAcceptErrorHandler(t *testing.T) { + t.Parallel() + + for _, tc := range acceptRejectionTests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + r := newHandshakeRequest() + if tc.modify != nil { + tc.modify(r) + } + w := httptest.NewRecorder() + + var ( + calls int + gotStatus int + gotErr error + gotHeader http.Header + ) + opts := tc.opts + opts.ErrorHandler = func(w http.ResponseWriter, _ *http.Request, status int, err error) { + calls++ + gotStatus, gotErr, gotHeader = status, err, w.Header().Clone() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"message":"custom"}`)) + } + + _, err := Accept(w, r, &opts) + assert.Equal(t, "handler calls", 1, calls) + assert.Equal(t, "handler status", tc.wantStatus, gotStatus) + assert.Equal(t, "handler err", tc.wantErr, gotErr.Error()) + assert.ErrorIs(t, gotErr, err) + assert.Equal(t, "header when handler runs", tc.wantHeader, gotHeader) + + assert.Equal(t, "response status", tc.wantStatus, w.Code) + assert.Equal(t, "response content type", "application/json", w.Header().Get("Content-Type")) + assert.Equal(t, "response body", `{"message":"custom"}`, w.Body.String()) + }) + } + + // The handler response reaches a client over a real connection, and a + // handler that writes nothing leaves net/http's default 200 OK. + t.Run("server", func(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + write bool + wantStatus int + wantBody string + }{ + {name: "writes", write: true, wantStatus: http.StatusForbidden, wantBody: `{"message":"custom"}`}, + {name: "writesNothing", write: false, wantStatus: http.StatusOK, wantBody: ""}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = Accept(w, r, &AcceptOptions{ + ErrorHandler: func(w http.ResponseWriter, _ *http.Request, status int, _ error) { + if tc.write { + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"message":"custom"}`)) + } + }, + }) + })) + defer s.Close() + + req, err := http.NewRequest("GET", s.URL, nil) + assert.Success(t, err) + req.Header = newHandshakeRequest().Header + req.Header.Set("Origin", "https://other.com") + res, err := s.Client().Do(req) + assert.Success(t, err) + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + assert.Success(t, err) + + assert.Equal(t, "status", tc.wantStatus, res.StatusCode) + assert.Equal(t, "body", tc.wantBody, string(body)) + }) + } + }) +} + // TestAcceptRejectionDefault pins the response Accept writes without an // ErrorHandler. func TestAcceptRejectionDefault(t *testing.T) { diff --git a/ws_js.go b/ws_js.go index 026b75fc..b39ca4b5 100644 --- a/ws_js.go +++ b/ws_js.go @@ -439,6 +439,7 @@ type AcceptOptions struct { OriginPatterns []string CompressionMode CompressionMode CompressionThreshold int + ErrorHandler func(w http.ResponseWriter, r *http.Request, status int, err error) } // Accept is stubbed out for Wasm. From 06b6aacabd8fe2df3614fe4c94a3e9ddc84e698f Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:38:29 +0000 Subject: [PATCH 03/12] refactor(accept.go): rename writeRejection body to defaultBody The doc comment on writeRejection only existed to explain that body is the fallback text for a nil ErrorHandler. The name defaultBody says that at each call site, so the comment is removed. --- accept.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/accept.go b/accept.go index 1d6a442b..52277459 100644 --- a/accept.go +++ b/accept.go @@ -194,14 +194,12 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con }), nil } -// writeRejection writes the response for a handshake request that Accept -// rejects. body is the http.Error text used when ErrorHandler is nil. -func (opts *AcceptOptions) writeRejection(w http.ResponseWriter, r *http.Request, status int, err error, body string) { +func (opts *AcceptOptions) writeRejection(w http.ResponseWriter, r *http.Request, status int, err error, defaultBody string) { if opts.ErrorHandler != nil { opts.ErrorHandler(w, r, status, err) return } - http.Error(w, body, status) + http.Error(w, defaultBody, status) } func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ error) { From 6427739f61f653fe02d22c5feb6fe19b92b54c4a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:38:55 +0000 Subject: [PATCH 04/12] docs(accept.go): state that ErrorHandler err is not for the client The ErrorHandler doc did not say whether err is safe to send to the client. For noHijacker, err names a server-side interface, so a handler that writes err.Error() into the response would expose server details. --- accept.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/accept.go b/accept.go index 52277459..2a6b12c8 100644 --- a/accept.go +++ b/accept.go @@ -83,11 +83,12 @@ type AcceptOptions struct { // ErrorHandler writes the HTTP response when Accept rejects the handshake // request. status is the HTTP status Accept chose, w.Header() holds the // headers Accept set for the rejection, and Accept returns an error - // wrapping err. ErrorHandler must write a response; otherwise net/http - // sends 200 OK with an empty body. If nil, Accept writes a plain text - // response with http.Error. ErrorHandler is not called when hijacking the - // connection fails, because Accept has already written 101 Switching - // Protocols. + // wrapping err. err is a diagnostic for logging that may contain server + // details, and is not meant to be sent to the client. ErrorHandler must + // write a response; otherwise net/http sends 200 OK with an empty body. If + // nil, Accept writes a plain text response with http.Error. ErrorHandler + // is not called when hijacking the connection fails, because Accept has + // already written 101 Switching Protocols. ErrorHandler func(w http.ResponseWriter, r *http.Request, status int, err error) } From 5728d8ece1b58ab7bca6daa3e5373f92d235fd0d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:39:15 +0000 Subject: [PATCH 05/12] test(accept_test.go): check ErrorHandler is not called on hijack failure No test failed if Accept called ErrorHandler after a failed hijack, when the 101 status line is already written. The new case uses a hijacker that returns an error and asserts the handler never runs. --- accept_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/accept_test.go b/accept_test.go index 83b3f2d0..309adf70 100644 --- a/accept_test.go +++ b/accept_test.go @@ -352,6 +352,24 @@ func TestAcceptErrorHandler(t *testing.T) { }) } + t.Run("hijackFails", func(t *testing.T) { + t.Parallel() + + errHijack := errors.New("hijack error") + w := mockHijacker{ + ResponseWriter: httptest.NewRecorder(), + hijack: func() (net.Conn, *bufio.ReadWriter, error) { + return nil, nil, errHijack + }, + } + var calls int + _, err := Accept(w, newHandshakeRequest(), &AcceptOptions{ + ErrorHandler: func(http.ResponseWriter, *http.Request, int, error) { calls++ }, + }) + assert.ErrorIs(t, errHijack, err) + assert.Equal(t, "handler calls", 0, calls) + }) + // The handler response reaches a client over a real connection, and a // handler that writes nothing leaves net/http's default 200 OK. t.Run("server", func(t *testing.T) { From 273a0acc0794c1044daa8b1817f1e273429502f3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:39:51 +0000 Subject: [PATCH 06/12] test(accept_test.go): send the server subtest request over a raw conn The http.Client transport goroutines in the server subtest exited after TestMain counted goroutines, so filtered runs such as -run TestAccept failed the leak check intermittently. A raw TCP connection with Connection: close leaves no client goroutines behind. --- accept_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/accept_test.go b/accept_test.go index 309adf70..5df6831a 100644 --- a/accept_test.go +++ b/accept_test.go @@ -370,8 +370,7 @@ func TestAcceptErrorHandler(t *testing.T) { assert.Equal(t, "handler calls", 0, calls) }) - // The handler response reaches a client over a real connection, and a - // handler that writes nothing leaves net/http's default 200 OK. + // Checks the response a client receives from a real server. t.Run("server", func(t *testing.T) { t.Parallel() @@ -399,11 +398,16 @@ func TestAcceptErrorHandler(t *testing.T) { })) defer s.Close() - req, err := http.NewRequest("GET", s.URL, nil) + // A raw connection avoids http.Client transport goroutines, + // which outlive s.Close and fail the TestMain leak check. + c, err := net.Dial("tcp", s.Listener.Addr().String()) assert.Success(t, err) - req.Header = newHandshakeRequest().Header + defer c.Close() + req := newHandshakeRequest() + req.Header.Set("Connection", "Upgrade, close") req.Header.Set("Origin", "https://other.com") - res, err := s.Client().Do(req) + assert.Success(t, req.Write(c)) + res, err := http.ReadResponse(bufio.NewReader(c), req) assert.Success(t, err) defer res.Body.Close() body, err := io.ReadAll(res.Body) From 38ae5582a179094996500ec3abc7b53406662c84 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:40:25 +0000 Subject: [PATCH 07/12] test(accept_test.go): check ErrorHandler receives the Accept request Every test handler discarded the request argument, so passing nil or a copy to ErrorHandler left the tests passing. The table test now asserts the handler gets the same *http.Request that was passed to Accept. --- accept_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/accept_test.go b/accept_test.go index 5df6831a..12dcffac 100644 --- a/accept_test.go +++ b/accept_test.go @@ -326,14 +326,15 @@ func TestAcceptErrorHandler(t *testing.T) { var ( calls int + gotReq *http.Request gotStatus int gotErr error gotHeader http.Header ) opts := tc.opts - opts.ErrorHandler = func(w http.ResponseWriter, _ *http.Request, status int, err error) { + opts.ErrorHandler = func(w http.ResponseWriter, r *http.Request, status int, err error) { calls++ - gotStatus, gotErr, gotHeader = status, err, w.Header().Clone() + gotReq, gotStatus, gotErr, gotHeader = r, status, err, w.Header().Clone() w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) _, _ = w.Write([]byte(`{"message":"custom"}`)) @@ -341,6 +342,9 @@ func TestAcceptErrorHandler(t *testing.T) { _, err := Accept(w, r, &opts) assert.Equal(t, "handler calls", 1, calls) + if gotReq != r { + t.Fatalf("handler request: expected %p but got %p", r, gotReq) + } assert.Equal(t, "handler status", tc.wantStatus, gotStatus) assert.Equal(t, "handler err", tc.wantErr, gotErr.Error()) assert.ErrorIs(t, gotErr, err) From 0f9f7d7a27c872f97b0986668e7fe06924cc80d9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:41:31 +0000 Subject: [PATCH 08/12] test(accept_test.go): derive default responses from the rejection rows TestAcceptRejectionDefault repeated each row's header and body in a map keyed by row name, and a new row without a map entry failed with a header mismatch that did not name the missing key. The header and body now come from the row, with wantDefaultBody for the one row whose default body is not wantErr. --- accept_test.go | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/accept_test.go b/accept_test.go index 12dcffac..6af3edff 100644 --- a/accept_test.go +++ b/accept_test.go @@ -228,6 +228,9 @@ var acceptRejectionTests = []struct { wantErr string // wantHeader is the response header when the handler runs. wantHeader http.Header + // wantDefaultBody is the body without an ErrorHandler, if it differs + // from wantErr. + wantDefaultBody string }{ { name: "http1.0", @@ -304,10 +307,11 @@ var acceptRejectionTests = []struct { wantHeader: http.Header{}, }, { - name: "noHijacker", - wantStatus: http.StatusNotImplemented, - wantErr: "http.ResponseWriter does not implement http.Hijacker", - wantHeader: http.Header{}, + name: "noHijacker", + wantStatus: http.StatusNotImplemented, + wantErr: "http.ResponseWriter does not implement http.Hijacker", + wantHeader: http.Header{}, + wantDefaultBody: "Not Implemented", }, } @@ -434,27 +438,16 @@ func TestAcceptRejectionDefault(t *testing.T) { h.Set("X-Content-Type-Options", "nosniff") return h } - want := map[string]struct { - header http.Header - body string - }{ - "http1.0": {plain(http.Header{}), `WebSocket protocol violation: handshake request must be at least HTTP/1.1: "HTTP/1.0"` + "\n"}, - "noConnectionUpgrade": {plain(http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}), `WebSocket protocol violation: Connection header "keep-alive" does not contain Upgrade` + "\n"}, - "noUpgradeWebSocket": {plain(http.Header{"Connection": {"Upgrade"}, "Upgrade": {"websocket"}}), `WebSocket protocol violation: Upgrade header "h2c" does not contain websocket` + "\n"}, - "methodNotGet": {plain(http.Header{}), `WebSocket protocol violation: handshake request method is not GET but "POST"` + "\n"}, - "unsupportedVersion": {plain(http.Header{"Sec-Websocket-Version": {"13"}}), `unsupported WebSocket protocol version (only 13 is supported): "8"` + "\n"}, - "missingKey": {plain(http.Header{}), "WebSocket protocol violation: missing Sec-WebSocket-Key\n"}, - "multipleKeys": {plain(http.Header{}), "WebSocket protocol violation: multiple Sec-WebSocket-Key headers\n"}, - "invalidKey": {plain(http.Header{}), `WebSocket protocol violation: invalid Sec-WebSocket-Key "notbase64", must be a 16 byte base64 encoded string` + "\n"}, - "unauthorizedOrigin": {plain(http.Header{}), `request Origin "other.com" is not authorized for Host "example.com"` + "\n"}, - "badOriginPattern": {plain(http.Header{}), "Forbidden\n"}, - "noHijacker": {plain(http.Header{}), "Not Implemented\n"}, - } for _, tc := range acceptRejectionTests { t.Run(tc.name, func(t *testing.T) { t.Parallel() + wantBody := tc.wantErr + if tc.wantDefaultBody != "" { + wantBody = tc.wantDefaultBody + } + r := newHandshakeRequest() if tc.modify != nil { tc.modify(r) @@ -464,8 +457,8 @@ func TestAcceptRejectionDefault(t *testing.T) { _, err := Accept(w, r, &tc.opts) assert.Equal(t, "err", "failed to accept WebSocket connection: "+tc.wantErr, err.Error()) assert.Equal(t, "status", tc.wantStatus, w.Code) - assert.Equal(t, "header", want[tc.name].header, w.Header()) - assert.Equal(t, "body", want[tc.name].body, w.Body.String()) + assert.Equal(t, "header", plain(tc.wantHeader.Clone()), w.Header()) + assert.Equal(t, "body", wantBody+"\n", w.Body.String()) }) } } From 5cf85d05738d03671d515160e0a3386850bf2ff4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:42:11 +0000 Subject: [PATCH 09/12] test(accept_test.go): cover the Origin parse and no-host rejections acceptRejectionTests is documented as covering every rejection, but the Origin parse failure and the Origin without a host had no row, so their responses were not pinned. --- accept_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/accept_test.go b/accept_test.go index 6af3edff..2c74e5c2 100644 --- a/accept_test.go +++ b/accept_test.go @@ -290,6 +290,20 @@ var acceptRejectionTests = []struct { wantErr: `WebSocket protocol violation: invalid Sec-WebSocket-Key "notbase64", must be a 16 byte base64 encoded string`, wantHeader: http.Header{}, }, + { + name: "invalidOrigin", + modify: func(r *http.Request) { r.Header.Set("Origin", "http://[::1") }, + wantStatus: http.StatusForbidden, + wantErr: `failed to parse Origin header "http://[::1": parse "http://[::1": missing ']' in host`, + wantHeader: http.Header{}, + }, + { + name: "noOriginHost", + modify: func(r *http.Request) { r.Header.Set("Origin", "null") }, + wantStatus: http.StatusForbidden, + wantErr: `request Origin "null" is not a valid URL with a host`, + wantHeader: http.Header{}, + }, { name: "unauthorizedOrigin", modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, From d01f39d56e14206815748394cad2d15ad8694bb6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:56:54 +0000 Subject: [PATCH 10/12] docs(accept.go): say only that ErrorHandler err may hold server details The previous wording said err is not meant for the client, but Accept's default response sends err.Error() for every rejection except noHijacker. The doc now leaves the choice of what to send to the handler. --- accept.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/accept.go b/accept.go index 2a6b12c8..4db69e22 100644 --- a/accept.go +++ b/accept.go @@ -83,12 +83,12 @@ type AcceptOptions struct { // ErrorHandler writes the HTTP response when Accept rejects the handshake // request. status is the HTTP status Accept chose, w.Header() holds the // headers Accept set for the rejection, and Accept returns an error - // wrapping err. err is a diagnostic for logging that may contain server - // details, and is not meant to be sent to the client. ErrorHandler must - // write a response; otherwise net/http sends 200 OK with an empty body. If - // nil, Accept writes a plain text response with http.Error. ErrorHandler - // is not called when hijacking the connection fails, because Accept has - // already written 101 Switching Protocols. + // wrapping err. err may contain server details, so ErrorHandler decides + // what to send to the client. ErrorHandler must write a response; + // otherwise net/http sends 200 OK with an empty body. If nil, Accept + // writes a plain text response with http.Error. ErrorHandler is not called + // when hijacking the connection fails, because Accept has already written + // 101 Switching Protocols. ErrorHandler func(w http.ResponseWriter, r *http.Request, status int, err error) } From b10f4adfac16cc74801f7635d3beddef4e728f27 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:56:54 +0000 Subject: [PATCH 11/12] test(accept_test.go): restore why the server subtest uses a real server The earlier rewrite dropped the reason for testing over a real connection along with the part that restated the writesNothing row. Only the restated part needed to go. --- accept_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accept_test.go b/accept_test.go index 2c74e5c2..95e8eb06 100644 --- a/accept_test.go +++ b/accept_test.go @@ -392,7 +392,7 @@ func TestAcceptErrorHandler(t *testing.T) { assert.Equal(t, "handler calls", 0, calls) }) - // Checks the response a client receives from a real server. + // The handler response reaches a client over a real connection. t.Run("server", func(t *testing.T) { t.Parallel() From 56eccc2b10b0d9a811900ec7cda48fa96740b8ec Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 20:27:17 +0000 Subject: [PATCH 12/12] fix(accept): pass the OriginPatterns parse error to ErrorHandler Accept replaced an invalid OriginPatterns error with "Forbidden", so an ErrorHandler could not tell a misconfigured server from a disallowed origin. The real error now reaches ErrorHandler and Accept's caller, and the default response body stays "Forbidden". --- accept.go | 5 +++-- accept_test.go | 25 +++++++++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/accept.go b/accept.go index 4db69e22..4a86686a 100644 --- a/accept.go +++ b/accept.go @@ -130,11 +130,12 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con if !opts.InsecureSkipVerify { err = authenticateOrigin(r, opts.OriginPatterns) if err != nil { + defaultBody := err.Error() if errors.Is(err, path.ErrBadPattern) { log.Printf("websocket: %v", err) - err = errors.New(http.StatusText(http.StatusForbidden)) + defaultBody = http.StatusText(http.StatusForbidden) } - opts.writeRejection(w, r, http.StatusForbidden, err, err.Error()) + opts.writeRejection(w, r, http.StatusForbidden, err, defaultBody) return nil, err } } diff --git a/accept_test.go b/accept_test.go index 95e8eb06..746b9cf0 100644 --- a/accept_test.go +++ b/accept_test.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/httptest" + "path" "strings" "sync" "testing" @@ -226,6 +227,9 @@ var acceptRejectionTests = []struct { opts AcceptOptions wantStatus int wantErr string + // wantErrIs, if set, must be wrapped by the error Accept returns and the + // error ErrorHandler receives. + wantErrIs error // wantHeader is the response header when the handler runs. wantHeader http.Header // wantDefaultBody is the body without an ErrorHandler, if it differs @@ -313,12 +317,14 @@ var acceptRejectionTests = []struct { }, { // The pattern error is logged and not sent to the client. - name: "badOriginPattern", - modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, - opts: AcceptOptions{OriginPatterns: []string{"["}}, - wantStatus: http.StatusForbidden, - wantErr: "Forbidden", - wantHeader: http.Header{}, + name: "badOriginPattern", + modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, + opts: AcceptOptions{OriginPatterns: []string{"["}}, + wantStatus: http.StatusForbidden, + wantErr: `failed to parse path pattern "[": syntax error in pattern`, + wantErrIs: path.ErrBadPattern, + wantHeader: http.Header{}, + wantDefaultBody: "Forbidden", }, { name: "noHijacker", @@ -363,6 +369,10 @@ func TestAcceptErrorHandler(t *testing.T) { if gotReq != r { t.Fatalf("handler request: expected %p but got %p", r, gotReq) } + if tc.wantErrIs != nil { + assert.ErrorIs(t, tc.wantErrIs, err) + assert.ErrorIs(t, tc.wantErrIs, gotErr) + } assert.Equal(t, "handler status", tc.wantStatus, gotStatus) assert.Equal(t, "handler err", tc.wantErr, gotErr.Error()) assert.ErrorIs(t, gotErr, err) @@ -469,6 +479,9 @@ func TestAcceptRejectionDefault(t *testing.T) { w := httptest.NewRecorder() _, err := Accept(w, r, &tc.opts) + if tc.wantErrIs != nil { + assert.ErrorIs(t, tc.wantErrIs, err) + } assert.Equal(t, "err", "failed to accept WebSocket connection: "+tc.wantErr, err.Error()) assert.Equal(t, "status", tc.wantStatus, w.Code) assert.Equal(t, "header", plain(tc.wantHeader.Clone()), w.Header())