Skip to content

fix(client): reset no-body transfer-encoding responses - #126

Merged
ronag merged 1 commit into
nxtedition:masterfrom
ronag:codex/upstream-5941-no-body-transfer-encoding
Oct 8, 2026
Merged

ronag merged 1 commit into
nxtedition:masterfrom
ronag:codex/upstream-5941-no-body-transfer-encoding

Conversation

@ronag

@ronag ronag commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Summary

  • track Transfer-Encoding while parsing HTTP/1 response headers
  • stop reusing a connection after a 204 or 304 response carries Transfer-Encoding, so trailing bytes cannot be assigned to the next pipelined request
  • cover 204 and 304 responses with blocking enabled and disabled

This adapts nodejs/undici#5941 to the slim HTTP/1 transport. The downstream baseline accepted the injected trailing response in all four regression cases.

Upstream head reviewed: 0191eab.
Downstream base: 2d840d6.

Validation

Exact candidate: a1f7876d6ba0321afe4f02ea2085ef28f2984c5d, Node.js 26.10.0.

  • macOS arm64: full unit suite 790 passed / 2 skipped; node-test suite 156 passed; declarations, full ESLint and diff checks passed.
  • Linux amd64: focused regression 8/8; full unit suite 790 passed / 2 skipped; node-test suite 156 passed; full ESLint and declarations passed. Isolated Docker on tv2k-srv4, 2 CPUs, 4 GiB, 512-process cap, pinned image nxtedition/node@sha256:8a47e2410fbb9718c3196525e9b35227e916e3dcae06c138f42786d22b1aae4f. The full unit suite used ordinary Docker DNS for the reserved .invalid lookup fixture; the no-network run stalled at that same existing test on both base and candidate.
  • Independent in-memory transport controls: 416 cases / 908 assertions passed across header split offsets, pipelining, informational responses, pause/abort and reset behavior. The base accepts all four injected-response cases; the candidate discards the injected response and obtains the real second response.
  • Fresh completed Copilot review covers this SHA with no findings or unresolved threads.

A legitimate 304 response may carry Transfer-Encoding under RFC 9112 §6.1; this change conservatively closes that connection to protect the response queue. HEAD handling retains the existing behavior.

No exact-head CI checks or commit statuses are currently reported.

@ronag
ronag requested a balanced review from Copilot October 8, 2026 08:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused parser change addresses the vulnerability and is covered across the relevant response and dispatch modes.

0 open findings

What changed in this PR

Prevents HTTP/1 response-queue poisoning when bodyless 204/304 responses include Transfer-Encoding.

Changes:

  • Tracks Transfer-Encoding during response parsing.
  • Disables connection reuse for affected 204/304 responses.
  • Adds regression coverage for blocking and pipelined requests.
File Description
lib/​dispatcher/​client-h1.js Detects unsafe responses and resets their connections.
test/​client-304-content-length.js Tests 204/304 handling across blocking modes.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@ronag
ronag marked this pull request as ready for review October 8, 2026 09:30
@ronag
ronag merged commit 8d2ac26 into nxtedition:master Oct 8, 2026
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.

2 participants