From b0502730fcab49134425ae0602d65f448c84b1d7 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 Oct 2026 17:58:31 +0000 Subject: [PATCH 01/10] fix: keep a read limit set before NetConn NetConn always called SetReadLimit(-1), so a limit set before NetConn was silently dropped while one set after it applied. NetConn now disables the limit only if SetReadLimit was never called on the Conn. --- conn.go | 1 + conn_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++ netconn.go | 7 +++++-- read.go | 5 +++++ ws_js.go | 6 ++++++ 5 files changed, 71 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index 0569bd2c..1df92623 100644 --- a/conn.go +++ b/conn.go @@ -59,6 +59,7 @@ type Conn struct { readHeaderBuf [8]byte readControlBuf [maxControlPayload]byte msgReader *msgReader + readLimitSet atomic.Bool // Write state. msgWriter *msgWriter diff --git a/conn_test.go b/conn_test.go index 22208520..10405a70 100644 --- a/conn_test.go +++ b/conn_test.go @@ -316,6 +316,60 @@ func TestConn(t *testing.T) { assert.Equal(t, "read msg", s, string(b)) }) + for _, tc := range []struct { + name string + setLimit func(ctx context.Context, c *websocket.Conn) net.Conn + }{ + {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(1024) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }}, + {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + nc := websocket.NetConn(ctx, c, websocket.MessageBinary) + c.SetReadLimit(1024) + return nc + }}, + } { + t.Run("netConn/readLimitKept/"+tc.name, func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + n1 := tc.setLimit(tt.ctx, c1) + _ = c2.CloseRead(tt.ctx) + + writeDone := xsync.Go(func() error { + return c2.Write(tt.ctx, websocket.MessageBinary, make([]byte, 4096)) + }) + + _, err := io.ReadFull(n1, make([]byte, 4096)) + assert.ErrorIs(t, websocket.ErrMessageTooBig, err) + + _ = c2.CloseNow() + <-writeDone + }) + } + + t.Run("netConn/readLimitDisabledBeforeNetConn", func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + c1.SetReadLimit(-1) + n1 := websocket.NetConn(tt.ctx, c1, websocket.MessageBinary) + n2 := websocket.NetConn(tt.ctx, c2, websocket.MessageBinary) + + s := strings.Repeat("papa", 1<<20) + errs := xsync.Go(func() error { + _, err := n2.Write([]byte(s)) + if err != nil { + return err + } + return n2.Close() + }) + + b, err := io.ReadAll(n1) + assert.Success(t, err) + assert.Success(t, <-errs) + assert.Equal(t, "read msg", s, string(b)) + }) + t.Run("netConn/pastDeadline", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) diff --git a/netconn.go b/netconn.go index a854faa4..1dc1e79f 100644 --- a/netconn.go +++ b/netconn.go @@ -44,9 +44,12 @@ import ( // A received StatusNormalClosure or StatusGoingAway close frame will be translated to // io.EOF when reading. // -// Furthermore, the ReadLimit is set to -1 to disable it. +// If SetReadLimit has not been called on c, NetConn disables the read limit. +// A limit set with SetReadLimit before or after NetConn is kept. func NetConn(ctx context.Context, c *Conn, msgType MessageType) net.Conn { - c.SetReadLimit(-1) + if !c.readLimitSet.Load() { + c.setReadLimit(-1) + } nc := &netConn{ c: c, diff --git a/read.go b/read.go index 06672e10..70ae54e7 100644 --- a/read.go +++ b/read.go @@ -95,6 +95,11 @@ func (c *Conn) CloseRead(ctx context.Context) context.Context { // // Set to -1 to disable. func (c *Conn) SetReadLimit(n int64) { + c.readLimitSet.Store(true) + c.setReadLimit(n) +} + +func (c *Conn) setReadLimit(n int64) { if n >= 0 { // We read one more byte than the limit in case // there is a fin frame that needs to be read. diff --git a/ws_js.go b/ws_js.go index 026b75fc..880530b0 100644 --- a/ws_js.go +++ b/ws_js.go @@ -46,6 +46,7 @@ type Conn struct { // read limit for a message in bytes. msgReadLimit atomic.Int64 + readLimitSet atomic.Bool closeReadMu sync.Mutex closeReadCtx context.Context @@ -414,6 +415,11 @@ func (c *Conn) CloseRead(ctx context.Context) context.Context { // SetReadLimit implements *Conn.SetReadLimit for wasm. func (c *Conn) SetReadLimit(n int64) { + c.readLimitSet.Store(true) + c.setReadLimit(n) +} + +func (c *Conn) setReadLimit(n int64) { c.msgReadLimit.Store(n) } From eeacc7faff537124a3b58311adf10f6bc68a09d5 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 Oct 2026 17:58:31 +0000 Subject: [PATCH 02/10] test: cover the NetConn read limit rule on Wasm The Wasm Conn tracks SetReadLimit calls separately from the native Conn, so the same four cases run in the browser against the echo server. --- ws_js_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/ws_js_test.go b/ws_js_test.go index 1fa242f4..180f8957 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -2,6 +2,8 @@ package websocket_test import ( "context" + "io" + "net" "net/http" "os" "testing" @@ -10,6 +12,7 @@ import ( "github.com/coder/websocket" "github.com/coder/websocket/internal/test/assert" "github.com/coder/websocket/internal/test/wstest" + "github.com/coder/websocket/internal/test/xrand" ) func TestWasm(t *testing.T) { @@ -37,6 +40,62 @@ func TestWasm(t *testing.T) { assert.Success(t, err) } +func TestWasmNetConnReadLimit(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + netConn func(ctx context.Context, c *websocket.Conn) net.Conn + msgSize int + wantErr error + }{ + {"Unset", func(ctx context.Context, c *websocket.Conn) net.Conn { + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 65536, nil}, + {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(-1) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 65536, nil}, + {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(1024) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 4096, websocket.ErrMessageTooBig}, + {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + nc := websocket.NetConn(ctx, c, websocket.MessageBinary) + c.SetReadLimit(1024) + return nc + }, 4096, websocket.ErrMessageTooBig}, + } { + t.Run(tc.name, func(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 echo server expects StatusNormalClosure. + defer c.Close(websocket.StatusNormalClosure, "") + + nc := tc.netConn(ctx, c) + msg := []byte(xrand.String(tc.msgSize)) + _, err = nc.Write(msg) + assert.Success(t, err) + + got := make([]byte, len(msg)) + _, err = io.ReadFull(nc, got) + if tc.wantErr != nil { + assert.ErrorIs(t, tc.wantErr, err) + return + } + assert.Success(t, err) + assert.Equal(t, "read msg", msg, got) + }) + } +} + func TestWasmDialTimeout(t *testing.T) { t.Parallel() From 125d60d8d4969b958420db14b729c854f96dfee3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 Oct 2026 18:02:59 +0000 Subject: [PATCH 03/10] test: move the Wasm NetConn read limit test to its own file The fix-close-race-net-errclosed branch adds a test at the same place in ws_js_test.go, and the two insertions conflict on merge. --- netconn_js_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++ ws_js_test.go | 59 -------------------------------------- 2 files changed, 70 insertions(+), 59 deletions(-) create mode 100644 netconn_js_test.go diff --git a/netconn_js_test.go b/netconn_js_test.go new file mode 100644 index 00000000..046162ca --- /dev/null +++ b/netconn_js_test.go @@ -0,0 +1,70 @@ +package websocket_test + +import ( + "context" + "io" + "net" + "os" + "testing" + "time" + + "github.com/coder/websocket" + "github.com/coder/websocket/internal/test/assert" + "github.com/coder/websocket/internal/test/xrand" +) + +func TestWasmNetConnReadLimit(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + netConn func(ctx context.Context, c *websocket.Conn) net.Conn + msgSize int + wantErr error + }{ + {"Unset", func(ctx context.Context, c *websocket.Conn) net.Conn { + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 65536, nil}, + {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(-1) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 65536, nil}, + {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(1024) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 4096, websocket.ErrMessageTooBig}, + {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + nc := websocket.NetConn(ctx, c, websocket.MessageBinary) + c.SetReadLimit(1024) + return nc + }, 4096, websocket.ErrMessageTooBig}, + } { + t.Run(tc.name, func(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 echo server expects StatusNormalClosure. + defer c.Close(websocket.StatusNormalClosure, "") + + nc := tc.netConn(ctx, c) + msg := []byte(xrand.String(tc.msgSize)) + _, err = nc.Write(msg) + assert.Success(t, err) + + got := make([]byte, len(msg)) + _, err = io.ReadFull(nc, got) + if tc.wantErr != nil { + assert.ErrorIs(t, tc.wantErr, err) + return + } + assert.Success(t, err) + assert.Equal(t, "read msg", msg, got) + }) + } +} diff --git a/ws_js_test.go b/ws_js_test.go index 180f8957..1fa242f4 100644 --- a/ws_js_test.go +++ b/ws_js_test.go @@ -2,8 +2,6 @@ package websocket_test import ( "context" - "io" - "net" "net/http" "os" "testing" @@ -12,7 +10,6 @@ import ( "github.com/coder/websocket" "github.com/coder/websocket/internal/test/assert" "github.com/coder/websocket/internal/test/wstest" - "github.com/coder/websocket/internal/test/xrand" ) func TestWasm(t *testing.T) { @@ -40,62 +37,6 @@ func TestWasm(t *testing.T) { assert.Success(t, err) } -func TestWasmNetConnReadLimit(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - name string - netConn func(ctx context.Context, c *websocket.Conn) net.Conn - msgSize int - wantErr error - }{ - {"Unset", func(ctx context.Context, c *websocket.Conn) net.Conn { - return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 65536, nil}, - {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(-1) - return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 65536, nil}, - {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(1024) - return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 4096, websocket.ErrMessageTooBig}, - {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - nc := websocket.NetConn(ctx, c, websocket.MessageBinary) - c.SetReadLimit(1024) - return nc - }, 4096, websocket.ErrMessageTooBig}, - } { - t.Run(tc.name, func(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 echo server expects StatusNormalClosure. - defer c.Close(websocket.StatusNormalClosure, "") - - nc := tc.netConn(ctx, c) - msg := []byte(xrand.String(tc.msgSize)) - _, err = nc.Write(msg) - assert.Success(t, err) - - got := make([]byte, len(msg)) - _, err = io.ReadFull(nc, got) - if tc.wantErr != nil { - assert.ErrorIs(t, tc.wantErr, err) - return - } - assert.Success(t, err) - assert.Equal(t, "read msg", msg, got) - }) - } -} - func TestWasmDialTimeout(t *testing.T) { t.Parallel() From cb74ece28b7c6da8ebc12dba72b9ca86309d5976 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:40:43 +0000 Subject: [PATCH 04/10] docs(netconn.go): say a kept read limit applies per message A net.Conn caller reads a byte stream and cannot tell that a kept read limit counts each WebSocket message separately. Before the read limit was kept, NetConn disabled it, so the doc did not need to say this. --- netconn.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/netconn.go b/netconn.go index 1dc1e79f..554ad90b 100644 --- a/netconn.go +++ b/netconn.go @@ -45,7 +45,9 @@ import ( // io.EOF when reading. // // If SetReadLimit has not been called on c, NetConn disables the read limit. -// A limit set with SetReadLimit before or after NetConn is kept. +// A limit set with SetReadLimit before or after NetConn is kept. It applies +// to each WebSocket message, such as each Write by a peer using NetConn. +// Reading a message that exceeds it returns an error wrapping ErrMessageTooBig. func NetConn(ctx context.Context, c *Conn, msgType MessageType) net.Conn { if !c.readLimitSet.Load() { c.setReadLimit(-1) From 598dd5b640bae3b63810ef23890e5fd04f895b1c Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:41:46 +0000 Subject: [PATCH 05/10] test: fold the disabled NetConn read limit case into the table The standalone test repeated netConn/readLimit with one added SetReadLimit(-1) call. As a table case with an expected result, it sits next to the cases it contrasts with, matching the Wasm test. --- conn_test.go | 51 +++++++++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/conn_test.go b/conn_test.go index 10405a70..2f380c35 100644 --- a/conn_test.go +++ b/conn_test.go @@ -319,16 +319,22 @@ func TestConn(t *testing.T) { for _, tc := range []struct { name string setLimit func(ctx context.Context, c *websocket.Conn) net.Conn + msgSize int + wantErr error }{ + {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(-1) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, 65536, nil}, {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { c.SetReadLimit(1024) return websocket.NetConn(ctx, c, websocket.MessageBinary) - }}, + }, 4096, websocket.ErrMessageTooBig}, {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { nc := websocket.NetConn(ctx, c, websocket.MessageBinary) c.SetReadLimit(1024) return nc - }}, + }, 4096, websocket.ErrMessageTooBig}, } { t.Run("netConn/readLimitKept/"+tc.name, func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) @@ -336,39 +342,24 @@ func TestConn(t *testing.T) { n1 := tc.setLimit(tt.ctx, c1) _ = c2.CloseRead(tt.ctx) + msg := xrand.Bytes(tc.msgSize) writeDone := xsync.Go(func() error { - return c2.Write(tt.ctx, websocket.MessageBinary, make([]byte, 4096)) + return c2.Write(tt.ctx, websocket.MessageBinary, msg) }) - _, err := io.ReadFull(n1, make([]byte, 4096)) - assert.ErrorIs(t, websocket.ErrMessageTooBig, err) - - _ = c2.CloseNow() - <-writeDone - }) - } - - t.Run("netConn/readLimitDisabledBeforeNetConn", func(t *testing.T) { - tt, c1, c2 := newConnTest(t, nil, nil) - - c1.SetReadLimit(-1) - n1 := websocket.NetConn(tt.ctx, c1, websocket.MessageBinary) - n2 := websocket.NetConn(tt.ctx, c2, websocket.MessageBinary) - - s := strings.Repeat("papa", 1<<20) - errs := xsync.Go(func() error { - _, err := n2.Write([]byte(s)) - if err != nil { - return err + got := make([]byte, len(msg)) + _, err := io.ReadFull(n1, got) + if tc.wantErr != nil { + assert.ErrorIs(t, tc.wantErr, err) + _ = c2.CloseNow() + <-writeDone + return } - return n2.Close() + assert.Success(t, err) + assert.Success(t, <-writeDone) + assert.Equal(t, "read msg", msg, got) }) - - b, err := io.ReadAll(n1) - assert.Success(t, err) - assert.Success(t, <-errs) - assert.Equal(t, "read msg", s, string(b)) - }) + } t.Run("netConn/pastDeadline", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) From 01daeff300b452b0a6816cce0f1aba86207afca6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:42:12 +0000 Subject: [PATCH 06/10] test: name the NetConn read limit test message sizes Each message size must exceed a limit defined elsewhere, and a bare literal hides which one. Naming the sizes by the limit they exceed shows which cases stop testing anything if a default or limit changes. --- conn_test.go | 16 +++++++++++----- netconn_js_test.go | 18 ++++++++++++------ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/conn_test.go b/conn_test.go index 2f380c35..196c9f36 100644 --- a/conn_test.go +++ b/conn_test.go @@ -316,6 +316,12 @@ func TestConn(t *testing.T) { assert.Equal(t, "read msg", s, string(b)) }) + const ( + readLimit = 1024 + overReadLimit = 4 * readLimit + overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. + ) + for _, tc := range []struct { name string setLimit func(ctx context.Context, c *websocket.Conn) net.Conn @@ -325,16 +331,16 @@ func TestConn(t *testing.T) { {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { c.SetReadLimit(-1) return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 65536, nil}, + }, overDefaultLimit, nil}, {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(1024) + c.SetReadLimit(readLimit) return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 4096, websocket.ErrMessageTooBig}, + }, overReadLimit, websocket.ErrMessageTooBig}, {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { nc := websocket.NetConn(ctx, c, websocket.MessageBinary) - c.SetReadLimit(1024) + c.SetReadLimit(readLimit) return nc - }, 4096, websocket.ErrMessageTooBig}, + }, overReadLimit, websocket.ErrMessageTooBig}, } { t.Run("netConn/readLimitKept/"+tc.name, func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) diff --git a/netconn_js_test.go b/netconn_js_test.go index 046162ca..d4cffccb 100644 --- a/netconn_js_test.go +++ b/netconn_js_test.go @@ -16,6 +16,12 @@ import ( func TestWasmNetConnReadLimit(t *testing.T) { t.Parallel() + const ( + readLimit = 1024 + overReadLimit = 4 * readLimit + overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. + ) + for _, tc := range []struct { name string netConn func(ctx context.Context, c *websocket.Conn) net.Conn @@ -24,20 +30,20 @@ func TestWasmNetConnReadLimit(t *testing.T) { }{ {"Unset", func(ctx context.Context, c *websocket.Conn) net.Conn { return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 65536, nil}, + }, overDefaultLimit, nil}, {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { c.SetReadLimit(-1) return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 65536, nil}, + }, overDefaultLimit, nil}, {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(1024) + c.SetReadLimit(readLimit) return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, 4096, websocket.ErrMessageTooBig}, + }, overReadLimit, websocket.ErrMessageTooBig}, {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { nc := websocket.NetConn(ctx, c, websocket.MessageBinary) - c.SetReadLimit(1024) + c.SetReadLimit(readLimit) return nc - }, 4096, websocket.ErrMessageTooBig}, + }, overReadLimit, websocket.ErrMessageTooBig}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() From ea90a58a4347555d6ee5139d5f4117044d2ea664 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:57:40 +0000 Subject: [PATCH 07/10] test: name the NetConn read limit cases the same on both platforms The native closure was named setLimit although it also creates the net.Conn, and the native cases ran as TestConn subtests under different names than the Wasm cases. A top-level TestNetConnReadLimit with a netConn field matches TestWasmNetConnReadLimit case for case. --- conn_test.go | 106 ++++++++++++++++++++++++++------------------------- 1 file changed, 55 insertions(+), 51 deletions(-) diff --git a/conn_test.go b/conn_test.go index 196c9f36..ea9200ff 100644 --- a/conn_test.go +++ b/conn_test.go @@ -316,57 +316,6 @@ func TestConn(t *testing.T) { assert.Equal(t, "read msg", s, string(b)) }) - const ( - readLimit = 1024 - overReadLimit = 4 * readLimit - overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. - ) - - for _, tc := range []struct { - name string - setLimit func(ctx context.Context, c *websocket.Conn) net.Conn - msgSize int - wantErr error - }{ - {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(-1) - return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, overDefaultLimit, nil}, - {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - c.SetReadLimit(readLimit) - return websocket.NetConn(ctx, c, websocket.MessageBinary) - }, overReadLimit, websocket.ErrMessageTooBig}, - {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { - nc := websocket.NetConn(ctx, c, websocket.MessageBinary) - c.SetReadLimit(readLimit) - return nc - }, overReadLimit, websocket.ErrMessageTooBig}, - } { - t.Run("netConn/readLimitKept/"+tc.name, func(t *testing.T) { - tt, c1, c2 := newConnTest(t, nil, nil) - - n1 := tc.setLimit(tt.ctx, c1) - _ = c2.CloseRead(tt.ctx) - - msg := xrand.Bytes(tc.msgSize) - writeDone := xsync.Go(func() error { - return c2.Write(tt.ctx, websocket.MessageBinary, msg) - }) - - got := make([]byte, len(msg)) - _, err := io.ReadFull(n1, got) - if tc.wantErr != nil { - assert.ErrorIs(t, tc.wantErr, err) - _ = c2.CloseNow() - <-writeDone - return - } - assert.Success(t, err) - assert.Success(t, <-writeDone) - assert.Equal(t, "read msg", msg, got) - }) - } - t.Run("netConn/pastDeadline", func(t *testing.T) { tt, c1, c2 := newConnTest(t, nil, nil) @@ -493,6 +442,61 @@ func TestConn(t *testing.T) { }) } +func TestNetConnReadLimit(t *testing.T) { + t.Parallel() + + const ( + readLimit = 1024 + overReadLimit = 4 * readLimit + overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. + ) + + for _, tc := range []struct { + name string + netConn func(ctx context.Context, c *websocket.Conn) net.Conn + msgSize int + wantErr error + }{ + {"DisabledBeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(-1) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, overDefaultLimit, nil}, + {"BeforeNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + c.SetReadLimit(readLimit) + return websocket.NetConn(ctx, c, websocket.MessageBinary) + }, overReadLimit, websocket.ErrMessageTooBig}, + {"AfterNetConn", func(ctx context.Context, c *websocket.Conn) net.Conn { + nc := websocket.NetConn(ctx, c, websocket.MessageBinary) + c.SetReadLimit(readLimit) + return nc + }, overReadLimit, websocket.ErrMessageTooBig}, + } { + t.Run(tc.name, func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + n1 := tc.netConn(tt.ctx, c1) + _ = c2.CloseRead(tt.ctx) + + msg := xrand.Bytes(tc.msgSize) + writeDone := xsync.Go(func() error { + return c2.Write(tt.ctx, websocket.MessageBinary, msg) + }) + + got := make([]byte, len(msg)) + _, err := io.ReadFull(n1, got) + if tc.wantErr != nil { + assert.ErrorIs(t, tc.wantErr, err) + _ = c2.CloseNow() + <-writeDone + return + } + assert.Success(t, err) + assert.Success(t, <-writeDone) + assert.Equal(t, "read msg", msg, got) + }) + } +} + func TestWasm(t *testing.T) { t.Parallel() if os.Getenv("CI") == "" { From 32c8a81c9e12382743adca254d93e994f9088bf6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:58:55 +0000 Subject: [PATCH 08/10] test: check the default read limit the NetConn cases rely on The cases that expect NetConn to read without error only test something while their message exceeds the default read limit. A control case that reads the same size without NetConn fails if the default is raised to that size, instead of letting those cases pass without a limit to disable. --- conn_test.go | 17 +++++++++++++++++ netconn_js_test.go | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/conn_test.go b/conn_test.go index ea9200ff..14052135 100644 --- a/conn_test.go +++ b/conn_test.go @@ -451,6 +451,23 @@ func TestNetConnReadLimit(t *testing.T) { overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. ) + // A default at or above overDefaultLimit fails this case instead of letting + // the cases that expect no error pass without a limit to disable. + t.Run("DefaultWithoutNetConn", func(t *testing.T) { + tt, c1, c2 := newConnTest(t, nil, nil) + + _ = c2.CloseRead(tt.ctx) + writeDone := xsync.Go(func() error { + return c2.Write(tt.ctx, websocket.MessageBinary, xrand.Bytes(overDefaultLimit)) + }) + + _, _, err := c1.Read(tt.ctx) + assert.ErrorIs(t, websocket.ErrMessageTooBig, err) + + _ = c2.CloseNow() + <-writeDone + }) + for _, tc := range []struct { name string netConn func(ctx context.Context, c *websocket.Conn) net.Conn diff --git a/netconn_js_test.go b/netconn_js_test.go index d4cffccb..5e5ef1be 100644 --- a/netconn_js_test.go +++ b/netconn_js_test.go @@ -22,6 +22,27 @@ func TestWasmNetConnReadLimit(t *testing.T) { overDefaultLimit = 2 * 32768 // SetReadLimit documents the 32768 byte default. ) + // A default at or above overDefaultLimit fails this case instead of letting + // the cases that expect no error pass without a limit to disable. + t.Run("DefaultWithoutNetConn", func(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) + defer c.Close(websocket.StatusNormalClosure, "") + + err = c.Write(ctx, websocket.MessageBinary, xrand.Bytes(overDefaultLimit)) + assert.Success(t, err) + + _, _, err = c.Read(ctx) + assert.ErrorIs(t, websocket.ErrMessageTooBig, err) + }) + for _, tc := range []struct { name string netConn func(ctx context.Context, c *websocket.Conn) net.Conn From 69b18fe1b612954485b0d4c52629aa5729331972 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 20:49:28 +0000 Subject: [PATCH 09/10] fix: prevent NetConn from overwriting a concurrent read limit NetConn checked readLimitSet and then disabled the limit, so a SetReadLimit that ran between the two steps was overwritten and left the connection without a read limit. A mutex around both operations makes the check and the update atomic. --- conn.go | 6 +++++- netconn.go | 4 +++- read.go | 4 +++- ws_js.go | 10 ++++++++-- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/conn.go b/conn.go index 1df92623..0a3377f7 100644 --- a/conn.go +++ b/conn.go @@ -59,7 +59,11 @@ type Conn struct { readHeaderBuf [8]byte readControlBuf [maxControlPayload]byte msgReader *msgReader - readLimitSet atomic.Bool + + // readLimitMu makes NetConn's readLimitSet check atomic with SetReadLimit. + // It is never held during I/O, so it need not be the context-aware mu. + readLimitMu sync.Mutex + readLimitSet bool // Write state. msgWriter *msgWriter diff --git a/netconn.go b/netconn.go index 554ad90b..f1b46ba7 100644 --- a/netconn.go +++ b/netconn.go @@ -49,9 +49,11 @@ import ( // to each WebSocket message, such as each Write by a peer using NetConn. // Reading a message that exceeds it returns an error wrapping ErrMessageTooBig. func NetConn(ctx context.Context, c *Conn, msgType MessageType) net.Conn { - if !c.readLimitSet.Load() { + c.readLimitMu.Lock() + if !c.readLimitSet { c.setReadLimit(-1) } + c.readLimitMu.Unlock() nc := &netConn{ c: c, diff --git a/read.go b/read.go index 70ae54e7..c4417160 100644 --- a/read.go +++ b/read.go @@ -95,7 +95,9 @@ func (c *Conn) CloseRead(ctx context.Context) context.Context { // // Set to -1 to disable. func (c *Conn) SetReadLimit(n int64) { - c.readLimitSet.Store(true) + c.readLimitMu.Lock() + defer c.readLimitMu.Unlock() + c.readLimitSet = true c.setReadLimit(n) } diff --git a/ws_js.go b/ws_js.go index 880530b0..ab7499bd 100644 --- a/ws_js.go +++ b/ws_js.go @@ -46,7 +46,11 @@ type Conn struct { // read limit for a message in bytes. msgReadLimit atomic.Int64 - readLimitSet atomic.Bool + + // readLimitMu makes NetConn's readLimitSet check atomic with SetReadLimit. + // It is never held during I/O, so it need not be the context-aware mu. + readLimitMu sync.Mutex + readLimitSet bool closeReadMu sync.Mutex closeReadCtx context.Context @@ -415,7 +419,9 @@ func (c *Conn) CloseRead(ctx context.Context) context.Context { // SetReadLimit implements *Conn.SetReadLimit for wasm. func (c *Conn) SetReadLimit(n int64) { - c.readLimitSet.Store(true) + c.readLimitMu.Lock() + defer c.readLimitMu.Unlock() + c.readLimitSet = true c.setReadLimit(n) } From bc04f4b1f104ac74fc6fd175a9563e09402c42a3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 21:02:33 +0000 Subject: [PATCH 10/10] docs: say what readLimitSet means on both Conn types The field comment described only readLimitMu. A reader also needs to know that only SetReadLimit sets readLimitSet, because NetConn disables a limit set through setReadLimit alone. --- conn.go | 5 +++-- ws_js.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/conn.go b/conn.go index 0a3377f7..7c7c945b 100644 --- a/conn.go +++ b/conn.go @@ -60,8 +60,9 @@ type Conn struct { readControlBuf [maxControlPayload]byte msgReader *msgReader - // readLimitMu makes NetConn's readLimitSet check atomic with SetReadLimit. - // It is never held during I/O, so it need not be the context-aware mu. + // readLimitSet is true once SetReadLimit has been called, so NetConn + // keeps the limit. readLimitMu makes NetConn's check and update atomic + // with SetReadLimit. It is never held during I/O. readLimitMu sync.Mutex readLimitSet bool diff --git a/ws_js.go b/ws_js.go index ab7499bd..04ff527c 100644 --- a/ws_js.go +++ b/ws_js.go @@ -47,8 +47,9 @@ type Conn struct { // read limit for a message in bytes. msgReadLimit atomic.Int64 - // readLimitMu makes NetConn's readLimitSet check atomic with SetReadLimit. - // It is never held during I/O, so it need not be the context-aware mu. + // readLimitSet is true once SetReadLimit has been called, so NetConn + // keeps the limit. readLimitMu makes NetConn's check and update atomic + // with SetReadLimit. It is never held during I/O. readLimitMu sync.Mutex readLimitSet bool