Skip to content

test(ws): walk the frames instead of sampling byte zero - #304

Merged
EdmondDantes merged 1 commit into
mainfrom
ws-035-walk-the-frames
Aug 26, 2026
Merged

EdmondDantes merged 1 commit into
mainfrom
ws-035-walk-the-frames

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

Two framing defects on the client side of the WebSocket tests, and one server-side finding that neither of them explains.

035-recv-queue-overflow read the socket into a single buffer and checked for the close opcode at byte 0, so anything the server sent ahead of the close left it reading a data frame's header as a control frame and never finding the close. It now walks frames.

ws_read_frame in _ws_client.inc dropped a partially read header: ws_take($fp, 2, false) returns whatever arrived, the caller threw away a single byte and returned null, and every frame after such a read parsed as garbage. ws_pushback exists for exactly this and was not used. A torn read is rare on loopback and likelier on Windows, so this had room to hide.

Neither fixes the test on Windows, and I am not claiming it does. It still reads client saw close: NULL while the handler side reports 1013, which means the decision is right and the frame does not arrive. The shape is the one #287 named for HTTP/1: the client writes about 16 KiB, the server caps the inbound queue at 8 KiB, stops reading, queues the close and tears the transport down at ws_session.c:1345 — and a socket closed with unread bytes in its receive buffer is reset rather than finished, which discards what this side has written. #288 gave HTTP/1 a lingering close for that; the WebSocket overflow path has none.

That is recorded as an open item in dev/PLAN.md, with the cheap proof named: cut the client's write to just past the cap so nothing is left unread, and see whether the close arrives.

Evidence for what is here: the whole WebSocket group on Windows is 60 of 61 executed tests passing either way — these two changes remove latent misreads, not the failure.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Coverage

Total lines: 83.19% → 83.16% (-0.03 pp)

File Baseline Current Δ Touched
src/core/http_connection_tls.c 73.55% 73.83% +0.28 pp
src/http1/http_parser.c 84.43% 83.30% -1.13 pp
src/http3/http3_io.c 89.45% 89.09% -0.36 pp
src/http_server_class.c 75.93% 75.97% +0.04 pp
src/websocket/ws_session.c 90.22% 89.67% -0.55 pp

`035-recv-queue-overflow` read the socket into one buffer and looked for
the close opcode at its first byte, so anything the server sent ahead of
the close left it parsing a data frame's header as a control frame. And
`ws_read_frame` dropped a header it read only half of: `ws_take` returns
what it got, the caller discarded it, and every frame after a torn read
came out as garbage — `ws_pushback` exists for that and was not used.

Neither is why the test fails on Windows. It still reads `NULL` there
while the handler reports 1013, and the reason is in the server: the
overflow path tears the transport down with the peer's bytes still
unread, which resets the connection and discards the close it had
queued. That is the shape #288 gave HTTP/1 a lingering close for, and
the WebSocket path has none. Recorded in dev/PLAN.md as open.
@EdmondDantes
EdmondDantes force-pushed the ws-035-walk-the-frames branch from 677446d to 77c4c7b Compare August 26, 2026 05:48
@EdmondDantes
EdmondDantes merged commit 18f1c5c into main Aug 26, 2026
7 of 8 checks passed
@EdmondDantes
EdmondDantes deleted the ws-035-walk-the-frames branch August 26, 2026 06:08
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.

1 participant