fetch: decide a streaming request body's framing once, reject caller framing headers it cannot honor

[robobun]

Problem

• fetch() cannot measure a stream body. It sent a caller Content-Length verbatim with the body raw behind it. Content-Length: 2 over a 7-byte stream left 5 surplus bytes on a keep-alive connection, which the peer parses as the next request. abc, -1, 5, 7 reached the wire too, and Transfer-Encoding: gzip went out over a chunk-framed body.
• Cause: build_request (src/http/lib.rs) and FetchTasklet::skip_chunked_framing each decided the framing from the raw headers.

Fix

• Decide once, in fetch_impl, before hop 1 (StreamFraming::for_body). A Content-Length of ASCII digits that fits u64 frames the raw bytes. A Transfer-Encoding ending in chunked is forwarded as written. Neither means chunked. Anything else rejects with ERR_HTTP_INVALID_HEADER_VALUE (a TypeError) before a byte is written.
• The decision rides to the HTTP thread in http_request_body::Stream. build_request consumes both caller headers on every hop and prints only what the Stream carries.
• The JS thread counts stream bytes against a declared length and reports a miss to the HTTP thread. It fails the request with ERR_HTTP_CONTENT_LENGTH_MISMATCH only while that body is being sent. After a 303 dropped it, the report is a no-op.
• Verified: describe in test/js/web/fetch/fetch.test.ts (10 of 24 fail on 1.4.3), h2 and h3 cases, redirect, keep-alive, upgrade suites.

Background

• The JS thread pumps the stream into a ThreadSafeStreamBuffer and chunk-frames it. The HTTP thread writes the head and flushes that buffer verbatim.
• A 303 (or 301/302 on POST) drops a stream body and follows with a GET. Only the HTTP thread knows when.
• FetchHeaders joins duplicate rows (5, 7).

Notes

• Wire bytes before the change (bun 1.4.3, 7-byte ReadableStream body nr1-nr2, raw TCP origin):
  • content-length: 2 → Content-Length: 2 + nr1-nr2 (5 surplus bytes). Same for an async generator and Readable.from().
  • +5, 0x5, 5.0, -1, 0, abc, 99999999999999999999 → sent verbatim, body raw. Content-Length: 50 → peer waits.
  • h.append('content-length','5'); h.append('content-length','7') → Content-Length: 5, 7, body raw.
  • transfer-encoding: gzip (or identity) → forwarded verbatim while the body was chunk-framed.
• After, for a streaming body that is not an upgrade:
  • Transfer-Encoding: chunked, Chunked, gzip, chunked go out exactly as written over 7\r\nnr1-nr2\r\n0\r\n\r\n. A Content-Length next to one is neither sent nor counted (it must still be a count). identity, gzip, chunked, gzip, chunked, chunked, gzip; q=1, chunked, br2, chunked, "" reject. The rule is the one the response side already applies to a Transfer-Encoding it receives (fold_transfer_encoding: HeaderValueIterator + Encoding::from_token): a comma-separated, OWS-trimmed, case-insensitive list of codings bun knows, where chunked must be the final coding (RFC 9112 section 6.1). Like a recipient, it skips empty list elements, so gzip,, chunked counts as gzip, chunked.
  • Content-Length: 7 is honored as before. 2, 0, 5 (OWS is not part of the value) reject with ERR_HTTP_CONTENT_LENGTH_MISMATCH before a body byte is written. 50 rejects when the stream ends. abc, +5, 0x5, 5.0, -1, "", 5, 7, 7, 7, 99999999999999999999 reject up front.
  • An up-front rejection cancels the body stream with the same error, sends nothing, opens no connection, and the next fetch to the origin works.
