From 828c5d5edb4437184a2fe5bb3a431ed8343251cf Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 14:03:08 +0000 Subject: [PATCH 01/11] docs(accept.go): say how Accept reports an invalid OriginPatterns entry A handler or caller could tell an invalid pattern from a disallowed origin only through the path.ErrBadPattern wrap, which no doc promised. --- accept.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/accept.go b/accept.go index 6339cbe3..af6f235a 100644 --- a/accept.go +++ b/accept.go @@ -44,6 +44,9 @@ type AcceptOptions struct { // https://golang.org/pkg/path/#Match). By default, it is matched // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". + // Patterns are tried in order. Reaching an invalid pattern rejects a + // cross origin request with 403 Forbidden and an error wrapping + // path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. From 2d49f552bd2ae9d4c8fc36793aa3527e3e0cddee Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 16:27:29 +0000 Subject: [PATCH 02/11] fix: check every OriginPatterns entry on each handshake An invalid pattern rejected only cross origin requests that reached it before a match, so a misconfigured server failed for some clients and worked for others. Accept now checks every pattern first and rejects every handshake, whatever its origin. --- accept.go | 18 +++++++++++------- accept_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/accept.go b/accept.go index af6f235a..82283936 100644 --- a/accept.go +++ b/accept.go @@ -44,9 +44,8 @@ type AcceptOptions struct { // https://golang.org/pkg/path/#Match). By default, it is matched // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". - // Patterns are tried in order. Reaching an invalid pattern rejects a - // cross origin request with 403 Forbidden and an error wrapping - // path.ErrBadPattern. + // If a pattern is invalid, Accept rejects every handshake, whatever its + // origin, with 403 Forbidden and an error wrapping path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. @@ -248,6 +247,14 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ } func authenticateOrigin(r *http.Request, originHosts []string) error { + // Check every pattern first, so an invalid one rejects every request, + // whatever its origin. + for _, hostPattern := range originHosts { + if _, err := match(hostPattern, ""); err != nil { + return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) + } + } + origin := r.Header.Get("Origin") if origin == "" { return nil @@ -267,10 +274,7 @@ func authenticateOrigin(r *http.Request, originHosts []string) error { if strings.Contains(hostPattern, "://") { target = u.Scheme + "://" + u.Host } - matched, err := match(hostPattern, target) - if err != nil { - return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) - } + matched, _ := match(hostPattern, target) // Every pattern was checked above. if matched { return nil } diff --git a/accept_test.go b/accept_test.go index a7d37a29..a704afa5 100644 --- a/accept_test.go +++ b/accept_test.go @@ -700,6 +700,7 @@ func Test_authenticateOrigin(t *testing.T) { host string originPatterns []string success bool + wantErrIs error }{ { name: "none", @@ -786,6 +787,26 @@ func Test_authenticateOrigin(t *testing.T) { }, success: true, }, + { + name: "invalidPatternNoOrigin", + host: "example.com", + originPatterns: []string{"["}, + wantErrIs: path.ErrBadPattern, + }, + { + name: "invalidPatternSameOrigin", + origin: "https://example.com", + host: "example.com", + originPatterns: []string{"["}, + wantErrIs: path.ErrBadPattern, + }, + { + name: "invalidPatternAfterMatch", + origin: "https://other.com", + host: "example.com", + originPatterns: []string{"other.com", "["}, + wantErrIs: path.ErrBadPattern, + }, } for _, tc := range testCases { @@ -797,6 +818,9 @@ func Test_authenticateOrigin(t *testing.T) { r.Header.Set("Origin", tc.origin) err := authenticateOrigin(r, tc.originPatterns) + if tc.wantErrIs != nil { + assert.ErrorIs(t, tc.wantErrIs, err) + } if tc.success { assert.Success(t, err) } else { From 3f3b727deb74b0851fbb7b3cd6ec035863634f5a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 16:27:29 +0000 Subject: [PATCH 03/11] fix: answer an invalid OriginPatterns entry with 500 An invalid pattern now rejects every handshake, including same origin requests, so 403 Forbidden told those clients their origin was not allowed. The fault is the server's configuration. --- accept.go | 10 ++++++---- accept_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/accept.go b/accept.go index 82283936..cd400fa4 100644 --- a/accept.go +++ b/accept.go @@ -45,7 +45,8 @@ type AcceptOptions struct { // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". // If a pattern is invalid, Accept rejects every handshake, whatever its - // origin, with 403 Forbidden and an error wrapping path.ErrBadPattern. + // origin, with 500 Internal Server Error and an error wrapping + // path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. @@ -128,12 +129,13 @@ 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() + status, defaultBody := http.StatusForbidden, err.Error() if errors.Is(err, path.ErrBadPattern) { log.Printf("websocket: %v", err) - defaultBody = http.StatusText(http.StatusForbidden) + status = http.StatusInternalServerError + defaultBody = http.StatusText(status) } - opts.writeRejection(w, r, http.StatusForbidden, err, defaultBody) + opts.writeRejection(w, r, status, err, defaultBody) return nil, err } } diff --git a/accept_test.go b/accept_test.go index a704afa5..cc9ae8fe 100644 --- a/accept_test.go +++ b/accept_test.go @@ -320,11 +320,11 @@ var acceptRejectionTests = []struct { name: "badOriginPattern", modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, opts: AcceptOptions{OriginPatterns: []string{"["}}, - wantStatus: http.StatusForbidden, + wantStatus: http.StatusInternalServerError, wantErr: `invalid OriginPatterns entry "[": syntax error in pattern`, wantErrIs: path.ErrBadPattern, wantHeader: http.Header{}, - wantDefaultBody: "Forbidden", + wantDefaultBody: "Internal Server Error", }, { name: "noHijacker", From 32328789a07c2bae1fb80ea639ae89d2d04e2c07 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 16:29:21 +0000 Subject: [PATCH 04/11] docs(accept.go): leave InsecureSkipVerify out of the pattern rule Accept does not check the patterns when InsecureSkipVerify is set, so "every handshake" was false in that mode. --- accept.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/accept.go b/accept.go index cd400fa4..8c6e4b88 100644 --- a/accept.go +++ b/accept.go @@ -44,9 +44,9 @@ type AcceptOptions struct { // https://golang.org/pkg/path/#Match). By default, it is matched // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". - // If a pattern is invalid, Accept rejects every handshake, whatever its - // origin, with 500 Internal Server Error and an error wrapping - // path.ErrBadPattern. + // Unless InsecureSkipVerify is set, an invalid pattern makes Accept reject + // every handshake, whatever its origin, with 500 Internal Server Error and + // an error wrapping path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. From f2fb7dc648f0a0680691bb0af2bc02ec44ad9059 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:06:29 +0000 Subject: [PATCH 05/11] docs(accept.go): stop saying the request host is always authorized An invalid pattern now rejects same origin requests too. The comment on the pattern check also restated the loop instead of saying why it runs before the Origin checks. --- accept.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/accept.go b/accept.go index 8c6e4b88..c6e6165f 100644 --- a/accept.go +++ b/accept.go @@ -33,7 +33,7 @@ type AcceptOptions struct { InsecureSkipVerify bool // OriginPatterns lists the host patterns for authorized origins. - // The request host is always authorized. + // The request host is authorized without a matching pattern. // Use this to enable cross origin WebSockets. // // i.e javascript running on example.com wants to access a WebSocket server at chat.example.com. @@ -249,8 +249,8 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ } func authenticateOrigin(r *http.Request, originHosts []string) error { - // Check every pattern first, so an invalid one rejects every request, - // whatever its origin. + // This check comes before the Origin checks, so an invalid pattern rejects + // every request. for _, hostPattern := range originHosts { if _, err := match(hostPattern, ""); err != nil { return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) From 18b66a2338580b1c939af8f863d6301915623b7d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:06:29 +0000 Subject: [PATCH 06/11] test(accept_test.go): cover InsecureSkipVerify with an invalid pattern The OriginPatterns doc says InsecureSkipVerify skips the pattern check, and no test set both. The badOriginPattern row also no longer sets an Origin, which no longer affects the result, so the Accept level tests check a request without one. --- accept_test.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/accept_test.go b/accept_test.go index cc9ae8fe..675857fe 100644 --- a/accept_test.go +++ b/accept_test.go @@ -207,6 +207,25 @@ func TestAccept(t *testing.T) { wg.Wait() assert.Success(t, err) }) + + t.Run("insecureSkipVerifyIgnoresBadPattern", func(t *testing.T) { + t.Parallel() + + server, client := net.Pipe() + defer client.Close() + w := mockHijacker{ + ResponseWriter: httptest.NewRecorder(), + hijack: func() (net.Conn, *bufio.ReadWriter, error) { + return server, bufio.NewReadWriter(bufio.NewReader(server), bufio.NewWriter(server)), nil + }, + } + r := newHandshakeRequest() + r.Header.Set("Origin", "https://other.com") + + c, err := Accept(w, r, &AcceptOptions{InsecureSkipVerify: true, OriginPatterns: []string{"["}}) + assert.Success(t, err) + c.CloseNow() + }) } func newHandshakeRequest() *http.Request { @@ -318,7 +337,6 @@ var acceptRejectionTests = []struct { { // The pattern error is logged, and the default body omits it. name: "badOriginPattern", - modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, opts: AcceptOptions{OriginPatterns: []string{"["}}, wantStatus: http.StatusInternalServerError, wantErr: `invalid OriginPatterns entry "[": syntax error in pattern`, From fd4a902e0dfd5b9cba78964004e4bbddc126c5fe Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:11:06 +0000 Subject: [PATCH 07/11] docs(accept.go): drop "whatever its origin" from the pattern rule "Every handshake" already covers every origin. --- accept.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accept.go b/accept.go index c6e6165f..cbf8e51a 100644 --- a/accept.go +++ b/accept.go @@ -45,8 +45,8 @@ type AcceptOptions struct { // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". // Unless InsecureSkipVerify is set, an invalid pattern makes Accept reject - // every handshake, whatever its origin, with 500 Internal Server Error and - // an error wrapping path.ErrBadPattern. + // every handshake with 500 Internal Server Error and an error wrapping + // path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. From 6950739110c06e45339693f5e61ebc2f5396ec82 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 5 Nov 2026 12:46:19 +0000 Subject: [PATCH 08/11] test(accept_test.go): name the InsecureSkipVerify test "invalid" The doc, the error text and the Test_authenticateOrigin cases call the pattern invalid, not bad. --- accept_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accept_test.go b/accept_test.go index 675857fe..095c17f6 100644 --- a/accept_test.go +++ b/accept_test.go @@ -208,7 +208,7 @@ func TestAccept(t *testing.T) { assert.Success(t, err) }) - t.Run("insecureSkipVerifyIgnoresBadPattern", func(t *testing.T) { + t.Run("insecureSkipVerifyIgnoresInvalidPattern", func(t *testing.T) { t.Parallel() server, client := net.Pipe() From 3b4d4c1317ffd07d01fcf01a8ce27d61fe80de01 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 5 Nov 2026 12:49:26 +0000 Subject: [PATCH 09/11] docs(accept.go): limit the invalid pattern rule to valid requests A request that fails the protocol checks gets its own 4xx before the pattern check runs, so "every handshake" was too broad. --- accept.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accept.go b/accept.go index cbf8e51a..e7e1ad9b 100644 --- a/accept.go +++ b/accept.go @@ -45,8 +45,8 @@ type AcceptOptions struct { // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". // Unless InsecureSkipVerify is set, an invalid pattern makes Accept reject - // every handshake with 500 Internal Server Error and an error wrapping - // path.ErrBadPattern. + // every valid handshake request with 500 Internal Server Error and an + // error wrapping path.ErrBadPattern. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. From fa62356c7622ff4ad5564a97f6fb00033f147c15 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 5 Nov 2026 12:53:00 +0000 Subject: [PATCH 10/11] refactor: match each OriginPatterns entry once authenticateOrigin matched every pattern against "" to check it, then matched the patterns again against the origin. path.Match reports a malformed pattern whatever the name, so one loop now checks and matches each pattern and keeps the result. --- accept.go | 46 ++++++++++++++++++++++------------------------ accept_test.go | 7 +++++++ 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/accept.go b/accept.go index e7e1ad9b..e5ef0938 100644 --- a/accept.go +++ b/accept.go @@ -249,42 +249,40 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ } func authenticateOrigin(r *http.Request, originHosts []string) error { - // This check comes before the Origin checks, so an invalid pattern rejects - // every request. - for _, hostPattern := range originHosts { - if _, err := match(hostPattern, ""); err != nil { - return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) - } - } - origin := r.Header.Get("Origin") - if origin == "" { - return nil - } - - u, err := url.Parse(origin) - if err != nil { - return fmt.Errorf("failed to parse Origin header %q: %w", origin, err) - } - - if strings.EqualFold(r.Host, u.Host) { - return nil + u, parseErr := url.Parse(origin) + if parseErr != nil { + u = &url.URL{} } + // path.Match returns ErrBadPattern for a malformed pattern whatever the + // name. Checking here, before the Origin checks, makes an invalid pattern + // reject every request. + matched := false for _, hostPattern := range originHosts { target := u.Host if strings.Contains(hostPattern, "://") { target = u.Scheme + "://" + u.Host } - matched, _ := match(hostPattern, target) // Every pattern was checked above. - if matched { - return nil + ok, err := match(hostPattern, target) + if err != nil { + return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) } + matched = matched || ok } - if u.Host == "" { + + switch { + case origin == "": + return nil + case parseErr != nil: + return fmt.Errorf("failed to parse Origin header %q: %w", origin, parseErr) + case strings.EqualFold(r.Host, u.Host), matched: + return nil + case u.Host == "": return fmt.Errorf("request Origin %q is not a valid URL with a host", origin) + default: + return fmt.Errorf("request Origin %q is not authorized for Host %q", u.Host, r.Host) } - return fmt.Errorf("request Origin %q is not authorized for Host %q", u.Host, r.Host) } func match(pattern, s string) (bool, error) { diff --git a/accept_test.go b/accept_test.go index 095c17f6..d7a07fc7 100644 --- a/accept_test.go +++ b/accept_test.go @@ -825,6 +825,13 @@ func Test_authenticateOrigin(t *testing.T) { originPatterns: []string{"other.com", "["}, wantErrIs: path.ErrBadPattern, }, + { + name: "invalidPatternInvalidOrigin", + origin: "http://[::1", + host: "example.com", + originPatterns: []string{"["}, + wantErrIs: path.ErrBadPattern, + }, } for _, tc := range testCases { From 5e45c9c3943796e6bc039c27bde99843ce74286a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 5 Nov 2026 15:46:32 +0000 Subject: [PATCH 11/11] fix: try OriginPatterns entries in order again Checking every pattern on each handshake added work to same origin requests and requests without Origin, and a caller can check its patterns with path.Match before use. Accept now reports an invalid pattern only when it tries it, as before, still with 500, and the doc says how to find invalid patterns up front. --- accept.go | 46 ++++++++++++++++++++++------------------------ accept_test.go | 18 ++++-------------- 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/accept.go b/accept.go index e5ef0938..acfb16dd 100644 --- a/accept.go +++ b/accept.go @@ -33,7 +33,7 @@ type AcceptOptions struct { InsecureSkipVerify bool // OriginPatterns lists the host patterns for authorized origins. - // The request host is authorized without a matching pattern. + // The request host is always authorized. // Use this to enable cross origin WebSockets. // // i.e javascript running on example.com wants to access a WebSocket server at chat.example.com. @@ -44,9 +44,10 @@ type AcceptOptions struct { // https://golang.org/pkg/path/#Match). By default, it is matched // against the request origin host. If the pattern contains a URI // scheme ("://"), it will be matched against "scheme://host". - // Unless InsecureSkipVerify is set, an invalid pattern makes Accept reject - // every valid handshake request with 500 Internal Server Error and an - // error wrapping path.ErrBadPattern. + // Accept tries the patterns in order. When it tries an invalid pattern, + // it rejects the request with 500 Internal Server Error and an error + // wrapping path.ErrBadPattern. To find invalid patterns up front, check + // each with path.Match. // // Please ensure you understand the ramifications of enabling this. // If used incorrectly your WebSocket server will be open to CSRF attacks. @@ -250,39 +251,36 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ func authenticateOrigin(r *http.Request, originHosts []string) error { origin := r.Header.Get("Origin") - u, parseErr := url.Parse(origin) - if parseErr != nil { - u = &url.URL{} + if origin == "" { + return nil + } + + u, err := url.Parse(origin) + if err != nil { + return fmt.Errorf("failed to parse Origin header %q: %w", origin, err) + } + + if strings.EqualFold(r.Host, u.Host) { + return nil } - // path.Match returns ErrBadPattern for a malformed pattern whatever the - // name. Checking here, before the Origin checks, makes an invalid pattern - // reject every request. - matched := false for _, hostPattern := range originHosts { target := u.Host if strings.Contains(hostPattern, "://") { target = u.Scheme + "://" + u.Host } - ok, err := match(hostPattern, target) + matched, err := match(hostPattern, target) if err != nil { return fmt.Errorf("invalid OriginPatterns entry %q: %w", hostPattern, err) } - matched = matched || ok + if matched { + return nil + } } - - switch { - case origin == "": - return nil - case parseErr != nil: - return fmt.Errorf("failed to parse Origin header %q: %w", origin, parseErr) - case strings.EqualFold(r.Host, u.Host), matched: - return nil - case u.Host == "": + if u.Host == "" { return fmt.Errorf("request Origin %q is not a valid URL with a host", origin) - default: - return fmt.Errorf("request Origin %q is not authorized for Host %q", u.Host, r.Host) } + return fmt.Errorf("request Origin %q is not authorized for Host %q", u.Host, r.Host) } func match(pattern, s string) (bool, error) { diff --git a/accept_test.go b/accept_test.go index d7a07fc7..89fe691f 100644 --- a/accept_test.go +++ b/accept_test.go @@ -337,6 +337,7 @@ var acceptRejectionTests = []struct { { // The pattern error is logged, and the default body omits it. name: "badOriginPattern", + modify: func(r *http.Request) { r.Header.Set("Origin", "https://other.com") }, opts: AcceptOptions{OriginPatterns: []string{"["}}, wantStatus: http.StatusInternalServerError, wantErr: `invalid OriginPatterns entry "[": syntax error in pattern`, @@ -718,7 +719,6 @@ func Test_authenticateOrigin(t *testing.T) { host string originPatterns []string success bool - wantErrIs error }{ { name: "none", @@ -809,28 +809,21 @@ func Test_authenticateOrigin(t *testing.T) { name: "invalidPatternNoOrigin", host: "example.com", originPatterns: []string{"["}, - wantErrIs: path.ErrBadPattern, + success: true, }, { name: "invalidPatternSameOrigin", origin: "https://example.com", host: "example.com", originPatterns: []string{"["}, - wantErrIs: path.ErrBadPattern, + success: true, }, { name: "invalidPatternAfterMatch", origin: "https://other.com", host: "example.com", originPatterns: []string{"other.com", "["}, - wantErrIs: path.ErrBadPattern, - }, - { - name: "invalidPatternInvalidOrigin", - origin: "http://[::1", - host: "example.com", - originPatterns: []string{"["}, - wantErrIs: path.ErrBadPattern, + success: true, }, } @@ -843,9 +836,6 @@ func Test_authenticateOrigin(t *testing.T) { r.Header.Set("Origin", tc.origin) err := authenticateOrigin(r, tc.originPatterns) - if tc.wantErrIs != nil { - assert.ErrorIs(t, tc.wantErrIs, err) - } if tc.success { assert.Success(t, err) } else {