Skip to content

Commit 3c58a2a

Browse files
Marko Petzoldclaude
andcommitted
transport/rawsocket: abort on deserialize error per WAMP §5.3.1
Changes recvHandler's malformed-payload path from `continue MsgLoop` (silently skip the bad frame and keep reading) to log + conn.Close() + return — the same teardown shape as the io.ReadFull error path above. Per WAMP §5.3.1 a malformed payload is a protocol violation and the receiver MUST abort. The original code carried a `// TODO: something more than merely logging?` comment hinting this was permissive. Senders that need fault-tolerance for occasional malformed frames must reconnect. Updates TestRawSocketC5_DeserializerErrorAborts on pr/rawsocket-frame-edges to assert the new strict semantic (Recv channel closes after the malformed frame). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6eca374 commit 3c58a2a

2 files changed

Lines changed: 24 additions & 26 deletions

File tree

transport/rawsocketpeer.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,13 @@ MsgLoop:
302302
}
303303
msg, err = rs.serializer.Deserialize(buf)
304304
if err != nil {
305-
// TODO: something more than merely logging?
305+
// A malformed payload is a protocol violation per
306+
// WAMP §5.3.1; abort the connection rather than
307+
// silently skipping the frame. Same teardown shape
308+
// as the io.ReadFull error path above.
306309
rs.log.Println("Cannot deserialize peer message:", err)
307-
continue MsgLoop
310+
_ = rs.conn.Close()
311+
return
308312
}
309313
case 1: // PING
310314
header[0] = 0x02

transport/rawsocketpeer_test.go

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -157,39 +157,33 @@ func TestRawSocketC2_TruncatedFrame(t *testing.T) {
157157
}
158158
}
159159

160-
// TestRawSocketC5_DeserializerError pins the current behavior:
161-
// when a frame's body is well-framed but its payload fails
162-
// deserialization, recvHandler logs the error and *continues* the
163-
// MsgLoop (does not abort the connection). A subsequent valid frame
164-
// is delivered normally.
160+
// TestRawSocketC5_DeserializerErrorAborts pins WAMP §5.3.1 strict
161+
// behavior: when a frame's body is well-framed but its payload fails
162+
// deserialization, recvHandler logs the error, closes the connection,
163+
// and exits — same teardown as a torn read. The peer's Recv channel
164+
// closes; no further frames are processed even if more arrive on the
165+
// wire.
165166
//
166-
// WAMP §5.3.1 calls protocol violations a hard-error condition that
167-
// SHOULD trigger an ABORT. nexus's permissive behavior is at odds
168-
// with that — pinning it here makes any future strict-mode change
169-
// visible.
170-
func TestRawSocketC5_DeserializerError(t *testing.T) {
167+
// Prior to the recvHandler fix in this commit, recvHandler silently
168+
// skipped the bad frame (`continue MsgLoop`) and kept reading —
169+
// permissive but at odds with the spec.
170+
func TestRawSocketC5_DeserializerErrorAborts(t *testing.T) {
171171
client, peer := newPipedRawSocket(t, 4) // 2^13 = 8192
172172
t.Cleanup(func() { peer.Close() })
173173

174-
// First frame: body is `not valid json {` — well-framed but
175-
// fails JSON deserialization.
174+
// First (and only) frame the server processes: body is
175+
// `not valid json {` — well-framed but fails JSON
176+
// deserialization.
176177
writeFrame(t, client, 0x00, []byte("not valid json {"))
177178

178-
// Second frame: a valid HELLO message in JSON form.
179-
hello := wamp.Hello{Realm: "test", Details: wamp.Dict{}}
180-
helloBytes, err := (&serialize.JSONSerializer{}).Serialize(&hello)
181-
require.NoError(t, err)
182-
writeFrame(t, client, 0x00, helloBytes)
183-
184-
// peer.Recv() should yield the HELLO. The bad first frame is
185-
// silently skipped per current behavior.
179+
// peer.Recv() must close (recvHandler returned via the new
180+
// abort-on-deserialize-error path). A subsequent frame is
181+
// irrelevant — we already proved recvHandler stopped.
186182
select {
187183
case msg, ok := <-peer.Recv():
188-
require.True(t, ok, "Recv closed unexpectedly")
189-
_, isHello := msg.(*wamp.Hello)
190-
require.True(t, isHello, "expected HELLO after deserializer error skip, got %T", msg)
184+
require.False(t, ok, "expected closed Recv after malformed frame, got msg %v", msg)
191185
case <-time.After(time.Second):
192-
require.FailNow(t, "peer.Recv() did not deliver post-error HELLO")
186+
require.FailNow(t, "peer.Recv() did not close after malformed frame")
193187
}
194188
_ = client.Close()
195189
}

0 commit comments

Comments
 (0)