• Redirects. build_request consumes the caller's Content-Length and Transfer-Encoding on every hop, streaming or not, so a 303 follow-up goes out as a bare GET for every accepted form (Content-Length: 7, gzip, chunked, no header), over HTTP/1.1 and HTTP/2. Validation runs once, in fetch_impl, before hop 1. 307/308 still reject with "Request body is a ReadableStream and cannot be replayed for this redirect".
• Why the HTTP thread decides a mismatch: the JS thread keeps pumping the stream after the HTTP thread followed a 303, and cannot know the body was dropped. It used to abort the transport itself, which killed hop 2 (an early 303 followed by a stream that ends short rejected with "Request body of 0 bytes does not match the Content-Length of 7"). Now it posts LengthMismatch on the same queue as Data/End. The HTTP thread fails the request only if original_request_body is still that Stream (HTTP/1.1 close_and_fail, h2 detach_with_failure, h3 fail), so the order against the redirect is total and there is no flag to race on.
• "Fully sent": request_stage becomes Done only after an End message with the buffer flushed, and the JS thread sends End only when the count equals the declared length (a short stream reports a mismatch instead, a surplus chunk never enters the buffer). So an early 3xx with unsent declared bytes closes the hop-1 socket, as is_request_fully_sent() already required.
• Unchanged: bodies fetch can measure (string, Blob, file, blob-backed stream) drop both caller headers and send the computed Content-Length. An upgrade request keeps its header handling (its "body" is the tunnel, its bytes are not counted and its headers are not validated). node:http's client writes over its own socket and does not use this path.
• HTTP/2 and HTTP/3: the framing decision is made before the protocol is known, so the same validation applies there. A Transfer-Encoding that does not end in chunked on a stream body now rejects on those protocols too. Before, their encoders dropped the header silently. An accepted Transfer-Encoding is still dropped by those encoders, and a valid Content-Length is forwarded and counted.
• Behavior changes a caller can meet: the same correct length under two keys that differ only in case (content-length and Content-Length) now rejects, because FetchHeaders joins them into 7, 7. Transfer-Encoding: gzip on a stream body rejects on every protocol. gzip, chunked is forwarded as written. Content-Length: abc rejects even next to Transfer-Encoding: chunked. A stream that never ends under Content-Length: 0 rejects with ERR_HTTP_CONTENT_LENGTH_MISMATCH at its first chunk.
• Error codes: ERR_HTTP_INVALID_HEADER_VALUE (a TypeError, with Node's message Invalid value "abc" for header "Content-Length") and ERR_HTTP_CONTENT_LENGTH_MISMATCH both exist already (node:http and NodeHTTPResponse throw them for the same conditions), so no new code is added. undici's names are UND_ERR_INVALID_ARG and UND_ERR_REQ_CONTENT_LENGTH_MISMATCH.
• No new header parsers. The strict 1*DIGIT Content-Length parser existed twice (inline in handle_response_metadata and privately in the HTTP/2 server, h2/connection.rs). It is now one function, bun_http_types::parse_content_length_strict, used by both and by StreamFraming. The Transfer-Encoding loop moved out of handle_response_metadata into fold_transfer_encoding so the request side shares it.
• StreamFraming::transfer_encoding (embedded in Stream) is a StringPointer into the client's header_buf (the same buffer every other caller header is printed from), so nothing is copied or allocated for it and the Stream stays bitwise-copyable across threads.
• Edge probes on the debug build: multi-chunk overflow (CL: 5, chunks 3+4) writes only the first 3 bytes, then fails. An infinite pull() source with CL: 10 stops after 4 pulls and its cancel() runs. An async generator's finally runs. A peer that replies after exactly CL bytes gets a resolved fetch and never sees the surplus. CL: 0 with an empty stream succeeds. After a mismatch the next fetch to the same origin uses a fresh connection.
• Gateway shape (a Bun.serve handler forwarding req.headers and req.body upstream): the inbound Content-Length is honored and a 256 KB body streams through with the exact count. An inbound chunked request is forwarded with its Transfer-Encoding: chunked.
• test/js/web/fetch/fetch.test.ts "bounds memory when a handler forwards req.body to a stalled target" measures 260 to 265 MB against a 256 MB debug threshold in my container, with and without this diff (main's src/ gives 260 MB). It is not related.
• Related PRs: fetch: enforce body framing for streamed request bodies #35790 had the count-and-reject half and is closed in favor of this one. fetch: engine-compute framing for ReadableStream bodies; honour Connection: close on 3xx #35786 and fetch: drop hop-by-hop/framing request headers before the wire #35788 switch every stream body to chunked, which would regress fix(http): preserve explicit Content-Length header with streaming request body #27062 (origins that reject chunked uploads).
• Deferred, not in this PR: is_node_http_client is always false on main now (fetch_impl::<true> has no caller), so the flag and the lenient bodyless Content-Length branch it guards in build_request are dead.

---

no test proof · iteration 8 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/fetch/fetch.test.ts, test/js/web/fetch/fetch-http3-client.test.ts

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: a85d7060-d316-4ed8-ade0-77d5616ba946

📥 Commits

Reviewing files that changed from the base of the PR and between 1b74d18 and 30c9f09.

📒 Files selected for processing (6)

• src/http/HTTPThread.rs
• src/http/h3_client/ClientSession.rs
• src/runtime/webcore/fetch/FetchTasklet.rs
• test/js/web/fetch/fetch-http2-client.test.ts
• test/js/web/fetch/fetch-http3-client.test.ts
• test/js/web/fetch/fetch.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.

---

Walkthrough

Streaming request bodies now use validated framing metadata. Fetch setup passes this metadata to transport code, which enforces declared lengths and reports mismatches across HTTP/1, HTTP/2, and HTTP/3. Tests cover framing, redirects, invalid headers, and stream types.

Changes

Streaming request framing

Layer / File(s)	Summary
Framing contracts and validation src/http/HTTPRequestBody.rs, src/http_types/ETag.rs	Adds StreamFraming, strict Content-Length parsing, transfer-encoding validation, upgrade framing, and pointer-based header lookup.
Fetch framing and request construction src/http/lib.rs, src/runtime/webcore/fetch.rs, src/runtime/webcore/fetch/FetchTasklet.rs	Resolves stream framing before queuing, passes it through FetchOptions, and generates request framing from stream metadata.
Length enforcement and protocol failure handling src/runtime/webcore/fetch/FetchTasklet.rs, src/http/HTTPThread.rs, src/http/error.rs, src/http/h2_client/*, src/http/h3_client/*	Counts streamed bytes, rejects surplus or short bodies, and propagates RequestBodyLengthMismatch through HTTP/1, HTTP/2, and HTTP/3 handling.
Framing behavior coverage test/js/web/fetch/fetch.test.ts, test/js/web/fetch/fetch-http2-client.test.ts, test/js/web/fetch/fetch-http3-client.test.ts	Tests wire framing, invalid headers, length mismatches, redirects, request wrappers, header overrides, and Blob-backed streams.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: streaming request-body framing is decided once, and unsupported caller framing headers are rejected.
Description check	✅ Passed	The description explains the problem, fix, behavior, error handling, redirects, and verification results in detail. It does not use the template headings "What does this PR do?" and "How did you verif…

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Reproduced on bun 1.4.3 (f42e980) against a raw node:net origin that records the exact request bytes. A 7-byte ReadableStream body with content-length: 2 produced Content-Length: 2 followed by all 7 raw bytes; the same for an async-generator body and Readable.from(). +5, 0x5, 5.0, -1, 0, abc, 99999999999999999999 and the two-row form 5, 7 were all copied into the head verbatim with the body unframed behind them, and content-length: 50 left the origin waiting. transfer-encoding: gzip was forwarded verbatim over a chunk-framed body.

With this branch the same probe shows: a Content-Length that is not a count and a Transfer-Encoding that does not end in chunked reject the fetch with a TypeError (ERR_HTTP_INVALID_HEADER_VALUE) before anything is sent. A Transfer-Encoding that ends in chunked is forwarded as written over a correctly chunk-framed body. A valid Content-Length is honored and counted, and a surplus or shortfall rejects with ERR_HTTP_CONTENT_LENGTH_MISMATCH with no surplus byte on the wire.

Across a redirect, a 303 follow-up carries no framing header from the caller (HTTP/1.1 and HTTP/2), a 303 that arrives before the body resolves with the final response even if the dropped stream then misses its count, and a 307 still rejects as not replayable.

The new describe in test/js/web/fetch/fetch.test.ts fails 10 of 24 on 1.4.3 and passes here, with new h2 and h3 cases in fetch-http2-client.test.ts and fetch-http3-client.test.ts; client-fetch, fetch-keepalive, fetch-redirect, fetch.upgrade, fetch-connection-header, fetch-abort-stream-body and test/regression/issue/27061.test.ts pass.

This supersedes #35790 and covers the stream-body framing parts of #35786 and #35788.

CI (build 116275): every test this PR adds or touches passes on all lanes (Linux, Windows x64 and aarch64, macOS x64 and aarch64). The red jobs all fail on one test, test/internal/build-post-link-ordering.test.ts, a build-script test that is also red on main (build 116264). This PR does not touch it. Ready for review.

[robobun]

^{Updated 7:04 PM PT - Sep 17th, 2026}

✅ @robobun, your commit 71e616a6916a9fdf32c1f77363c8b8d7f3c91ce7 passed in Build #117435! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 42024

That installs a local version of the PR into your bun-42024 executable, so you can run:

bun-42024 --bun

[claude]

I reviewed this PR and didn't find any bugs. Because it changes how fetch() frames HTTP/1.1 request bodies on the wire — a request-smuggling-adjacent path with cross-thread state — a human look would still be worthwhile.

What was reviewed:

• Framing decision flow: single decision on the JS thread, carried via Stream::content_length, emitted by build_request_headers — no path re-reads raw caller headers.
• Refcount balance on the new shortfall early-return in the stream-end handler; saturating_add on the byte counter; the detach_lifetime on request_content_len_buf mirrors the existing non-streaming branch immediately below it.
• content_length_for_framing rejects empty/+/hex/float/joined/overflow; upgraded connections skip counting; error routes through the existing ERR_HTTP_CONTENT_LENGTH_MISMATCH code.
• Test uses a local net server on port: 0, awaits observable conditions (no sleeps), and the shortfall assertion avoids the Windows RST race.

Extended reasoning...

Overview

This PR reworks how fetch() frames a streaming request body (ReadableStream, async generator, node Readable) over HTTP/1.1. Previously, the HTTP thread and the JS thread each inspected raw caller headers independently to decide framing, and a caller-supplied Content-Length was copied to the wire verbatim without validating that the stream would produce exactly that many bytes. The fix moves the framing decision to a single point on the JS thread (content_length_for_framing strictly parses ASCII digits fitting u64), stores it on http_request_body::Stream::content_length, and has build_request_headers announce that decision rather than the raw caller value. write_request_data counts bytes against the declared length and aborts with ERR_HTTP_CONTENT_LENGTH_MISMATCH on surplus or shortfall. Caller Transfer-Encoding is now always dropped. Four files touched: src/http/HTTPRequestBody.rs, src/http/lib.rs, src/runtime/webcore/fetch/FetchTasklet.rs, and test/js/web/fetch/fetch.test.ts.

Security risks

The change is itself a security hardening: without it, a Content-Length: 2 header on a 7-byte streaming body would leave 5 surplus bytes on a keep-alive connection where the peer parses them as the next request — classic request smuggling. The fix closes that by refusing to write past the declared length and aborting the transport (keeping the connection out of the pool). I checked that no surplus byte reaches thread_safe_stream_buffer (the mismatch check runs before the buffer append), that unparseable values fall back to chunked rather than reaching the wire, and that the abort path sets an abort reason and calls abort_task() so the connection is not reused. The remaining risk is behavioral/compat: the change alters what goes on the wire for edge-case caller headers, and interactions with HTTP/2, redirects that re-send the body, and the upgrade path deserve human eyes.

Level of scrutiny

High. HTTP/1.1 message framing is a protocol boundary where mistakes become request smuggling or desync. The change coordinates state across two threads (JS thread decides and counts; HTTP thread emits the header), adds two new FetchTasklet fields with lifecycle implications, and introduces a new abort path mid-write. The PR description is thorough and the test coverage is broad (invalid values, joined headers, all three streaming body types, Transfer-Encoding variants, blob-backed streams), but the wire-level consequences and the compat call (honoring vs. rejecting a caller Content-Length, the ERR_HTTP_CONTENT_LENGTH_MISMATCH vs. undici's UND_ERR_REQ_CONTENT_LENGTH_MISMATCH choice) are the kind of decision a maintainer should sign off on.

Other factors

No CODEOWNERS entry covers these paths. The unsafe { bun_ptr::detach_lifetime } on request_content_len_buf is identical to the pre-existing pattern for the non-streaming branch a few lines below, and the buffer is a fixed-size array field on self. The new early-return on shortfall balances the ref it holds (FetchTasklet::deref(this_ptr)) matching the neighboring is_completed() early-return. The error code reuses the existing ERR_HTTP_CONTENT_LENGTH_MISMATCH from ErrorCode.ts rather than minting a new one. The PR notes it supersedes/overlaps three other PRs and touches the same functions as two in-flight unsafe-removal PRs — merge ordering is a maintainer call. The bug hunt exited on dry_streak with no findings and no ruled-out candidates.

[Jarred-Sumner]

The Content-Length half is right: decide once on the JS thread, count the bytes, fail with ERR_HTTP_CONTENT_LENGTH_MISMATCH and close instead of leaving surplus on a keep-alive connection. Keep all of that.

The Transfer-Encoding half needs to change. "A caller Transfer-Encoding is never forwarded" throws away the one case that is legitimate (gzip, chunked on a body the caller compressed, or plain chunked copied from an inbound request), and silently relabels the others. Accept it only when fetch can honor it:

Streaming body, not an upgrade

• Parse the caller value as a comma-separated list of codings (OWS-trimmed, case-insensitive).
• Final coding is chunked (chunked, Chunked, gzip, chunked): put the caller's value on the wire as written, in place of CHUNKED_ENCODED_HEADER, and chunk-frame the body as now. A Content-Length next to it is not sent and not counted, which is what the PR already does.
• Anything else (identity, gzip, chunked, gzip, empty, a list we cannot parse): reject the fetch before a byte is written. We chunk-frame every streaming body ourselves, so these can only describe framing we are not producing; on 1.4.3 transfer-encoding: gzip goes out as Transfer-Encoding: gzip over a chunk-framed body. undici rejects every caller Transfer-Encoding (UND_ERR_INVALID_ARG, "invalid transfer-encoding header"), so rejecting this subset breaks nothing that runs on Node. Do not drop-and-continue here: a body the caller gzipped would arrive as plain chunked with nothing saying it is compressed.

Body fetch can measure (string, Blob, file): unchanged, the header is dropped and the computed Content-Length wins, as today.

HTTP/2 / upgrade: unchanged.

Where: make this the same decision as content_length_for_framing, on the JS thread in FetchTasklet where content_length is computed, and carry the result to the HTTP thread in http_request_body::Stream next to content_length (the validated caller value, or "chunked, ours"). build_request's CHUNKED_ENCODED_HEADER arm then emits what Stream says instead of continue-ing unconditionally, so the two threads still cannot disagree, which is the point of this PR.

Test table in fetch.test.ts (the block that currently expects ["transfer-encoding: chunked"] for every row):

• chunked, Chunked, gzip, chunked → that exact value on the wire, body 7\r\nnr1-nr2\r\n0\r\n\r\n
• chunked + Content-Length: 7, chunked + Content-Length: 2 → transfer-encoding: chunked only, no content-length line, fetch resolves
• identity, gzip, chunked, gzip, "" → fetch rejects, the origin sees no request bytes, and the next fetch to the origin works
• same rows for an async generator and Readable.from(), as the Content-Length cases already do

Then fix the title and the description's "A caller Transfer-Encoding is never forwarded" to match.

[Jarred-Sumner]

Two more, same theme: invalid framing headers are an error, not something to paper over.

Throw for an invalid Content-Length too. Right now content_length_for_framing returning None for abc, -1, +5, 0x5, 5.0, 5, 7, 99999999999999999999 makes the request go out as Transfer-Encoding: chunked as if the caller had said nothing. The caller said something wrong; tell them. With a streaming body, a Content-Length that is not ASCII digits fitting a u64 rejects the fetch up front (a TypeError, before the stream is pulled or a byte is written), the same as the unusable Transfer-Encoding values above. undici throws UND_ERR_INVALID_ARG "invalid content-length header" for these. Only an absent Content-Length means "chunk it for me".

So the JS-thread decision has three outcomes, not two:

• valid Content-Length, no Transfer-Encoding → framed by that count
• no Content-Length, or a Transfer-Encoding whose final coding is chunked → chunked
• anything else → throw

Whitespace: 5 is fine to accept as 5 (OWS around a field value is not part of it); it then hits the mismatch path like any other wrong count.

Pass the integer to the other thread, nothing else. Stream::content_length: Option<u64> is the right shape. Make it the only thing the HTTP thread uses for framing: build_request should not read content-length or transfer-encoding out of the raw caller headers at all for a streaming body (the original_content_length capture and the CHUNKED_ENCODED_HEADER arm both go away for that case). It prints the u64 it was handed, or the chunked header it was handed. Same for the accepted Transfer-Encoding value: validated on the JS thread, handed over in Stream, emitted as given. Nothing on the HTTP thread parses or re-validates a framing header.

Test rows to flip: every invalid Content-Length currently expected to produce ["transfer-encoding: chunked"] should expect a rejected fetch, no request bytes at the origin, and a working next fetch.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Removed the header parsers this PR had added (4dbfead, b035772). It now reuses what the tree already has:

• Content-Length: the strict 1*DIGIT parser existed twice, inline in handle_response_metadata and privately in the HTTP/2 server (h2/connection.rs). It is now one function, bun_http_types::parse_content_length_strict, used by both of those and by StreamFraming. My content_length_for_framing is gone.
• Transfer-Encoding: the request side now applies the same rule the response side applies to a Transfer-Encoding it receives. That loop (HeaderValueIterator + Encoding::from_token, chunked must be the final coding) moved out of handle_response_metadata into fold_transfer_encoding next to the other header-value helpers, and both sides call it. My transfer_encoding_ends_in_chunked and the is_tchar table are gone.
• Stream embeds StreamFraming instead of repeating its two fields. The length mismatch message goes through the existing stream_body_by_http_id entry points (their ended: bool became the WriteMessageType), so the three parallel fail_request_body_by_http_id functions are gone. fetch_impl shares one reject_before_send with the pre-aborted-signal path, and the error uses Node's message for that code: Invalid value "abc" for header "Content-Length".

Two behavior differences follow from sharing the recipient's rule, both visible in the test table:

• A coding bun does not know rejects (br2, chunked), the same as on a response.
• Empty list elements are skipped, as RFC 9110 section 5.6.1.2 requires of a recipient, so gzip,, chunked and chunked, now count as ending in chunked and go out as written. Before, my parser rejected them. Say so if you want the request side to stay stricter than the response side here.

fetch.test.ts (the framing describe), fetch-http2-client, fetch-http3-client, fetch-redirect, fetch-gzip, serve-http2, serve-http2-protocol and node-http2 pass on the debug build.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.