diff --git a/accept.go b/accept.go index 6339cbe3..acfb16dd 100644 --- a/accept.go +++ b/accept.go @@ -44,6 +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". + // 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. @@ -126,12 +130,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 a7d37a29..89fe691f 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("insecureSkipVerifyIgnoresInvalidPattern", 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 { @@ -320,11 +339,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", @@ -786,6 +805,26 @@ func Test_authenticateOrigin(t *testing.T) { }, success: true, }, + { + name: "invalidPatternNoOrigin", + host: "example.com", + originPatterns: []string{"["}, + success: true, + }, + { + name: "invalidPatternSameOrigin", + origin: "https://example.com", + host: "example.com", + originPatterns: []string{"["}, + success: true, + }, + { + name: "invalidPatternAfterMatch", + origin: "https://other.com", + host: "example.com", + originPatterns: []string{"other.com", "["}, + success: true, + }, } for _, tc := range testCases {