From 1223d271177923243e266d1ea9ede7372ae5ea99 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 14:50:22 +0000 Subject: [PATCH 01/19] fix(close): send fallback close frame for invalid codes Since db18a31, writeClose returned the marshal error before writing, so Close with an invalid code or reason sent no close frame and the peer saw an abnormal closure. Close now sends StatusInternalError, completes the handshake and still returns the error, as v1.8.7 to v1.8.10 did. --- close.go | 63 +++++++++++++++++++++++++++------------------------ close_test.go | 19 +++++++++++++++- conn_test.go | 8 ++++++- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/close.go b/close.go index fcc68065..10342756 100644 --- a/close.go +++ b/close.go @@ -93,6 +93,11 @@ func CloseStatus(err error) StatusCode { // are no-ops. // // The maximum length of reason must be 125 bytes. Avoid sending a dynamic reason. +// An invalid code or reason is replaced by StatusInternalError and reported as +// an error. +// +// Close always closes the connection. A non-nil error reports a problem +// during the close, not that the connection is still open. // // Close will unblock all goroutines interacting with the connection once // complete. @@ -106,13 +111,19 @@ func (c *Conn) Close(code StatusCode, reason string) (err error) { } return net.ErrClosed } + ce, wireErr := wireCloseError(code, reason) defer func() { if errors.Is(err, net.ErrClosed) { err = nil } + // Joined after the check above so it is not dropped along with a + // net.ErrClosed from the handshake. + if wireErr != nil { + err = errors.Join(wireErr, err) + } }() - err = c.closeHandshake(code, reason) + err = c.closeHandshake(ce.Code, ce.Reason) err2 := c.close() if err == nil && err2 != nil { @@ -168,24 +179,19 @@ func (c *Conn) closeHandshake(code StatusCode, reason string) error { } func (c *Conn) writeClose(code StatusCode, reason string) error { - ce := CloseError{ - Code: code, - Reason: reason, - } + // Close reports the wire error to the user. Internal callers only need + // a close frame to be sent. + ce, _ := wireCloseError(code, reason) var p []byte - var err error if ce.Code != StatusNoStatusRcvd { - p, err = ce.bytes() - if err != nil { - return err - } + p = ce.bytes() } ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - err = c.writeControl(ctx, opClose, p) + err := c.writeControl(ctx, opClose, p) // If the connection closed as we're writing we ignore the error as we might // have written the close frame, the peer responded and then someone else read it // and closed the connection. @@ -292,33 +298,30 @@ func validWireCloseCode(code StatusCode) bool { return false } -func (ce CloseError) bytes() ([]byte, error) { - p, err := ce.bytesErr() - if err != nil { - err = fmt.Errorf("failed to marshal close frame: %w", err) - ce = CloseError{ - Code: StatusInternalError, - } - p, _ = ce.bytesErr() +// wireCloseError returns the CloseError to send for code and reason. A marshal +// problem must not prevent sending a close frame, so an invalid code or reason +// is replaced by StatusInternalError. The error reports the change. +func wireCloseError(code StatusCode, reason string) (CloseError, error) { + if code == StatusNoStatusRcvd { + return CloseError{Code: code}, nil // Sent as an empty close frame. + } + if len(reason) > maxCloseReason { + return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: reason string max is %v but got %q with length %v", maxCloseReason, reason, len(reason)) } - return p, err + if !validWireCloseCode(code) { + return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: status code %v cannot be set", code) + } + return CloseError{Code: code, Reason: reason}, nil } const maxCloseReason = maxControlPayload - 2 -func (ce CloseError) bytesErr() ([]byte, error) { - if len(ce.Reason) > maxCloseReason { - return nil, fmt.Errorf("reason string max is %v but got %q with length %v", maxCloseReason, ce.Reason, len(ce.Reason)) - } - - if !validWireCloseCode(ce.Code) { - return nil, fmt.Errorf("status code %v cannot be set", ce.Code) - } - +// bytes encodes ce as a close frame payload. ce must come from wireCloseError. +func (ce CloseError) bytes() []byte { buf := make([]byte, 2+len(ce.Reason)) binary.BigEndian.PutUint16(buf, uint16(ce.Code)) copy(buf[2:], ce.Reason) - return buf, nil + return buf } func (c *Conn) casClosing() bool { diff --git a/close_test.go b/close_test.go index 1e04807e..5b53a79d 100644 --- a/close_test.go +++ b/close_test.go @@ -17,6 +17,7 @@ func TestCloseError(t *testing.T) { testCases := []struct { name string ce CloseError + want CloseError success bool }{ { @@ -25,6 +26,10 @@ func TestCloseError(t *testing.T) { Code: StatusNormalClosure, Reason: strings.Repeat("x", maxCloseReason), }, + want: CloseError{ + Code: StatusNormalClosure, + Reason: strings.Repeat("x", maxCloseReason), + }, success: true, }, { @@ -33,6 +38,7 @@ func TestCloseError(t *testing.T) { Code: StatusNormalClosure, Reason: strings.Repeat("x", maxCloseReason+1), }, + want: CloseError{Code: StatusInternalError}, success: false, }, { @@ -41,8 +47,18 @@ func TestCloseError(t *testing.T) { Code: math.MaxUint16, Reason: strings.Repeat("x", maxCloseReason), }, + want: CloseError{Code: StatusInternalError}, success: false, }, + { + name: "noStatus", + ce: CloseError{ + Code: StatusNoStatusRcvd, + Reason: "dropped", + }, + want: CloseError{Code: StatusNoStatusRcvd}, + success: true, + }, } for _, tc := range testCases { @@ -50,12 +66,13 @@ func TestCloseError(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := tc.ce.bytesErr() + ce, err := wireCloseError(tc.ce.Code, tc.ce.Reason) if tc.success { assert.Success(t, err) } else { assert.Error(t, err) } + assert.Equal(t, "wire close error", tc.want, ce) }) } diff --git a/conn_test.go b/conn_test.go index 01af8cd4..0100c6c2 100644 --- a/conn_test.go +++ b/conn_test.go @@ -64,10 +64,16 @@ func TestConn(t *testing.T) { t.Run("badClose", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) - c2.CloseRead(tt.ctx) + readErr := xsync.Go(func() error { + _, _, err := c2.Read(tt.ctx) + return err + }) err := c1.Close(-1, "") assert.Contains(t, err, "failed to marshal close frame: status code StatusCode(-1) cannot be set") + + // The peer still gets a close frame. + assert.Equal(t, "peer close status", websocket.StatusInternalError, websocket.CloseStatus(<-readErr)) }) t.Run("ping", func(t *testing.T) { From 5427a1f8c76bc2bc87a9516feb5d79390b29d12b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 14:52:20 +0000 Subject: [PATCH 02/19] feat(close): truncate close reasons over 123 bytes A reason over the 123 bytes a close frame can carry made Close replace the caller's code with StatusInternalError and drop the reason, so the peer lost both. Close now sends the code with the longest prefix that does not split a UTF-8 rune and reports ErrCloseReasonTruncated. --- close.go | 20 ++++++++--------- close_test.go | 46 ++++++++++++++++++++++++++++++++++++++- conn_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ errors.go | 32 +++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 12 deletions(-) diff --git a/close.go b/close.go index 10342756..ff17f71d 100644 --- a/close.go +++ b/close.go @@ -92,9 +92,10 @@ func CloseStatus(err error) StatusCode { // The connection can only be closed once. Additional calls to Close // are no-ops. // -// The maximum length of reason must be 125 bytes. Avoid sending a dynamic reason. -// An invalid code or reason is replaced by StatusInternalError and reported as -// an error. +// The reason is limited to 123 bytes. A longer reason is cut to the longest +// prefix that does not split a UTF-8 encoded rune, the handshake proceeds, and +// Close returns an error wrapping ErrCloseReasonTruncated. An invalid code is +// replaced by StatusInternalError and reported as an error. // // Close always closes the connection. A non-nil error reports a problem // during the close, not that the connection is still open. @@ -299,23 +300,20 @@ func validWireCloseCode(code StatusCode) bool { } // wireCloseError returns the CloseError to send for code and reason. A marshal -// problem must not prevent sending a close frame, so an invalid code or reason -// is replaced by StatusInternalError. The error reports the change. +// problem must not prevent sending a close frame, so an invalid code is +// replaced by StatusInternalError and a long reason is truncated. The error +// reports either change. func wireCloseError(code StatusCode, reason string) (CloseError, error) { if code == StatusNoStatusRcvd { return CloseError{Code: code}, nil // Sent as an empty close frame. } - if len(reason) > maxCloseReason { - return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: reason string max is %v but got %q with length %v", maxCloseReason, reason, len(reason)) - } if !validWireCloseCode(code) { return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: status code %v cannot be set", code) } - return CloseError{Code: code, Reason: reason}, nil + reason, err := truncateCloseReason(reason) + return CloseError{Code: code, Reason: reason}, err } -const maxCloseReason = maxControlPayload - 2 - // bytes encodes ce as a close frame payload. ce must come from wireCloseError. func (ce CloseError) bytes() []byte { buf := make([]byte, 2+len(ce.Reason)) diff --git a/close_test.go b/close_test.go index 5b53a79d..1f1b7b9b 100644 --- a/close_test.go +++ b/close_test.go @@ -7,6 +7,7 @@ import ( "math" "strings" "testing" + "unicode/utf8" "github.com/coder/websocket/internal/test/assert" ) @@ -38,7 +39,10 @@ func TestCloseError(t *testing.T) { Code: StatusNormalClosure, Reason: strings.Repeat("x", maxCloseReason+1), }, - want: CloseError{Code: StatusInternalError}, + want: CloseError{ + Code: StatusNormalClosure, + Reason: strings.Repeat("x", maxCloseReason), + }, success: false, }, { @@ -86,6 +90,46 @@ func TestCloseError(t *testing.T) { }) } +func Test_truncateCloseReason(t *testing.T) { + t.Parallel() + + ascii := strings.Repeat("x", maxCloseReason) + testCases := []struct { + name string + reason string + want string + }{ + {name: "empty", reason: "", want: ""}, + {name: "max", reason: ascii, want: ascii}, + {name: "maxPlusOne", reason: ascii + "x", want: ascii}, + {name: "twoByteFits", reason: ascii[:121] + "é", want: ascii[:121] + "é"}, + {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122]}, + {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41)}, + {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122]}, + {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30)}, + // Invalid UTF-8 is cut at the limit, not scanned for a boundary. + {name: "continuationBytes", reason: strings.Repeat("\x80", 200), want: strings.Repeat("\x80", maxCloseReason)}, + {name: "incompleteRune", reason: ascii[:122] + "\xe2\x82x", want: ascii[:122] + "\xe2"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := truncateCloseReason(tc.reason) + assert.Equal(t, "reason", tc.want, got) + if got == tc.reason { + assert.Success(t, err) + } else { + assert.ErrorIs(t, ErrCloseReasonTruncated, err) + } + if utf8.ValidString(tc.reason) && !utf8.ValidString(got) { + t.Fatalf("truncated valid UTF-8 into invalid UTF-8: %q", got) + } + }) + } +} + func Test_parseClosePayload(t *testing.T) { t.Parallel() diff --git a/conn_test.go b/conn_test.go index 0100c6c2..1d91aedb 100644 --- a/conn_test.go +++ b/conn_test.go @@ -76,6 +76,66 @@ func TestConn(t *testing.T) { assert.Equal(t, "peer close status", websocket.StatusInternalError, websocket.CloseStatus(<-readErr)) }) + t.Run("closeReason", func(t *testing.T) { + t.Parallel() + + const code websocket.StatusCode = 4000 + ascii := strings.Repeat("x", 123) + testCases := []struct { + name string + reason string + want string + }{ + {name: "max", reason: ascii, want: ascii}, + {name: "ascii", reason: ascii + "xyz", want: ascii}, + {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122]}, + {name: "fourByteSplit", reason: strings.Repeat("😀", 40), want: strings.Repeat("😀", 30)}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + readErr := xsync.Go(func() error { + _, _, err := c2.Read(tt.ctx) + return err + }) + + err := c1.Close(code, tc.reason) + if tc.want == tc.reason { + assert.Success(t, err) + } else { + assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) + // The handshake succeeded, so nothing else is reported. + assert.Equal(t, "close error", fmt.Sprintf("failed to close WebSocket: websocket: close reason truncated: sent %d of %d bytes", len(tc.want), len(tc.reason)), err.Error()) + } + + var ce websocket.CloseError + if !errors.As(<-readErr, &ce) { + t.Fatal("peer did not receive a close frame") + } + assert.Equal(t, "peer close error", websocket.CloseError{Code: code, Reason: tc.want}, ce) + }) + } + }) + + // The CloseRead goroutine may read the echoed close frame and close the + // connection, failing the handshake with net.ErrClosed, which Close + // ignores. The truncation must still be reported. + t.Run("closeReasonTruncatedWithCloseRead", func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + _ = c1.CloseRead(tt.ctx) + readErr := xsync.Go(func() error { + _, _, err := c2.Read(tt.ctx) + return err + }) + + err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) + assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) + assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated: sent 123 of 124 bytes", err.Error()) + assert.Equal(t, "peer close status", websocket.StatusNormalClosure, websocket.CloseStatus(<-readErr)) + }) + t.Run("ping", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) diff --git a/errors.go b/errors.go index bf4fc2b0..d8e1e133 100644 --- a/errors.go +++ b/errors.go @@ -2,7 +2,39 @@ package websocket import ( "errors" + "fmt" + "unicode/utf8" ) // ErrMessageTooBig is returned when a message exceeds the read limit. var ErrMessageTooBig = errors.New("websocket: message too big") + +// ErrCloseReasonTruncated is returned by Close when the reason did not fit in +// a close frame and a truncated reason was sent instead. +var ErrCloseReasonTruncated = errors.New("websocket: close reason truncated") + +// maxCloseReason is the 125 byte control frame payload limit minus the 2 byte +// status code, see RFC 6455 section 5.5. +const maxCloseReason = 123 + +// truncateCloseReason returns the longest prefix of reason that fits in a +// close frame without splitting a valid UTF-8 encoded rune. The error wraps +// ErrCloseReasonTruncated if reason was cut. +func truncateCloseReason(reason string) (string, error) { + if len(reason) <= maxCloseReason { + return reason, nil + } + n := maxCloseReason + // Back off only to keep a valid multi-byte rune whole. Invalid UTF-8 is + // cut as is because the library does not validate it. + for i := n; i > n-utf8.UTFMax; i-- { + if utf8.RuneStart(reason[i]) { + r, size := utf8.DecodeRuneInString(reason[i:]) + if (r != utf8.RuneError || size > 1) && i+size > n { + n = i + } + break + } + } + return reason[:n], fmt.Errorf("%w: sent %d of %d bytes", ErrCloseReasonTruncated, n, len(reason)) +} From 550d8cf4e648b696f93c30783beac349980c2c3b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 14:53:20 +0000 Subject: [PATCH 03/19] fix(wasm): truncate close reasons over 123 bytes The browser's close() throws for a reason over 123 UTF-8 bytes before changing any state, so Close returned an error and left the connection open. Close now truncates the reason as the native implementation does and records the reason it actually sent. --- ws_js.go | 10 ++++++++-- ws_js_test.go | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/ws_js.go b/ws_js.go index 026b75fc..3f9bc6a9 100644 --- a/ws_js.go +++ b/ws_js.go @@ -228,6 +228,9 @@ func (c *Conn) write(typ MessageType, p []byte) error { // It will wait until the peer responds with a close frame // or the connection is closed. // It thus performs the full WebSocket close handshake. +// +// A reason over 123 bytes is truncated, and Close returns an error wrapping +// ErrCloseReasonTruncated. func (c *Conn) Close(code StatusCode, reason string) error { err := c.exportedClose(code, reason) if err != nil { @@ -246,6 +249,9 @@ func (c *Conn) CloseNow() error { } func (c *Conn) exportedClose(code StatusCode, reason string) error { + // Browsers throw on a reason over 123 bytes and leave the connection open. + reason, truncErr := truncateCloseReason(reason) + c.closingMu.Lock() defer c.closingMu.Unlock() @@ -266,9 +272,9 @@ func (c *Conn) exportedClose(code StatusCode, reason string) error { <-c.closed if !c.closeWasClean { - return c.closeErr + return errors.Join(truncErr, c.closeErr) } - return nil + return truncErr } // Subprotocol returns the negotiated subprotocol. diff --git a/ws_js_test.go b/ws_js_test.go index 1fa242f4..e9c87b1f 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "os" + "strings" "testing" "time" @@ -37,6 +38,24 @@ func TestWasm(t *testing.T) { assert.Success(t, err) } +func TestWasmCloseReasonTruncated(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + c, _, err := websocket.Dial(ctx, os.Getenv("WS_ECHO_SERVER_URL"), &websocket.DialOptions{ + Subprotocols: []string{"echo"}, + }) + assert.Success(t, err) + + // The browser throws on reasons over 123 bytes, so a clean close shows + // the reason was truncated. The echo server checks the code. + err = c.Close(websocket.StatusNormalClosure, strings.Repeat("é", 62)) + assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) + assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated: sent 122 of 124 bytes", err.Error()) +} + func TestWasmDialTimeout(t *testing.T) { t.Parallel() From db23beaeb476e2a57bf62a1d14acc648bbf2ec44 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 15:25:49 +0000 Subject: [PATCH 04/19] docs(close): limit the Close guarantee to the first call The previous wording promised that Close always closes the connection, but a concurrent Close can return after waitGoroutines times out while the first call is still blocked in the handshake. Only the first call is guaranteed to return with the connection closed. --- close.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/close.go b/close.go index ff17f71d..04f5a436 100644 --- a/close.go +++ b/close.go @@ -97,8 +97,8 @@ func CloseStatus(err error) StatusCode { // Close returns an error wrapping ErrCloseReasonTruncated. An invalid code is // replaced by StatusInternalError and reported as an error. // -// Close always closes the connection. A non-nil error reports a problem -// during the close, not that the connection is still open. +// When the first call to Close returns, the connection is closed, even if +// Close returns an error. A non-nil error reports a problem during the close. // // Close will unblock all goroutines interacting with the connection once // complete. From f7d9efa9df5823cbbf3722ce03ce8706a4fc2c6a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 15:26:07 +0000 Subject: [PATCH 05/19] fix: stop the truncation error from claiming bytes were sent ErrCloseReasonTruncated is reported even when writing the close frame fails, so its doc and the "sent N of M bytes" detail could be false. Both now describe only the shortening of the reason. --- conn_test.go | 4 ++-- errors.go | 6 +++--- ws_js_test.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/conn_test.go b/conn_test.go index 1d91aedb..13946cbf 100644 --- a/conn_test.go +++ b/conn_test.go @@ -106,7 +106,7 @@ func TestConn(t *testing.T) { } else { assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) // The handshake succeeded, so nothing else is reported. - assert.Equal(t, "close error", fmt.Sprintf("failed to close WebSocket: websocket: close reason truncated: sent %d of %d bytes", len(tc.want), len(tc.reason)), err.Error()) + assert.Equal(t, "close error", fmt.Sprintf("failed to close WebSocket: websocket: close reason truncated from %d to %d bytes", len(tc.reason), len(tc.want)), err.Error()) } var ce websocket.CloseError @@ -132,7 +132,7 @@ func TestConn(t *testing.T) { err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated: sent 123 of 124 bytes", err.Error()) + assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 123 bytes", err.Error()) assert.Equal(t, "peer close status", websocket.StatusNormalClosure, websocket.CloseStatus(<-readErr)) }) diff --git a/errors.go b/errors.go index d8e1e133..8a149fe7 100644 --- a/errors.go +++ b/errors.go @@ -9,8 +9,8 @@ import ( // ErrMessageTooBig is returned when a message exceeds the read limit. var ErrMessageTooBig = errors.New("websocket: message too big") -// ErrCloseReasonTruncated is returned by Close when the reason did not fit in -// a close frame and a truncated reason was sent instead. +// ErrCloseReasonTruncated is returned by Close when the reason was longer than +// 123 bytes and Close shortened it. var ErrCloseReasonTruncated = errors.New("websocket: close reason truncated") // maxCloseReason is the 125 byte control frame payload limit minus the 2 byte @@ -36,5 +36,5 @@ func truncateCloseReason(reason string) (string, error) { break } } - return reason[:n], fmt.Errorf("%w: sent %d of %d bytes", ErrCloseReasonTruncated, n, len(reason)) + return reason[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(reason), n) } diff --git a/ws_js_test.go b/ws_js_test.go index e9c87b1f..044d58cb 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -53,7 +53,7 @@ func TestWasmCloseReasonTruncated(t *testing.T) { // the reason was truncated. The echo server checks the code. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("é", 62)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated: sent 122 of 124 bytes", err.Error()) + assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 122 bytes", err.Error()) } func TestWasmDialTimeout(t *testing.T) { From fd190bf82627e45613278ee9657ab67f81f25789 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 15:26:38 +0000 Subject: [PATCH 06/19] docs(ws_js.go): note that invalid UTF-8 reasons can still be rejected Go strings become JavaScript strings with invalid bytes replaced by U+FFFD, so a 50 byte reason of invalid UTF-8 already exceeds the browser's 123 byte limit. Truncation only guarantees a fit for valid UTF-8, and the library does not validate reasons. --- ws_js.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ws_js.go b/ws_js.go index 3f9bc6a9..2f6c3fa5 100644 --- a/ws_js.go +++ b/ws_js.go @@ -230,7 +230,9 @@ func (c *Conn) write(typ MessageType, p []byte) error { // It thus performs the full WebSocket close handshake. // // A reason over 123 bytes is truncated, and Close returns an error wrapping -// ErrCloseReasonTruncated. +// ErrCloseReasonTruncated. Truncation only makes valid UTF-8 fit: invalid +// bytes grow when converted to a JavaScript string, so the browser may still +// reject the reason. func (c *Conn) Close(code StatusCode, reason string) error { err := c.exportedClose(code, reason) if err != nil { From 052f77c859ad6a146cd756f63fc6e2da59636cea Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 15:27:01 +0000 Subject: [PATCH 07/19] test: cover U+FFFD at the limit and truncation with a write failure U+FFFD decodes as utf8.RuneError with size 3, so a check on the rune alone would split it. Close must also report the transport error alongside ErrCloseReasonTruncated when the close frame write fails. --- close_test.go | 2 ++ conn_test.go | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/close_test.go b/close_test.go index 1f1b7b9b..610e203a 100644 --- a/close_test.go +++ b/close_test.go @@ -107,6 +107,8 @@ func Test_truncateCloseReason(t *testing.T) { {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41)}, {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122]}, {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30)}, + // U+FFFD decodes as utf8.RuneError but is a valid 3 byte rune. + {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122]}, // Invalid UTF-8 is cut at the limit, not scanned for a boundary. {name: "continuationBytes", reason: strings.Repeat("\x80", 200), want: strings.Repeat("\x80", maxCloseReason)}, {name: "incompleteRune", reason: ascii[:122] + "\xe2\x82x", want: ascii[:122] + "\xe2"}, diff --git a/conn_test.go b/conn_test.go index 13946cbf..7d8e9d43 100644 --- a/conn_test.go +++ b/conn_test.go @@ -136,6 +136,17 @@ func TestConn(t *testing.T) { assert.Equal(t, "peer close status", websocket.StatusNormalClosure, websocket.CloseStatus(<-readErr)) }) + t.Run("closeReasonTruncatedWithWriteError", func(t *testing.T) { + _, c1, c2 := newConnTest(t, nil, nil) + + // Closing the peer's end of the pipe makes writing the close frame fail. + assert.Success(t, c2.CloseNow()) + + err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) + assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) + assert.ErrorIs(t, io.ErrClosedPipe, err) + }) + t.Run("ping", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) From 3c08db6628395a2e274b9d4481ab82e75d58dbd2 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 16:17:08 +0000 Subject: [PATCH 08/19] fix: replace invalid UTF-8 in close reasons with U+FFFD RFC 6455 defines the close reason as UTF-8, and peers that check it fail the connection instead of reading the code. On js/wasm, invalid bytes also grow when converted to a JavaScript string, so the browser threw for reasons that fit. Truncation now applies to the replaced reason, which can be longer than the caller's. --- close.go | 15 ++++++++------- close_test.go | 40 +++++++++++++++++++++++----------------- conn_test.go | 16 +++++++++------- errors.go | 37 ++++++++++++++++++------------------- ws_js.go | 2 +- ws_js_test.go | 18 ++++++++++++++++++ 6 files changed, 77 insertions(+), 51 deletions(-) diff --git a/close.go b/close.go index 04f5a436..4e79ff5d 100644 --- a/close.go +++ b/close.go @@ -92,10 +92,11 @@ func CloseStatus(err error) StatusCode { // The connection can only be closed once. Additional calls to Close // are no-ops. // -// The reason is limited to 123 bytes. A longer reason is cut to the longest -// prefix that does not split a UTF-8 encoded rune, the handshake proceeds, and -// Close returns an error wrapping ErrCloseReasonTruncated. An invalid code is -// replaced by StatusInternalError and reported as an error. +// The reason is limited to 123 bytes. Invalid UTF-8 in the reason is replaced +// with U+FFFD. A longer reason is then cut to the longest prefix that does not +// split a UTF-8 encoded rune, the handshake proceeds, and Close returns an +// error wrapping ErrCloseReasonTruncated. An invalid code is replaced by +// StatusInternalError and reported as an error. // // When the first call to Close returns, the connection is closed, even if // Close returns an error. A non-nil error reports a problem during the close. @@ -301,8 +302,8 @@ func validWireCloseCode(code StatusCode) bool { // wireCloseError returns the CloseError to send for code and reason. A marshal // problem must not prevent sending a close frame, so an invalid code is -// replaced by StatusInternalError and a long reason is truncated. The error -// reports either change. +// replaced by StatusInternalError and the reason goes through wireCloseReason. +// The error reports a replaced code or a truncated reason. func wireCloseError(code StatusCode, reason string) (CloseError, error) { if code == StatusNoStatusRcvd { return CloseError{Code: code}, nil // Sent as an empty close frame. @@ -310,7 +311,7 @@ func wireCloseError(code StatusCode, reason string) (CloseError, error) { if !validWireCloseCode(code) { return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: status code %v cannot be set", code) } - reason, err := truncateCloseReason(reason) + reason, err := wireCloseReason(reason) return CloseError{Code: code, Reason: reason}, err } diff --git a/close_test.go b/close_test.go index 610e203a..39c3c4b5 100644 --- a/close_test.go +++ b/close_test.go @@ -90,43 +90,49 @@ func TestCloseError(t *testing.T) { }) } -func Test_truncateCloseReason(t *testing.T) { +func Test_wireCloseReason(t *testing.T) { t.Parallel() ascii := strings.Repeat("x", maxCloseReason) testCases := []struct { - name string - reason string - want string + name string + reason string + want string + truncated string // Expected error message, empty if not truncated. }{ {name: "empty", reason: "", want: ""}, {name: "max", reason: ascii, want: ascii}, - {name: "maxPlusOne", reason: ascii + "x", want: ascii}, + {name: "maxPlusOne", reason: ascii + "x", want: ascii, truncated: "from 124 to 123 bytes"}, {name: "twoByteFits", reason: ascii[:121] + "é", want: ascii[:121] + "é"}, - {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122]}, - {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41)}, - {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122]}, - {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30)}, + {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122], truncated: "from 124 to 122 bytes"}, + {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41), truncated: "from 126 to 123 bytes"}, + {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122], truncated: "from 125 to 122 bytes"}, + {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30), truncated: "from 124 to 120 bytes"}, // U+FFFD decodes as utf8.RuneError but is a valid 3 byte rune. - {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122]}, - // Invalid UTF-8 is cut at the limit, not scanned for a boundary. - {name: "continuationBytes", reason: strings.Repeat("\x80", 200), want: strings.Repeat("\x80", maxCloseReason)}, - {name: "incompleteRune", reason: ascii[:122] + "\xe2\x82x", want: ascii[:122] + "\xe2"}, + {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122], truncated: "from 125 to 122 bytes"}, + {name: "invalidByte", reason: "a\x80b", want: "a\uFFFDb"}, + {name: "invalidRun", reason: "a\x80\x80\xffb", want: "a\uFFFDb"}, + {name: "incompleteRune", reason: "a\xe2\x82", want: "a\uFFFD"}, + {name: "invalidRunOverMax", reason: strings.Repeat("\x80", 200), want: "\uFFFD"}, + // Replacement grows these past maxCloseReason. + {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], truncated: "from 123 to 122 bytes"}, + {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", truncated: "from 122 to 123 bytes"}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, err := truncateCloseReason(tc.reason) + got, err := wireCloseReason(tc.reason) assert.Equal(t, "reason", tc.want, got) - if got == tc.reason { + if tc.truncated == "" { assert.Success(t, err) } else { assert.ErrorIs(t, ErrCloseReasonTruncated, err) + assert.Equal(t, "error", "websocket: close reason truncated "+tc.truncated, err.Error()) } - if utf8.ValidString(tc.reason) && !utf8.ValidString(got) { - t.Fatalf("truncated valid UTF-8 into invalid UTF-8: %q", got) + if !utf8.ValidString(got) || len(got) > maxCloseReason { + t.Fatalf("reason %q is not valid UTF-8 within %d bytes", got, maxCloseReason) } }) } diff --git a/conn_test.go b/conn_test.go index 7d8e9d43..5f555ff5 100644 --- a/conn_test.go +++ b/conn_test.go @@ -82,14 +82,16 @@ func TestConn(t *testing.T) { const code websocket.StatusCode = 4000 ascii := strings.Repeat("x", 123) testCases := []struct { - name string - reason string - want string + name string + reason string + want string + truncated bool }{ {name: "max", reason: ascii, want: ascii}, - {name: "ascii", reason: ascii + "xyz", want: ascii}, - {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122]}, - {name: "fourByteSplit", reason: strings.Repeat("😀", 40), want: strings.Repeat("😀", 30)}, + {name: "ascii", reason: ascii + "xyz", want: ascii, truncated: true}, + {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122], truncated: true}, + {name: "fourByteSplit", reason: strings.Repeat("😀", 40), want: strings.Repeat("😀", 30), truncated: true}, + {name: "invalidUTF8", reason: "bad \xff\xfe reason", want: "bad \uFFFD reason"}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -101,7 +103,7 @@ func TestConn(t *testing.T) { }) err := c1.Close(code, tc.reason) - if tc.want == tc.reason { + if !tc.truncated { assert.Success(t, err) } else { assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) diff --git a/errors.go b/errors.go index 8a149fe7..dc107540 100644 --- a/errors.go +++ b/errors.go @@ -3,38 +3,37 @@ package websocket import ( "errors" "fmt" + "strings" "unicode/utf8" ) // ErrMessageTooBig is returned when a message exceeds the read limit. var ErrMessageTooBig = errors.New("websocket: message too big") -// ErrCloseReasonTruncated is returned by Close when the reason was longer than -// 123 bytes and Close shortened it. +// ErrCloseReasonTruncated is returned by Close when the reason, after invalid +// UTF-8 was replaced, was longer than 123 bytes and Close shortened it. var ErrCloseReasonTruncated = errors.New("websocket: close reason truncated") // maxCloseReason is the 125 byte control frame payload limit minus the 2 byte // status code, see RFC 6455 section 5.5. const maxCloseReason = 123 -// truncateCloseReason returns the longest prefix of reason that fits in a -// close frame without splitting a valid UTF-8 encoded rune. The error wraps -// ErrCloseReasonTruncated if reason was cut. -func truncateCloseReason(reason string) (string, error) { - if len(reason) <= maxCloseReason { - return reason, nil +// wireCloseReason returns reason as Close sends it: invalid UTF-8 replaced +// with U+FFFD, then cut to the longest prefix of at most maxCloseReason bytes +// that does not split a rune. The error wraps ErrCloseReasonTruncated if the +// reason was cut. +func wireCloseReason(reason string) (string, error) { + // RFC 6455 section 5.5.1 defines the reason as UTF-8, and peers that + // check it fail the connection (section 8.1) instead of reading the code. + valid := strings.ToValidUTF8(reason, "\uFFFD") + if len(valid) <= maxCloseReason { + return valid, nil } n := maxCloseReason - // Back off only to keep a valid multi-byte rune whole. Invalid UTF-8 is - // cut as is because the library does not validate it. - for i := n; i > n-utf8.UTFMax; i-- { - if utf8.RuneStart(reason[i]) { - r, size := utf8.DecodeRuneInString(reason[i:]) - if (r != utf8.RuneError || size > 1) && i+size > n { - n = i - } - break - } + for !utf8.RuneStart(valid[n]) { + n-- } - return reason[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(reason), n) + // "from" is the caller's length and "to" the sent length. Replacement can + // grow a reason, so "from" can be maxCloseReason or less, even below "to". + return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(reason), n) } diff --git a/ws_js.go b/ws_js.go index 2f6c3fa5..7c49bf7b 100644 --- a/ws_js.go +++ b/ws_js.go @@ -252,7 +252,7 @@ func (c *Conn) CloseNow() error { func (c *Conn) exportedClose(code StatusCode, reason string) error { // Browsers throw on a reason over 123 bytes and leave the connection open. - reason, truncErr := truncateCloseReason(reason) + reason, truncErr := wireCloseReason(reason) c.closingMu.Lock() defer c.closingMu.Unlock() diff --git a/ws_js_test.go b/ws_js_test.go index 044d58cb..2595dc67 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -56,6 +56,24 @@ func TestWasmCloseReasonTruncated(t *testing.T) { assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 122 bytes", err.Error()) } +func TestWasmCloseReasonInvalidUTF8(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + c, _, err := websocket.Dial(ctx, os.Getenv("WS_ECHO_SERVER_URL"), &websocket.DialOptions{ + Subprotocols: []string{"echo"}, + }) + assert.Success(t, err) + + // Without replacement, converting these bytes to a JavaScript string + // grows them past 123 bytes and the browser throws. The echo server + // checks the code. + err = c.Close(websocket.StatusNormalClosure, strings.Repeat("\x80", 50)) + assert.Success(t, err) +} + func TestWasmDialTimeout(t *testing.T) { t.Parallel() From e9a0c9307f985207250785d5431df1bd83112bb4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 16:17:15 +0000 Subject: [PATCH 09/19] docs(ws_js.go): document U+FFFD replacement in the Close reason The caveat that invalid UTF-8 can still make the browser reject the reason no longer holds, since Close now replaces it before truncating. The doc now matches the native Close. --- ws_js.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ws_js.go b/ws_js.go index 7c49bf7b..a32f4139 100644 --- a/ws_js.go +++ b/ws_js.go @@ -229,10 +229,9 @@ func (c *Conn) write(typ MessageType, p []byte) error { // or the connection is closed. // It thus performs the full WebSocket close handshake. // -// A reason over 123 bytes is truncated, and Close returns an error wrapping -// ErrCloseReasonTruncated. Truncation only makes valid UTF-8 fit: invalid -// bytes grow when converted to a JavaScript string, so the browser may still -// reject the reason. +// The reason is limited to 123 bytes. Invalid UTF-8 in the reason is replaced +// with U+FFFD. A longer reason is then truncated, and Close returns an error +// wrapping ErrCloseReasonTruncated. func (c *Conn) Close(code StatusCode, reason string) error { err := c.exportedClose(code, reason) if err != nil { From f788d53f30ec7ae0d4ea438815b7c8e396aabb6d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 16:20:58 +0000 Subject: [PATCH 10/19] fix: report the replaced reason length in the truncation error The error said "from" the caller's length, but replacing invalid UTF-8 can grow a reason, so the message could read "truncated from 122 to 123 bytes". Both numbers now describe the reason that was cut, so "from" is always larger than "to". --- close_test.go | 4 ++-- errors.go | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/close_test.go b/close_test.go index 39c3c4b5..6fe4d81b 100644 --- a/close_test.go +++ b/close_test.go @@ -115,8 +115,8 @@ func Test_wireCloseReason(t *testing.T) { {name: "incompleteRune", reason: "a\xe2\x82", want: "a\uFFFD"}, {name: "invalidRunOverMax", reason: strings.Repeat("\x80", 200), want: "\uFFFD"}, // Replacement grows these past maxCloseReason. - {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], truncated: "from 123 to 122 bytes"}, - {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", truncated: "from 122 to 123 bytes"}, + {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], truncated: "from 125 to 122 bytes"}, + {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", truncated: "from 244 to 123 bytes"}, } for _, tc := range testCases { diff --git a/errors.go b/errors.go index dc107540..7283dbec 100644 --- a/errors.go +++ b/errors.go @@ -33,7 +33,5 @@ func wireCloseReason(reason string) (string, error) { for !utf8.RuneStart(valid[n]) { n-- } - // "from" is the caller's length and "to" the sent length. Replacement can - // grow a reason, so "from" can be maxCloseReason or less, even below "to". - return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(reason), n) + return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(valid), n) } From 007027ad901e97a1112a95810118ece079b49bdb Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 16:57:34 +0000 Subject: [PATCH 11/19] fix: put the close failure before the truncation error Close joined the truncation error first, so the message led with how the reason changed and the failure itself followed on the next line. On WASM, a close() that throws also dropped the truncation error the Close doc promises. --- close.go | 6 ++++-- conn_test.go | 2 ++ ws_js.go | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/close.go b/close.go index 4e79ff5d..199b7150 100644 --- a/close.go +++ b/close.go @@ -119,9 +119,11 @@ func (c *Conn) Close(code StatusCode, reason string) (err error) { err = nil } // Joined after the check above so it is not dropped along with a - // net.ErrClosed from the handshake. + // net.ErrClosed from the handshake. The close failure comes first + // because wireErr only reports how the sent frame differs from the + // arguments. if wireErr != nil { - err = errors.Join(wireErr, err) + err = errors.Join(err, wireErr) } }() diff --git a/conn_test.go b/conn_test.go index 5f555ff5..dae1a739 100644 --- a/conn_test.go +++ b/conn_test.go @@ -147,6 +147,8 @@ func TestConn(t *testing.T) { err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) assert.ErrorIs(t, io.ErrClosedPipe, err) + // The close failure leads; the truncation notice follows it. + assert.Equal(t, "truncation last", true, strings.HasSuffix(err.Error(), "\nwebsocket: close reason truncated from 124 to 123 bytes")) }) t.Run("ping", func(t *testing.T) { diff --git a/ws_js.go b/ws_js.go index a32f4139..6d2e92b8 100644 --- a/ws_js.go +++ b/ws_js.go @@ -268,12 +268,12 @@ func (c *Conn) exportedClose(code StatusCode, reason string) error { c.setCloseErr(ce) err := c.ws.Close(int(code), reason) if err != nil { - return err + return errors.Join(err, truncErr) } <-c.closed if !c.closeWasClean { - return errors.Join(truncErr, c.closeErr) + return errors.Join(c.closeErr, truncErr) } return truncErr } From 9726c86b3a17237aec0d6d3a8f5d78d0bf09f29a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 17:32:23 +0000 Subject: [PATCH 12/19] refactor: move wireCloseReason out of errors.go errors.go holds the exported errors. The helper and its limit move to close_shared.go, which has no build tag because both the native Close in close.go (!js) and the WASM Close in ws_js.go call it. --- close_shared.go | 29 +++++++++++++++++++++++++++++ errors.go | 25 ------------------------- 2 files changed, 29 insertions(+), 25 deletions(-) create mode 100644 close_shared.go diff --git a/close_shared.go b/close_shared.go new file mode 100644 index 00000000..b0a0ba3d --- /dev/null +++ b/close_shared.go @@ -0,0 +1,29 @@ +package websocket + +import ( + "fmt" + "strings" + "unicode/utf8" +) + +// maxCloseReason is the 125 byte control frame payload limit minus the 2 byte +// status code, see RFC 6455 section 5.5. +const maxCloseReason = 123 + +// wireCloseReason returns reason as Close sends it: invalid UTF-8 replaced +// with U+FFFD, then cut to the longest prefix of at most maxCloseReason bytes +// that does not split a rune. The error wraps ErrCloseReasonTruncated if the +// reason was cut. +func wireCloseReason(reason string) (string, error) { + // RFC 6455 section 5.5.1 defines the reason as UTF-8, and peers that + // check it fail the connection (section 8.1) instead of reading the code. + valid := strings.ToValidUTF8(reason, "\uFFFD") + if len(valid) <= maxCloseReason { + return valid, nil + } + n := maxCloseReason + for !utf8.RuneStart(valid[n]) { + n-- + } + return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(valid), n) +} diff --git a/errors.go b/errors.go index 7283dbec..662079d4 100644 --- a/errors.go +++ b/errors.go @@ -2,9 +2,6 @@ package websocket import ( "errors" - "fmt" - "strings" - "unicode/utf8" ) // ErrMessageTooBig is returned when a message exceeds the read limit. @@ -13,25 +10,3 @@ var ErrMessageTooBig = errors.New("websocket: message too big") // ErrCloseReasonTruncated is returned by Close when the reason, after invalid // UTF-8 was replaced, was longer than 123 bytes and Close shortened it. var ErrCloseReasonTruncated = errors.New("websocket: close reason truncated") - -// maxCloseReason is the 125 byte control frame payload limit minus the 2 byte -// status code, see RFC 6455 section 5.5. -const maxCloseReason = 123 - -// wireCloseReason returns reason as Close sends it: invalid UTF-8 replaced -// with U+FFFD, then cut to the longest prefix of at most maxCloseReason bytes -// that does not split a rune. The error wraps ErrCloseReasonTruncated if the -// reason was cut. -func wireCloseReason(reason string) (string, error) { - // RFC 6455 section 5.5.1 defines the reason as UTF-8, and peers that - // check it fail the connection (section 8.1) instead of reading the code. - valid := strings.ToValidUTF8(reason, "\uFFFD") - if len(valid) <= maxCloseReason { - return valid, nil - } - n := maxCloseReason - for !utf8.RuneStart(valid[n]) { - n-- - } - return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(valid), n) -} From 2bd2b4a56fa3bd4d82995909a1ecae91373a861d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 17:42:57 +0000 Subject: [PATCH 13/19] refactor: derive maxCloseReason from maxControlPayload again master defines maxCloseReason as maxControlPayload - 2, but the shared helper needs it on js, where frame.go is not built, so it became a literal 123. maxControlPayload moves to close_shared.go so both builds can derive the limit from it. --- close_shared.go | 10 +++++++--- frame.go | 4 ---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/close_shared.go b/close_shared.go index b0a0ba3d..26e74bd4 100644 --- a/close_shared.go +++ b/close_shared.go @@ -6,9 +6,13 @@ import ( "unicode/utf8" ) -// maxCloseReason is the 125 byte control frame payload limit minus the 2 byte -// status code, see RFC 6455 section 5.5. -const maxCloseReason = 123 +// maxControlPayload is the maximum length of a control frame payload. +// See https://tools.ietf.org/html/rfc6455#section-5.5. +const maxControlPayload = 125 + +// maxCloseReason is the control frame payload limit minus the 2 byte status +// code. +const maxCloseReason = maxControlPayload - 2 // wireCloseReason returns reason as Close sends it: invalid UTF-8 replaced // with U+FFFD, then cut to the longest prefix of at most maxCloseReason bytes diff --git a/frame.go b/frame.go index e7ab76be..a6c8a10e 100644 --- a/frame.go +++ b/frame.go @@ -101,10 +101,6 @@ func readFrameHeader(r *bufio.Reader, readBuf []byte) (h header, err error) { return h, nil } -// maxControlPayload is the maximum length of a control frame payload. -// See https://tools.ietf.org/html/rfc6455#section-5.5. -const maxControlPayload = 125 - // writeFrameHeader writes the bytes of the header to w. // See https://tools.ietf.org/html/rfc6455#section-5.2 func writeFrameHeader(h header, w *bufio.Writer, buf []byte) (err error) { From 7daec2cb057dd521829a0c413757ca79aa4c17fb Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 17:57:14 +0000 Subject: [PATCH 14/19] docs: drop comments that repeat the Close docs The unexported helpers and the tests restated what the Close doc, the code or the test names already say, and the RFC note in wireCloseReason was hard to follow. Keep each fact in the exported docs and only the comments that explain something the code does not show. --- close.go | 28 +++++++++------------------- close_shared.go | 9 +-------- close_test.go | 4 +--- conn_test.go | 7 ------- errors.go | 4 ++-- ws_js_test.go | 6 +----- 6 files changed, 14 insertions(+), 44 deletions(-) diff --git a/close.go b/close.go index 199b7150..b95a2501 100644 --- a/close.go +++ b/close.go @@ -92,14 +92,13 @@ func CloseStatus(err error) StatusCode { // The connection can only be closed once. Additional calls to Close // are no-ops. // -// The reason is limited to 123 bytes. Invalid UTF-8 in the reason is replaced -// with U+FFFD. A longer reason is then cut to the longest prefix that does not -// split a UTF-8 encoded rune, the handshake proceeds, and Close returns an -// error wrapping ErrCloseReasonTruncated. An invalid code is replaced by -// StatusInternalError and reported as an error. +// Close replaces invalid UTF-8 in reason with U+FFFD, then truncates it to fit +// 123 bytes without splitting a rune. The handshake proceeds, but Close returns +// an error wrapping ErrCloseReasonTruncated if the reason was truncated. // -// When the first call to Close returns, the connection is closed, even if -// Close returns an error. A non-nil error reports a problem during the close. +// Invalid codes are sent as StatusInternalError and reported as errors. +// +// When the first call to Close returns, the connection is closed, even on error. // // Close will unblock all goroutines interacting with the connection once // complete. @@ -118,10 +117,7 @@ func (c *Conn) Close(code StatusCode, reason string) (err error) { if errors.Is(err, net.ErrClosed) { err = nil } - // Joined after the check above so it is not dropped along with a - // net.ErrClosed from the handshake. The close failure comes first - // because wireErr only reports how the sent frame differs from the - // arguments. + // Preserve wireErr when net.ErrClosed is ignored. if wireErr != nil { err = errors.Join(err, wireErr) } @@ -183,8 +179,6 @@ func (c *Conn) closeHandshake(code StatusCode, reason string) error { } func (c *Conn) writeClose(code StatusCode, reason string) error { - // Close reports the wire error to the user. Internal callers only need - // a close frame to be sent. ce, _ := wireCloseError(code, reason) var p []byte @@ -302,13 +296,9 @@ func validWireCloseCode(code StatusCode) bool { return false } -// wireCloseError returns the CloseError to send for code and reason. A marshal -// problem must not prevent sending a close frame, so an invalid code is -// replaced by StatusInternalError and the reason goes through wireCloseReason. -// The error reports a replaced code or a truncated reason. func wireCloseError(code StatusCode, reason string) (CloseError, error) { if code == StatusNoStatusRcvd { - return CloseError{Code: code}, nil // Sent as an empty close frame. + return CloseError{Code: code}, nil } if !validWireCloseCode(code) { return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: status code %v cannot be set", code) @@ -317,7 +307,7 @@ func wireCloseError(code StatusCode, reason string) (CloseError, error) { return CloseError{Code: code, Reason: reason}, err } -// bytes encodes ce as a close frame payload. ce must come from wireCloseError. +// bytes requires ce to come from wireCloseError. func (ce CloseError) bytes() []byte { buf := make([]byte, 2+len(ce.Reason)) binary.BigEndian.PutUint16(buf, uint16(ce.Code)) diff --git a/close_shared.go b/close_shared.go index 26e74bd4..22d2e849 100644 --- a/close_shared.go +++ b/close_shared.go @@ -10,17 +10,10 @@ import ( // See https://tools.ietf.org/html/rfc6455#section-5.5. const maxControlPayload = 125 -// maxCloseReason is the control frame payload limit minus the 2 byte status -// code. +// Reserve 2 payload bytes for the status code. const maxCloseReason = maxControlPayload - 2 -// wireCloseReason returns reason as Close sends it: invalid UTF-8 replaced -// with U+FFFD, then cut to the longest prefix of at most maxCloseReason bytes -// that does not split a rune. The error wraps ErrCloseReasonTruncated if the -// reason was cut. func wireCloseReason(reason string) (string, error) { - // RFC 6455 section 5.5.1 defines the reason as UTF-8, and peers that - // check it fail the connection (section 8.1) instead of reading the code. valid := strings.ToValidUTF8(reason, "\uFFFD") if len(valid) <= maxCloseReason { return valid, nil diff --git a/close_test.go b/close_test.go index 6fe4d81b..a54795d2 100644 --- a/close_test.go +++ b/close_test.go @@ -98,7 +98,7 @@ func Test_wireCloseReason(t *testing.T) { name string reason string want string - truncated string // Expected error message, empty if not truncated. + truncated string }{ {name: "empty", reason: "", want: ""}, {name: "max", reason: ascii, want: ascii}, @@ -108,13 +108,11 @@ func Test_wireCloseReason(t *testing.T) { {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41), truncated: "from 126 to 123 bytes"}, {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122], truncated: "from 125 to 122 bytes"}, {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30), truncated: "from 124 to 120 bytes"}, - // U+FFFD decodes as utf8.RuneError but is a valid 3 byte rune. {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122], truncated: "from 125 to 122 bytes"}, {name: "invalidByte", reason: "a\x80b", want: "a\uFFFDb"}, {name: "invalidRun", reason: "a\x80\x80\xffb", want: "a\uFFFDb"}, {name: "incompleteRune", reason: "a\xe2\x82", want: "a\uFFFD"}, {name: "invalidRunOverMax", reason: strings.Repeat("\x80", 200), want: "\uFFFD"}, - // Replacement grows these past maxCloseReason. {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], truncated: "from 125 to 122 bytes"}, {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", truncated: "from 244 to 123 bytes"}, } diff --git a/conn_test.go b/conn_test.go index dae1a739..6e37fd06 100644 --- a/conn_test.go +++ b/conn_test.go @@ -72,7 +72,6 @@ func TestConn(t *testing.T) { err := c1.Close(-1, "") assert.Contains(t, err, "failed to marshal close frame: status code StatusCode(-1) cannot be set") - // The peer still gets a close frame. assert.Equal(t, "peer close status", websocket.StatusInternalError, websocket.CloseStatus(<-readErr)) }) @@ -107,7 +106,6 @@ func TestConn(t *testing.T) { assert.Success(t, err) } else { assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - // The handshake succeeded, so nothing else is reported. assert.Equal(t, "close error", fmt.Sprintf("failed to close WebSocket: websocket: close reason truncated from %d to %d bytes", len(tc.reason), len(tc.want)), err.Error()) } @@ -120,9 +118,6 @@ func TestConn(t *testing.T) { } }) - // The CloseRead goroutine may read the echoed close frame and close the - // connection, failing the handshake with net.ErrClosed, which Close - // ignores. The truncation must still be reported. t.Run("closeReasonTruncatedWithCloseRead", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) @@ -141,13 +136,11 @@ func TestConn(t *testing.T) { t.Run("closeReasonTruncatedWithWriteError", func(t *testing.T) { _, c1, c2 := newConnTest(t, nil, nil) - // Closing the peer's end of the pipe makes writing the close frame fail. assert.Success(t, c2.CloseNow()) err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) assert.ErrorIs(t, io.ErrClosedPipe, err) - // The close failure leads; the truncation notice follows it. assert.Equal(t, "truncation last", true, strings.HasSuffix(err.Error(), "\nwebsocket: close reason truncated from 124 to 123 bytes")) }) diff --git a/errors.go b/errors.go index 662079d4..71f36e67 100644 --- a/errors.go +++ b/errors.go @@ -7,6 +7,6 @@ import ( // ErrMessageTooBig is returned when a message exceeds the read limit. var ErrMessageTooBig = errors.New("websocket: message too big") -// ErrCloseReasonTruncated is returned by Close when the reason, after invalid -// UTF-8 was replaced, was longer than 123 bytes and Close shortened it. +// ErrCloseReasonTruncated indicates that Close shortened the reason to fit +// 123 bytes after replacing invalid UTF-8. var ErrCloseReasonTruncated = errors.New("websocket: close reason truncated") diff --git a/ws_js_test.go b/ws_js_test.go index 2595dc67..58b60a0d 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -49,8 +49,6 @@ func TestWasmCloseReasonTruncated(t *testing.T) { }) assert.Success(t, err) - // The browser throws on reasons over 123 bytes, so a clean close shows - // the reason was truncated. The echo server checks the code. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("é", 62)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 122 bytes", err.Error()) @@ -67,9 +65,7 @@ func TestWasmCloseReasonInvalidUTF8(t *testing.T) { }) assert.Success(t, err) - // Without replacement, converting these bytes to a JavaScript string - // grows them past 123 bytes and the browser throws. The echo server - // checks the code. + // JavaScript string conversion expands these 50 bytes past the reason limit. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("\x80", 50)) assert.Success(t, err) } From 0b09d991ab2b96e2e0854786387411b397151641 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 18:28:00 +0000 Subject: [PATCH 15/19] docs: restore the comments that explain the close error handling The previous cleanup also removed comments that state facts the code does not show: why writeClose ignores the wireCloseError error, why wireErr is joined after the net.ErrClosed check and in that order, what wireCloseError guarantees, and why the CloseRead test exists. The truncated test field is renamed to wantErrSuffix because it holds the expected end of the error message. --- close.go | 8 +++++++- close_shared.go | 1 + close_test.go | 28 ++++++++++++++-------------- conn_test.go | 2 ++ ws_js_test.go | 4 +++- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/close.go b/close.go index b95a2501..988ec528 100644 --- a/close.go +++ b/close.go @@ -99,6 +99,7 @@ func CloseStatus(err error) StatusCode { // Invalid codes are sent as StatusInternalError and reported as errors. // // When the first call to Close returns, the connection is closed, even on error. +// A non-nil error reports a problem during the close. // // Close will unblock all goroutines interacting with the connection once // complete. @@ -117,7 +118,8 @@ func (c *Conn) Close(code StatusCode, reason string) (err error) { if errors.Is(err, net.ErrClosed) { err = nil } - // Preserve wireErr when net.ErrClosed is ignored. + // Joined after the net.ErrClosed check so that wireErr is not set to nil. + // The close failure is first; wireErr only reports changes to the sent frame. if wireErr != nil { err = errors.Join(err, wireErr) } @@ -179,6 +181,7 @@ func (c *Conn) closeHandshake(code StatusCode, reason string) error { } func (c *Conn) writeClose(code StatusCode, reason string) error { + // Close returns this error to its caller. ce, _ := wireCloseError(code, reason) var p []byte @@ -296,6 +299,9 @@ func validWireCloseCode(code StatusCode) bool { return false } +// wireCloseError returns a CloseError that can always be sent. It replaces an +// invalid code with StatusInternalError and passes the reason through +// wireCloseReason. The error reports a replaced code or a truncated reason. func wireCloseError(code StatusCode, reason string) (CloseError, error) { if code == StatusNoStatusRcvd { return CloseError{Code: code}, nil diff --git a/close_shared.go b/close_shared.go index 22d2e849..24164be3 100644 --- a/close_shared.go +++ b/close_shared.go @@ -14,6 +14,7 @@ const maxControlPayload = 125 const maxCloseReason = maxControlPayload - 2 func wireCloseReason(reason string) (string, error) { + // RFC 6455 section 5.5.1 requires a UTF-8 reason. valid := strings.ToValidUTF8(reason, "\uFFFD") if len(valid) <= maxCloseReason { return valid, nil diff --git a/close_test.go b/close_test.go index a54795d2..e9855c15 100644 --- a/close_test.go +++ b/close_test.go @@ -95,26 +95,26 @@ func Test_wireCloseReason(t *testing.T) { ascii := strings.Repeat("x", maxCloseReason) testCases := []struct { - name string - reason string - want string - truncated string + name string + reason string + want string + wantErrSuffix string }{ {name: "empty", reason: "", want: ""}, {name: "max", reason: ascii, want: ascii}, - {name: "maxPlusOne", reason: ascii + "x", want: ascii, truncated: "from 124 to 123 bytes"}, + {name: "maxPlusOne", reason: ascii + "x", want: ascii, wantErrSuffix: "from 124 to 123 bytes"}, {name: "twoByteFits", reason: ascii[:121] + "é", want: ascii[:121] + "é"}, - {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122], truncated: "from 124 to 122 bytes"}, - {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41), truncated: "from 126 to 123 bytes"}, - {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122], truncated: "from 125 to 122 bytes"}, - {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30), truncated: "from 124 to 120 bytes"}, - {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122], truncated: "from 125 to 122 bytes"}, + {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122], wantErrSuffix: "from 124 to 122 bytes"}, + {name: "threeByteAligned", reason: strings.Repeat("€", 42), want: strings.Repeat("€", 41), wantErrSuffix: "from 126 to 123 bytes"}, + {name: "threeByteSplit", reason: ascii[:122] + "€", want: ascii[:122], wantErrSuffix: "from 125 to 122 bytes"}, + {name: "fourByteSplit", reason: strings.Repeat("😀", 31), want: strings.Repeat("😀", 30), wantErrSuffix: "from 124 to 120 bytes"}, + {name: "replacementCharSplit", reason: ascii[:122] + "\uFFFD", want: ascii[:122], wantErrSuffix: "from 125 to 122 bytes"}, {name: "invalidByte", reason: "a\x80b", want: "a\uFFFDb"}, {name: "invalidRun", reason: "a\x80\x80\xffb", want: "a\uFFFDb"}, {name: "incompleteRune", reason: "a\xe2\x82", want: "a\uFFFD"}, {name: "invalidRunOverMax", reason: strings.Repeat("\x80", 200), want: "\uFFFD"}, - {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], truncated: "from 125 to 122 bytes"}, - {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", truncated: "from 244 to 123 bytes"}, + {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], wantErrSuffix: "from 125 to 122 bytes"}, + {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", wantErrSuffix: "from 244 to 123 bytes"}, } for _, tc := range testCases { @@ -123,11 +123,11 @@ func Test_wireCloseReason(t *testing.T) { got, err := wireCloseReason(tc.reason) assert.Equal(t, "reason", tc.want, got) - if tc.truncated == "" { + if tc.wantErrSuffix == "" { assert.Success(t, err) } else { assert.ErrorIs(t, ErrCloseReasonTruncated, err) - assert.Equal(t, "error", "websocket: close reason truncated "+tc.truncated, err.Error()) + assert.Equal(t, "error", "websocket: close reason truncated "+tc.wantErrSuffix, err.Error()) } if !utf8.ValidString(got) || len(got) > maxCloseReason { t.Fatalf("reason %q is not valid UTF-8 within %d bytes", got, maxCloseReason) diff --git a/conn_test.go b/conn_test.go index 6e37fd06..6bc042d5 100644 --- a/conn_test.go +++ b/conn_test.go @@ -118,6 +118,8 @@ func TestConn(t *testing.T) { } }) + // The CloseRead goroutine can read the peer's close frame before Close does, + // so the handshake fails with net.ErrClosed, which Close ignores. t.Run("closeReasonTruncatedWithCloseRead", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) diff --git a/ws_js_test.go b/ws_js_test.go index 58b60a0d..b02d9aff 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -49,6 +49,7 @@ func TestWasmCloseReasonTruncated(t *testing.T) { }) assert.Success(t, err) + // The echo server checks the close code. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("é", 62)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 122 bytes", err.Error()) @@ -65,7 +66,8 @@ func TestWasmCloseReasonInvalidUTF8(t *testing.T) { }) assert.Success(t, err) - // JavaScript string conversion expands these 50 bytes past the reason limit. + // Without the U+FFFD replacement, these 50 bytes become more than 123 bytes + // when converted to a JavaScript string. The echo server checks the close code. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("\x80", 50)) assert.Success(t, err) } From a747f21865e11fcdeae2993de6a954187587a811 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 18:45:16 +0000 Subject: [PATCH 16/19] fix: say why a truncated reason is longer than the caller's Replacing invalid UTF-8 with U+FFFD can make a reason longer, so a 122-byte reason could report "truncated from 244 to 123 bytes". The error now ends with "after replacing invalid UTF-8" when the replaced length differs from the caller's. --- close_shared.go | 10 ++++++++-- close_test.go | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/close_shared.go b/close_shared.go index 24164be3..8b0b4c1e 100644 --- a/close_shared.go +++ b/close_shared.go @@ -10,9 +10,11 @@ import ( // See https://tools.ietf.org/html/rfc6455#section-5.5. const maxControlPayload = 125 -// Reserve 2 payload bytes for the status code. +// maxCloseReason is maxControlPayload minus the 2-byte status code. const maxCloseReason = maxControlPayload - 2 +// wireCloseReason returns a reason that can always be sent, and an error if it +// truncated reason. func wireCloseReason(reason string) (string, error) { // RFC 6455 section 5.5.1 requires a UTF-8 reason. valid := strings.ToValidUTF8(reason, "\uFFFD") @@ -23,5 +25,9 @@ func wireCloseReason(reason string) (string, error) { for !utf8.RuneStart(valid[n]) { n-- } - return valid[:n], fmt.Errorf("%w from %d to %d bytes", ErrCloseReasonTruncated, len(valid), n) + var replaced string + if len(valid) != len(reason) { + replaced = " after replacing invalid UTF-8" + } + return valid[:n], fmt.Errorf("%w from %d to %d bytes%s", ErrCloseReasonTruncated, len(valid), n, replaced) } diff --git a/close_test.go b/close_test.go index e9855c15..2808c252 100644 --- a/close_test.go +++ b/close_test.go @@ -113,8 +113,8 @@ func Test_wireCloseReason(t *testing.T) { {name: "invalidRun", reason: "a\x80\x80\xffb", want: "a\uFFFDb"}, {name: "incompleteRune", reason: "a\xe2\x82", want: "a\uFFFD"}, {name: "invalidRunOverMax", reason: strings.Repeat("\x80", 200), want: "\uFFFD"}, - {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], wantErrSuffix: "from 125 to 122 bytes"}, - {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", wantErrSuffix: "from 244 to 123 bytes"}, + {name: "growthFromMax", reason: ascii[:122] + "\x80", want: ascii[:122], wantErrSuffix: "from 125 to 122 bytes after replacing invalid UTF-8"}, + {name: "growthIsolated", reason: strings.Repeat("\x80x", 61), want: strings.Repeat("\uFFFDx", 30) + "\uFFFD", wantErrSuffix: "from 244 to 123 bytes after replacing invalid UTF-8"}, } for _, tc := range testCases { From b8f74f0ea692188f1477b1ba3d0023fe3ae01019 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 18:45:16 +0000 Subject: [PATCH 17/19] fix: return the truncation error only when Close succeeds Close joined the truncation error with any close failure and prefixed it with "failed to close WebSocket", so errors.Is could not tell a completed close with a shortened reason from a failed close. Close now returns the close failure if there is one, and otherwise the unprefixed wire error, so a match on ErrCloseReasonTruncated means the close completed. The invalid code error states that 1011 was sent, the Close doc lists valid codes and StatusNoStatusRcvd, and a test covers a long reason passed to writeError. --- close.go | 30 ++++++++++++++++++------------ conn_test.go | 29 +++++++++++++++++++++-------- export_test.go | 4 ++++ ws_js.go | 19 +++++++++---------- ws_js_test.go | 2 +- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/close.go b/close.go index 988ec528..c27795a0 100644 --- a/close.go +++ b/close.go @@ -93,17 +93,29 @@ func CloseStatus(err error) StatusCode { // are no-ops. // // Close replaces invalid UTF-8 in reason with U+FFFD, then truncates it to fit -// 123 bytes without splitting a rune. The handshake proceeds, but Close returns -// an error wrapping ErrCloseReasonTruncated if the reason was truncated. +// 123 bytes without splitting a rune. If it truncated the reason and the close +// had no other error, Close returns an error wrapping ErrCloseReasonTruncated. // -// Invalid codes are sent as StatusInternalError and reported as errors. +// Valid codes are 1000 to 1003, 1007 to 1014, and 3000 to 4999. Close sends +// any other code as StatusInternalError with no reason and returns an error. +// The exception is StatusNoStatusRcvd, which sends a close frame with no code +// and no reason. // // When the first call to Close returns, the connection is closed, even on error. // A non-nil error reports a problem during the close. // // Close will unblock all goroutines interacting with the connection once // complete. -func (c *Conn) Close(code StatusCode, reason string) (err error) { +func (c *Conn) Close(code StatusCode, reason string) error { + ce, wireErr := wireCloseError(code, reason) + err := c.exportedClose(ce.Code, ce.Reason) + if err != nil { + return err + } + return wireErr +} + +func (c *Conn) exportedClose(code StatusCode, reason string) (err error) { defer errd.Wrap(&err, "failed to close WebSocket") if c.casClosing() { @@ -113,19 +125,13 @@ func (c *Conn) Close(code StatusCode, reason string) (err error) { } return net.ErrClosed } - ce, wireErr := wireCloseError(code, reason) defer func() { if errors.Is(err, net.ErrClosed) { err = nil } - // Joined after the net.ErrClosed check so that wireErr is not set to nil. - // The close failure is first; wireErr only reports changes to the sent frame. - if wireErr != nil { - err = errors.Join(err, wireErr) - } }() - err = c.closeHandshake(ce.Code, ce.Reason) + err = c.closeHandshake(code, reason) err2 := c.close() if err == nil && err2 != nil { @@ -307,7 +313,7 @@ func wireCloseError(code StatusCode, reason string) (CloseError, error) { return CloseError{Code: code}, nil } if !validWireCloseCode(code) { - return CloseError{Code: StatusInternalError}, fmt.Errorf("failed to marshal close frame: status code %v cannot be set", code) + return CloseError{Code: StatusInternalError}, fmt.Errorf("websocket: invalid close status code %v, sent %v instead", code, StatusInternalError) } reason, err := wireCloseReason(reason) return CloseError{Code: code, Reason: reason}, err diff --git a/conn_test.go b/conn_test.go index 6bc042d5..d54e0c72 100644 --- a/conn_test.go +++ b/conn_test.go @@ -70,7 +70,7 @@ func TestConn(t *testing.T) { }) err := c1.Close(-1, "") - assert.Contains(t, err, "failed to marshal close frame: status code StatusCode(-1) cannot be set") + assert.Equal(t, "close error", "websocket: invalid close status code StatusCode(-1), sent StatusInternalError instead", err.Error()) assert.Equal(t, "peer close status", websocket.StatusInternalError, websocket.CloseStatus(<-readErr)) }) @@ -87,7 +87,7 @@ func TestConn(t *testing.T) { truncated bool }{ {name: "max", reason: ascii, want: ascii}, - {name: "ascii", reason: ascii + "xyz", want: ascii, truncated: true}, + {name: "overMax", reason: ascii + "xyz", want: ascii, truncated: true}, {name: "twoByteSplit", reason: ascii[:122] + "é", want: ascii[:122], truncated: true}, {name: "fourByteSplit", reason: strings.Repeat("😀", 40), want: strings.Repeat("😀", 30), truncated: true}, {name: "invalidUTF8", reason: "bad \xff\xfe reason", want: "bad \uFFFD reason"}, @@ -106,11 +106,11 @@ func TestConn(t *testing.T) { assert.Success(t, err) } else { assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", fmt.Sprintf("failed to close WebSocket: websocket: close reason truncated from %d to %d bytes", len(tc.reason), len(tc.want)), err.Error()) + assert.Equal(t, "close error", fmt.Sprintf("websocket: close reason truncated from %d to %d bytes", len(tc.reason), len(tc.want)), err.Error()) } - var ce websocket.CloseError - if !errors.As(<-readErr, &ce) { + ce, ok := errors.AsType[websocket.CloseError](<-readErr) + if !ok { t.Fatal("peer did not receive a close frame") } assert.Equal(t, "peer close error", websocket.CloseError{Code: code, Reason: tc.want}, ce) @@ -131,7 +131,7 @@ func TestConn(t *testing.T) { err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 123 bytes", err.Error()) + assert.Equal(t, "close error", "websocket: close reason truncated from 124 to 123 bytes", err.Error()) assert.Equal(t, "peer close status", websocket.StatusNormalClosure, websocket.CloseStatus(<-readErr)) }) @@ -141,9 +141,22 @@ func TestConn(t *testing.T) { assert.Success(t, c2.CloseNow()) err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) - assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) assert.ErrorIs(t, io.ErrClosedPipe, err) - assert.Equal(t, "truncation last", true, strings.HasSuffix(err.Error(), "\nwebsocket: close reason truncated from 124 to 123 bytes")) + assert.Equal(t, "reports truncation", false, errors.Is(err, websocket.ErrCloseReasonTruncated)) + }) + + t.Run("writeErrorReasonTruncated", func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + _ = c1.CloseRead(tt.ctx) + readErr := xsync.Go(func() error { + _, _, err := c2.Read(tt.ctx) + return err + }) + + c1.WriteError(websocket.StatusProtocolError, errors.New(strings.Repeat("x", 124))) + ce, _ := errors.AsType[websocket.CloseError](<-readErr) + assert.Equal(t, "peer close error", websocket.CloseError{Code: websocket.StatusProtocolError, Reason: strings.Repeat("x", 123)}, ce) }) t.Run("ping", func(t *testing.T) { diff --git a/export_test.go b/export_test.go index e0071922..d211e2ac 100644 --- a/export_test.go +++ b/export_test.go @@ -37,3 +37,7 @@ var ( ) var CompressionModeOpts = CompressionMode.opts + +func (c *Conn) WriteError(code StatusCode, err error) { + c.writeError(code, err) +} diff --git a/ws_js.go b/ws_js.go index 6d2e92b8..d8bed4ba 100644 --- a/ws_js.go +++ b/ws_js.go @@ -229,15 +229,17 @@ func (c *Conn) write(typ MessageType, p []byte) error { // or the connection is closed. // It thus performs the full WebSocket close handshake. // -// The reason is limited to 123 bytes. Invalid UTF-8 in the reason is replaced -// with U+FFFD. A longer reason is then truncated, and Close returns an error -// wrapping ErrCloseReasonTruncated. +// Close replaces invalid UTF-8 in reason with U+FFFD, then truncates it to fit +// 123 bytes without splitting a rune. If it truncated the reason and the close +// had no other error, Close returns an error wrapping ErrCloseReasonTruncated. func (c *Conn) Close(code StatusCode, reason string) error { + // Browsers throw on a reason over 123 bytes and leave the connection open. + reason, truncErr := wireCloseReason(reason) err := c.exportedClose(code, reason) if err != nil { return fmt.Errorf("failed to close WebSocket: %w", err) } - return nil + return truncErr } // CloseNow closes the WebSocket connection without attempting a close handshake. @@ -250,9 +252,6 @@ func (c *Conn) CloseNow() error { } func (c *Conn) exportedClose(code StatusCode, reason string) error { - // Browsers throw on a reason over 123 bytes and leave the connection open. - reason, truncErr := wireCloseReason(reason) - c.closingMu.Lock() defer c.closingMu.Unlock() @@ -268,14 +267,14 @@ func (c *Conn) exportedClose(code StatusCode, reason string) error { c.setCloseErr(ce) err := c.ws.Close(int(code), reason) if err != nil { - return errors.Join(err, truncErr) + return err } <-c.closed if !c.closeWasClean { - return errors.Join(c.closeErr, truncErr) + return c.closeErr } - return truncErr + return nil } // Subprotocol returns the negotiated subprotocol. diff --git a/ws_js_test.go b/ws_js_test.go index b02d9aff..a1d846fa 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -52,7 +52,7 @@ func TestWasmCloseReasonTruncated(t *testing.T) { // The echo server checks the close code. err = c.Close(websocket.StatusNormalClosure, strings.Repeat("é", 62)) assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", "failed to close WebSocket: websocket: close reason truncated from 124 to 122 bytes", err.Error()) + assert.Equal(t, "close error", "websocket: close reason truncated from 124 to 122 bytes", err.Error()) } func TestWasmCloseReasonInvalidUTF8(t *testing.T) { From ec299b6351b800e9565369c856503384ac5433e1 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 20:57:15 +0000 Subject: [PATCH 18/19] fix: encode every wireCloseError result in CloseError.bytes bytes encoded StatusNoStatusRcvd as a 1005 status code, which peers reject, and only a check in writeClose kept it off the wire, so the wireCloseError doc was wrong. bytes now returns no payload for it, and the writeClose comment states which callers drop the wire error. Tests cover Close with StatusNoStatusRcvd and print the peer's error when no close frame arrives; the CloseRead subtest is removed because other tests cover the net.ErrClosed path. --- close.go | 21 ++++++++++----------- conn_test.go | 47 ++++++++++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/close.go b/close.go index c27795a0..2252e949 100644 --- a/close.go +++ b/close.go @@ -187,18 +187,14 @@ func (c *Conn) closeHandshake(code StatusCode, reason string) error { } func (c *Conn) writeClose(code StatusCode, reason string) error { - // Close returns this error to its caller. + // The error is dropped: Close reports it from its own wireCloseError call, + // and writeError and the reply to a peer's close frame do not report it. ce, _ := wireCloseError(code, reason) - var p []byte - if ce.Code != StatusNoStatusRcvd { - p = ce.bytes() - } - ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - err := c.writeControl(ctx, opClose, p) + err := c.writeControl(ctx, opClose, ce.bytes()) // If the connection closed as we're writing we ignore the error as we might // have written the close frame, the peer responded and then someone else read it // and closed the connection. @@ -305,9 +301,8 @@ func validWireCloseCode(code StatusCode) bool { return false } -// wireCloseError returns a CloseError that can always be sent. It replaces an -// invalid code with StatusInternalError and passes the reason through -// wireCloseReason. The error reports a replaced code or a truncated reason. +// wireCloseError returns the CloseError to send for code and reason, also when +// the error is non-nil. The error reports a replaced code or a truncated reason. func wireCloseError(code StatusCode, reason string) (CloseError, error) { if code == StatusNoStatusRcvd { return CloseError{Code: code}, nil @@ -319,8 +314,12 @@ func wireCloseError(code StatusCode, reason string) (CloseError, error) { return CloseError{Code: code, Reason: reason}, err } -// bytes requires ce to come from wireCloseError. +// bytes returns the close frame payload for ce, which must come from +// wireCloseError. func (ce CloseError) bytes() []byte { + if ce.Code == StatusNoStatusRcvd { + return nil + } buf := make([]byte, 2+len(ce.Reason)) binary.BigEndian.PutUint16(buf, uint16(ce.Code)) copy(buf[2:], ce.Reason) diff --git a/conn_test.go b/conn_test.go index d54e0c72..c955e86f 100644 --- a/conn_test.go +++ b/conn_test.go @@ -72,7 +72,24 @@ func TestConn(t *testing.T) { err := c1.Close(-1, "") assert.Equal(t, "close error", "websocket: invalid close status code StatusCode(-1), sent StatusInternalError instead", err.Error()) - assert.Equal(t, "peer close status", websocket.StatusInternalError, websocket.CloseStatus(<-readErr)) + assert.Success(t, assertCloseStatus(websocket.StatusInternalError, <-readErr)) + }) + + t.Run("noStatusClose", func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + readErr := xsync.Go(func() error { + _, _, err := c2.Read(tt.ctx) + return err + }) + + assert.Success(t, c1.Close(websocket.StatusNoStatusRcvd, "dropped")) + peerErr := <-readErr + ce, ok := errors.AsType[websocket.CloseError](peerErr) + if !ok { + t.Fatalf("peer did not receive a close frame: %v", peerErr) + } + assert.Equal(t, "peer close error", websocket.CloseError{Code: websocket.StatusNoStatusRcvd}, ce) }) t.Run("closeReason", func(t *testing.T) { @@ -109,32 +126,16 @@ func TestConn(t *testing.T) { assert.Equal(t, "close error", fmt.Sprintf("websocket: close reason truncated from %d to %d bytes", len(tc.reason), len(tc.want)), err.Error()) } - ce, ok := errors.AsType[websocket.CloseError](<-readErr) + peerErr := <-readErr + ce, ok := errors.AsType[websocket.CloseError](peerErr) if !ok { - t.Fatal("peer did not receive a close frame") + t.Fatalf("peer did not receive a close frame: %v", peerErr) } assert.Equal(t, "peer close error", websocket.CloseError{Code: code, Reason: tc.want}, ce) }) } }) - // The CloseRead goroutine can read the peer's close frame before Close does, - // so the handshake fails with net.ErrClosed, which Close ignores. - t.Run("closeReasonTruncatedWithCloseRead", func(t *testing.T) { - tt, c1, c2 := newConnTest(t, nil, nil) - - _ = c1.CloseRead(tt.ctx) - readErr := xsync.Go(func() error { - _, _, err := c2.Read(tt.ctx) - return err - }) - - err := c1.Close(websocket.StatusNormalClosure, strings.Repeat("x", 124)) - assert.ErrorIs(t, websocket.ErrCloseReasonTruncated, err) - assert.Equal(t, "close error", "websocket: close reason truncated from 124 to 123 bytes", err.Error()) - assert.Equal(t, "peer close status", websocket.StatusNormalClosure, websocket.CloseStatus(<-readErr)) - }) - t.Run("closeReasonTruncatedWithWriteError", func(t *testing.T) { _, c1, c2 := newConnTest(t, nil, nil) @@ -155,7 +156,11 @@ func TestConn(t *testing.T) { }) c1.WriteError(websocket.StatusProtocolError, errors.New(strings.Repeat("x", 124))) - ce, _ := errors.AsType[websocket.CloseError](<-readErr) + peerErr := <-readErr + ce, ok := errors.AsType[websocket.CloseError](peerErr) + if !ok { + t.Fatalf("peer did not receive a close frame: %v", peerErr) + } assert.Equal(t, "peer close error", websocket.CloseError{Code: websocket.StatusProtocolError, Reason: strings.Repeat("x", 123)}, ce) }) From ac78959c1ed54e6b3f103a36979870550663134e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 23 Oct 2026 21:04:23 +0000 Subject: [PATCH 19/19] docs(close): drop the Close doc line about non-nil errors "A non-nil error reports a problem during the close" repeats the line before it, and a truncation-only error is not a problem during the close. --- close.go | 1 - 1 file changed, 1 deletion(-) diff --git a/close.go b/close.go index 2252e949..36ef0a63 100644 --- a/close.go +++ b/close.go @@ -102,7 +102,6 @@ func CloseStatus(err error) StatusCode { // and no reason. // // When the first call to Close returns, the connection is closed, even on error. -// A non-nil error reports a problem during the close. // // Close will unblock all goroutines interacting with the connection once // complete.