Skip to content

transport: answer WS pings and read the control payload out of the stream - #326

Open
Mliviu79 wants to merge 1 commit into
emiago:mainfrom
Mliviu79:pr/ws-control-frames
Open

transport: answer WS pings and read the control payload out of the stream#326
Mliviu79 wants to merge 1 commit into
emiago:mainfrom
Mliviu79:pr/ws-control-frames

Conversation

@Mliviu79

Copy link
Copy Markdown
Contributor

Fixes #325

A Ping carrying a payload left those bytes in the stream, so the next header read started mid payload and the frame after the ping did not decode. transport_ws.go:409 had the Ping arm commented out.

Control frames now go through wsutil.ControlHandler, which answers a Ping with a Pong carrying the same payload (RFC 6455 section 5.5.2), discards a Pong, and reads the payload out of the stream either way.

That introduces a second writer on the connection. ws writes a frame header and payload separately, so a Pong from the read goroutine can interleave with a concurrent WriteMsg and neither frame decodes. All frame writes now go through writeFrame under a mutex, and the lock is held across the control handler.

Tests cover a ping with a payload (asserting the following text frame still decodes), the empty-ping fast path, and 8 concurrent writers proving frames stay intact — the last one is meaningful under -race.

…he stream

A Ping carrying a payload left those bytes in the stream, so the next header
read started mid payload and the following frame did not decode. Control frames
are now handled with wsutil.ControlHandler, which answers a Ping with a Pong
carrying the same payload, discards a Pong, and reads the payload out either
way. RFC 6455 section 5.5.

ws writes a frame header and payload separately, so a Pong written from the read
goroutine can interleave with a concurrent WriteMsg and neither frame decodes.
All frame writes now go through writeFrame under a mutex, and the write lock is
held across the control handler.
Refs emiago#325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sip: WS ping with a payload desyncs the frame stream

1 participant