[oss-candidate] fix(websocket): reset the compressed flag when a message completes - #1

Summary

• ByteParser never cleared #info.compressed after delivering a permessage-deflate message, so the flag described the previous message while the next frame header was parsed.
• The guard at lib/web/websocket/receiver.js:171 that rejects a continuation frame with no message in progress is conditioned on !this.#info.compressed, so it was skipped after any compressed message.
• Result: a stray continuation frame arriving after a compressed message is decompressed and delivered to the application as an extra message, instead of failing the connection with 1002 as RFC 6455 §5.4 requires.
• Fix: clear the flag in consumeFragments(), the one place where the message the flag describes is completed (both websocketMessageReceived call sites go through it).
• 8 regression tests added to test/websocket/receiver-unit.js (plus 1 pre-existing test in the file, 9 total): 4 discriminate on this bug and 4 are declared controls.

Both arms below were measured at head 998d2a463b8c348f1c3029ba728a818e3bb423c7, with all 9 tests present in the file and only lib/web/websocket/receiver.js swapped between arms.

$ git checkout origin/main -- lib/web/websocket/receiver.js   # base arm: fix reverted, tests kept
$ node --test test/websocket/receiver-unit.js
✔ ByteParser rejects 64-bit payload lengths with a non-zero upper word (27.118782ms)
✖ ByteParser fails the connection on a continuation frame after a compressed message (119.98587ms)
✔ ByteParser fails the connection on a continuation frame after an uncompressed message (control) (105.086991ms)
✔ ByteParser still assembles a fragmented compressed message (control) (106.584768ms)
✖ ByteParser fails the connection on a continuation frame after a multi-fragment compressed message (157.444873ms)
✖ ByteParser fails the connection on a continuation frame separated from a compressed message by a control frame (155.700874ms)
✖ ByteParser fails the connection on a non-empty continuation frame after a compressed message (105.319193ms)
✔ ByteParser delivers two consecutive compressed messages (control) (106.370579ms)
✔ ByteParser keeps the compressed flag across a control frame inside a fragmented message (control) (154.134819ms)
ℹ tests 9
ℹ pass 5
ℹ fail 4
✖ failing tests:
✖ ByteParser fails the connection on a continuation frame after a compressed message (119.98587ms)
    actual: [ 'message:5', 'message:0' ],
    expected: [ 'message:5', 'abort' ],
✖ ByteParser fails the connection on a continuation frame after a multi-fragment compressed message (157.444873ms)
    actual: [ 'message:5', 'message:0' ],
    expected: [ 'message:5', 'abort' ],
✖ ByteParser fails the connection on a continuation frame separated from a compressed message by a control frame (155.700874ms)
    actual: [ 'message:5', 'ping', 'message:0' ],
    expected: [ 'message:5', 'ping', 'abort' ],
✖ ByteParser fails the connection on a non-empty continuation frame after a compressed message (105.319193ms)
    actual: [ 'message:5', 'message:5' ],
    expected: [ 'message:5', 'abort' ],

$ git checkout HEAD -- lib/web/websocket/receiver.js   # fixed arm, head 998d2a46
$ node --test test/websocket/receiver-unit.js
✔ ByteParser rejects 64-bit payload lengths with a non-zero upper word (17.284235ms)
✔ ByteParser fails the connection on a continuation frame after a compressed message (113.10889ms)
✔ ByteParser fails the connection on a continuation frame after an uncompressed message (control) (102.501077ms)
✔ ByteParser still assembles a fragmented compressed message (control) (104.682335ms)
✔ ByteParser fails the connection on a continuation frame after a multi-fragment compressed message (156.259085ms)
✔ ByteParser fails the connection on a continuation frame separated from a compressed message by a control frame (155.52651ms)
✔ ByteParser fails the connection on a non-empty continuation frame after a compressed message (108.682308ms)
✔ ByteParser delivers two consecutive compressed messages (control) (107.327427ms)
✔ ByteParser keeps the compressed flag across a control frame inside a fragmented message (control) (154.650232ms)
ℹ tests 9
ℹ pass 9
ℹ fail 0
ℹ duration_ms 1309.719693

Upstream

• Repository: nodejs/undici
• Default branch: main
• Base sha: 2a783e77dee2212366dc000e1fe0bd9fb6c919d4
• Files: lib/web/websocket/receiver.js (ByteParser.consumeFragments, +5 lines), test/websocket/receiver-unit.js (+227 lines, 8 new tests)
• Diff at head 998d2a46: 2 files changed, 232 insertions(+), 0 deletions(-)

Bug

When a permessage-deflate message is received, run() sets this.#info.compressed = rsv1 !== 0 at receiver.js:191 while parsing the frame header, and consumeFragments() hands the assembled message to the application. The flag is never reset, so it stays true until the next text or binary frame header overwrites it at line 191. A continuation frame (opcode 0x0) carries no such assignment, so between a completed compressed message and the next data frame, #info.compressed is stale.

The guard at receiver.js:171 reads isContinuationFrame(opcode) && this.#fragments.length === 0 && !this.#info.compressed. The !this.#info.compressed term exists because a compressed fragmented message legitimately drains #fragments on each decompressed chunk while remaining in progress. But with the flag stale, that same term suppresses the guard for a continuation frame that follows a completed message. The parser then treats the stray frame as message data, feeds it to the inflater, and emits a second message event. Trigger: any peer (or on-path attacker able to inject a frame) that sends a continuation frame after a compressed message on a connection where permessage-deflate was negotiated. Blast radius: every undici WebSocket client that negotiates permessage-deflate — the default when the server offers it — receives spurious application messages on a connection the spec requires to be failed. It is a protocol-conformance hole, and the extra message is attacker-chosen data delivered as though it were a real message.

Repro

repro.js at the repository root, driving ByteParser directly (no node_modules needed). Historical transcript, measured at the fix commit a3cf4645 against base 2a783e77 — the script is deliberately not part of the diff, and the identical scenario is pinned at the current head by the first regression test below, whose verbatim both-arm output is in ## Summary:

$ node repro.js   # on base 2a783e77
events: ["message(len=5)","message(len=0)"]
messages delivered: 2 (expected 1)
connection failed:  false (expected true)
RESULT: BUG — stray continuation frame accepted after a compressed message
exit=1

$ node repro.js   # with fix
events: ["message(len=5)","abort"]
messages delivered: 1 (expected 1)
connection failed:  true (expected true)
RESULT: OK — stray continuation frame rejected
exit=0

The repro writes one complete compressed text frame (FIN|RSV1, opcode 0x1, deflate-raw payload with the trailing 00 00 ff ff stripped) followed by a bare 80 00 continuation frame.

Fix

   consumeFragments () {
     const fragments = this.#fragments
 
+    // The compressed flag describes the message that is being completed here.
+    // It must not survive into the next message, otherwise a stray
+    // continuation frame is accepted instead of failing the connection.
+    this.#info.compressed = false
+
     if (fragments.length === 1) {

consumeFragments() is the correct place: it is called from exactly the two websocketMessageReceived sites (receiver.js:258 uncompressed, receiver.js:290 compressed), i.e. every point where a message completes, and it already owns the rest of the per-message state reset (#fragments, #fragmentsBytes). Clearing the flag there keeps the reset with the state it belongs to and costs one assignment per message.

Alternatives rejected:

• Clear the flag at the two websocketMessageReceived call sites. Same effect, duplicated in two places, and a future third completion path would miss it.
• Drop !this.#info.compressed from the guard at line 171. Reverts the fix from permessage-deflate decompression support in websocket nodejs/undici#3263 and breaks compressed fragmented messages, which drain #fragments while still in progress — the ...still assembles a fragmented compressed message (control) test is the control that would catch that.
• Track a separate "message in progress" boolean. Larger change, new state to keep in sync, no behaviour this one-line reset does not already give.

Test evidence

9 tests now live in test/websocket/receiver-unit.js: 1 pre-existing, 4 added with the fix (3 in a3cf4645, 1 in ffb1247b), and 4 added by the independent verification pass (998d2a46). All follow the file's existing unit-level style: construct ByteParser with a stub handler, no server. The shared createRecordingHandler() records every onMessage/onPing/abort/close the parser produces, so the assertions compare full event sequences rather than a single flag.

Verbatim base/fixed output for all 9 is in the ## Summary console block above.

What each control controls for, one thing each:

• T3 — the fragments.length === 0 term still fires when the flag was never set, so a future change cannot make the guard vacuous and still pass.
• T4 — legal compressed fragmentation, the reason the !compressed term exists at all, is not broken by the reset.
• T8 — the reverse direction: clearing the flag on completion must not stop the next frame from being read as compressed (two consecutive compressed messages both arrive intact).
• T9 — the flag must survive a control frame interleaved into a message still in progress; it pins that the reset happens at completion, not at any control frame. T4 and T9 differ in that only T9 interleaves a control frame mid-message.

The four discriminating tests differ in the code path that reaches the guard: T2 the single-fragment consumeFragments() shortcut, T5 the concatenating branch, T6 a control frame between the message and the stray continuation, T7 a stray continuation carrying a decompressible payload (proving the failure is on the opcode, not incidentally on a zlib error).

Linter: run and clean. eslint@9 + neostandard@0.13 (the linter eslint.config.js configures, matching the repo's npm run lint) were installed into a scratch directory and linked in, then:

$ npx eslint --no-config-lookup --config <neostandard> test/websocket/receiver-unit.js lib/web/websocket/receiver.js
$ echo $?
0

Zero errors, zero warnings on both touched files. The scratch node_modules symlink was removed afterwards; git status --porcelain is empty at head.

Verification method

executed — Node.js v24.19.0 on Linux, via node --test (the repo's test:websocket script uses borp, which needs node_modules; node --test runs the same file unmodified). Both arms were run at the final head by reverting only lib/web/websocket/receiver.js to origin/main with all 9 tests in place, then restoring with git checkout HEAD -- <file> (not a cp backup, which would leave the index dirty). Each arm was guarded by grep -c 'compressed = false' lib/web/websocket/receiver.js (0 = base, 1 = head) in the same shell call as the test run, so no arm could silently run the wrong source.

Two passes at two heads:

In both passes the base-arm passers are exactly the declared controls, and every non-control test fails on base.

Production source is byte-identical across both heads: git diff ffb1247b 998d2a46 -- lib/ is empty. The verification pass added tests only.

Head integrity measured, not inferred: git status --porcelain empty and git diff HEAD --stat empty after the final commit; git diff origin/main..HEAD --stat = 2 files, +232/-0.

Fork CI: gh pr checks 1 --repo askalf/undici → no checks reported on the 'fix/websocket-stale-compressed-flag' branch. GitHub Actions has never been enabled on askalf/undici, so this is an absence of a signal, not a failure — there are no non-green jobs to explain. Upstream CI, including the Autobahn conformance suite, which is the authoritative check for a protocol-conformance change like this one, has not run against this branch and remains outstanding.

Prior art

• gh search prs --repo nodejs/undici "continuation frame" --limit 15 → fix websocket empty fragments parsing nodejs/undici#5424 "fix websocket empty fragments parsing" (CLOSED, empty-fragment parsing, superseded by merged commit c5ed7875), feat: implement websockets nodejs/undici#1795 "feat: implement websockets" (merged, the original implementation). Neither touches the compressed-flag lifecycle.
• gh search prs --repo nodejs/undici "permessage-deflate" --limit 15 → open fix(websocket): validate inline payload with current opcode nodejs/undici#5432 "validate inline payload with current opcode" and open fix(websocket): enforce deflate remaining payload budget nodejs/undici#5435 "enforce deflate remaining payload budget". Both are maxPayloadSize accounting in the same file; neither resets or reads #info.compressed outside the existing sites. No conflict, no overlap.
• gh search prs --repo nodejs/undici "compressed flag reset" --limit 10 → [].
• gh search issues --repo nodejs/undici "continuation frame" --limit 15 → []; gh search issues --repo nodejs/undici "unexpected continuation frame" → [].
• git log -S'!this.#info.compressed' -- lib/web/websocket/receiver.js → 5a564bd9 "permessage-deflate decompression support in websocket (permessage-deflate decompression support in websocket nodejs/undici#3263)", which introduced the guard term. The stale-flag hole has been present since that commit.
• git log --oneline -12 -- lib/web/websocket/receiver.js → the last twelve changes are fragment/limit/overflow fixes (c5ed7875, 32dbf0b3, b4c287b3, 84235c62, …), i.e. exactly this bug class, and several are by the maintainer who owns this file.

Policy

• CONTRIBUTING.md, CODE_OF_CONDUCT.md and .github/PULL_REQUEST_TEMPLATE.md contain no mention of AI, LLM, generated or agent contributions — grep for ai|llm|generated|agent|copilot|chatgpt|claude returns only unrelated hits (deps/llhttp/...). Policy is silent, not a ban. There is no AGENTS.md and no .github/CONTRIBUTING*.
• DCO, not a CLA. .github/PULL_REQUEST_TEMPLATE.md: "You will also need to ensure that your contribution complies with the Developer's Certificate of Origin, outlined in CONTRIBUTING.md", with the checkbox "- [ ] I have read and agreed to the [Developer's Certificate of Origin][cert]". CONTRIBUTING.md:354: "Developer's Certificate of Origin 1.1 — By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file...". 13 of the last 30 upstream commits carry a Signed-off-by trailer, so it is not mechanically enforced — the operator decides whether to add -s when submitting; that certification is theirs to make, not ours.
• PR template requires: "1. Include tests for any bug fixes or new features. 2. Update documentation if relevant. 3. Ensure that tests and linting pass." Tests are included (9 in the touched file, all green at head); no documented behaviour changes, so no docs update; linting now runs clean — see ## Test evidence.
• No changelog or changeset file is required by the repo (releases are automated from main per CONTRIBUTING.md:170).

Disclosure facts for the operator

Plain facts, for you to word your own disclosure:

• An AI agent found this bug by reading lib/web/websocket/receiver.js and tracing the lifetime of #info.compressed against the guard at line 171. It was not reported by a user or taken from an issue.
• The AI wrote the one-line fix, the comment above it, and all 8 regression tests.
• A second, independent AI run adversarially verified the fix: it rebuilt the boundary ledger from the diff without reading the existing write-up, added 4 tests (2 of which fail on base and 2 of which are declared controls), and re-ran both arms. It found no defect in the fix. It did correct one claim in the earlier write-up: the row for "control frame between the compressed message and the stray continuation" had been dismissed as not varying with the diff, but it does vary — that case is now test T6 and it fails on base.
• The AI executed both arms (base and fixed) on Linux with Node.js v24.19.0 and captured the transcripts quoted above verbatim.
• The AI ran the repo's linter (eslint + neostandard) on both touched files: clean.
• The AI did not run the borp-based test:websocket script (the full websocket suite), and did not run the Autobahn conformance suite.
• Nothing was posted to nodejs/undici by the AI: no issues, comments, reviews or PRs. All work happened on the askalf/undici fork.
• The DCO sign-off, if you add one, is your certification — the AI did not add a Signed-off-by trailer.

Boundaries

One row per predicate, comparison or truthiness check the diff adds or changes. The diff adds a single unconditional assignment, so the rows below enumerate the state that assignment feeds — the guard at receiver.js:171, the branch at receiver.js:248, the branch inside consumeFragments(), and the two ByteParser construction sites. Ledger rebuilt from the diff by the verification pass; rows 5, 8, 9 and 12–14 were re-derived and their previous dispositions re-tested rather than inherited.

Suggested upstream PR title

fix(websocket): reset the compressed flag when a message completes