Fix Bun.file().slice() being treated as the rest of the file - #32794
Fix Bun.file().slice() being treated as the rest of the file#32794robobun wants to merge 4 commits into
Conversation
|
Updated 10:38 PM PT - Jun 29th, 2026
❌ @robobun, your commit 9be8e38 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 32794That installs a local version of the PR into your bun-32794 --bun |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Status at 9be8e38 (rebased onto current This PR's diff has already passed a full CI run. Build 65279 carried the identical
The one real failure this PR ever caused ( This stays a draft pending one decision: whether to drop the documented |
Three RFC violations in `Bun.serve`'s HTTP/1 response framing, all with
the same shape: the HEAD path, the 204 path, and the empty-file path
each derived framing independently of what GET puts on the wire. Review
turned up a fourth member of the class: the static `routes:` paths
(`StaticRoute`, `FileRoute`) put a null-body status's Response body
bytes on the wire, which GET's `render()` drops.
### Repro
```js
// 1. Handler-supplied Content-Length: corrected on GET, forwarded verbatim on HEAD
using a = Bun.serve({ port: 0, fetch: () => new Response("hi", { headers: { "Content-Length": "999" } }) });
(await fetch(a.url)).headers.get("content-length"); // "2"
(await fetch(a.url, { method: "HEAD" })).headers.get("content-length"); // "999"
// 2. Default-200 Response with an empty file body is rewritten to 204, but not on HEAD
require("fs").writeFileSync("/tmp/e", "");
using b = Bun.serve({ port: 0, fetch: () => new Response(Bun.file("/tmp/e")) });
(await fetch(b.url)).status; // 204
(await fetch(b.url, { method: "HEAD" })).status; // 200
// 3. Every 204 carries Content-Length: 0
using c = Bun.serve({ port: 0, fetch: () => new Response(null, { status: 204 }) });
// raw socket: HTTP/1.1 204 No Content\r\nDate: ...\r\nContent-Length: 0\r\n\r\n
// 4. A static route puts a null-body status's body bytes on the wire
using d = Bun.serve({ port: 0, routes: { "/": new Response("data", { status: 204 }) }, fetch: () => new Response(null) });
// raw socket: HTTP/1.1 204 No Content\r\n...Content-Length: 4\r\n\r\ndata
```
RFC 9110 9.3.2: a HEAD response carries the same header fields a GET of
the same target would have. RFC 9110 8.6: a server MUST NOT send
`Content-Length` in a response with a 1xx or 204 status (case 3 is issue
#20676, where such a 204 is rejected as a framing error by a proxy). RFC
9112 6.3: a 1xx/204/304 is terminated by the blank line after the header
fields regardless of what follows, so case 4's stray body bytes desync
the next keep-alive response.
### Causes
1. `do_render_head_response` forwarded a handler-supplied
`Content-Length` / `Transfer-Encoding` header before looking at the
body. GET strips both (`render_metadata` -> `do_write_headers`) and
frames from the body's byte count.
2. `render_metadata` rewrote 200 -> 204 when `size == 0 &&
!blob.is_detached()`. Every in-memory empty body (`""`, `Uint8Array(0)`,
`new Blob([])`) happens to report `is_detached() == true`, so only the
file / Blob-store forms got the rewrite, and the HEAD path (whose
`self.blob` is never populated) never did. `StaticRoute` and `FileRoute`
each carried a copy of the same rewrite.
3. `Bun.serve` never marked the uWS response as a no-body status, and
uWS's `tryEnd` -> `internalEnd` path (unlike `end()`) never consulted
`noBodyStatus`, so `render_bytes`'s `try_end("", 0)` always emitted
`Content-Length: 0`.
4. The WHATWG null-body-status set `{101, 103, 204, 205, 304}` was
spelled out inline at three sites and absent from two: `render()`
dropped the body, but `StaticRoute` unconditionally sent its cached
blob, and `FileRoute`'s bodiless list (`204 | 205 | 304 | 307 | 308`)
was missing 1xx.
### Fix
- `uws::HttpResponse::writeStatus` records `noBodyStatus` for 1xx / 204
statuses (the existing flag `node:http` already sets for 204/304).
`internalEnd` then drops the Content-Length, the data, and the
totalSize, exactly what `end()` already does on its `noBodyStatus`
short-circuit; `writeHeader(key, uint64_t)` skips an integer
`Content-Length` too. 205 (MUST indicate a zero-length body, RFC 9110
15.3.6) and 304 (MAY carry one, 15.4.5) keep `Content-Length: 0`.
- `do_render_head_response` short-circuits null-body statuses to the
same `render_metadata()` + empty `try_end` that GET's `render()` uses,
instead of framing from the dropped body or the user headers.
- `do_render_head_response` honors a handler-supplied `Content-Length` /
`Transfer-Encoding` only when the Response's body is null. A null body
carries no framing of its own, so the header is the only description of
what a GET would have sent; that is the escape hatch issue #15355 added
for HEAD handlers and it is preserved (its tests still pass). A real
body's byte count now wins for both methods.
- The 200 -> 204-on-empty rewrite is removed from `render_metadata`,
`StaticRoute`, and `FileRoute`. An empty file is a valid zero-byte
representation. The rewrite was undocumented, applied to only one of six
empty body forms, never applied on HEAD, and produced the spec-violating
`204 + Content-Length: 0`. Every empty body now frames as `200` with
`Content-Length: 0`, as `new Response("")` already did.
- `StaticRoute` drops the body for a null-body status and its HEAD path
reports `Content-Length: 0` (mirroring GET); `FileRoute`'s bodiless
early return gains 1xx. `FileResponseStream` ships the file via sendfile
/ `write()`, neither of which goes through `internalEnd`, so `FileRoute`
needs its own guard. The set now lives in one
`HTTPStatusText::is_null_body` predicate used by all four paths.
### Existing tests updated
- `bun-server.test.ts` "transfer-encoding / content-length whose
StringImpl is held only by the header map" used `new Response("hello",
...)` with duplicate `Transfer-Encoding` / `Content-Length` headers and
asserted HEAD forwarded them, which is the behavior item 1 removes. The
test exists to catch a use-after-free in the header fast path (`fastGet`
-> `render_metadata` frees the `StringImpl` it borrowed); that path is
now only reachable with a null body, so the fixture bodies became `null`
and the assertions are unchanged.
- `bun-serve-file.test.ts` "serves empty file" and "returns 204 for
empty files with 200 status" now assert `200` + `Content-Length: 0`, for
GET and HEAD, on both the route and fetch-handler paths.
### Verification
Full framing matrix (raw socket, body form x GET/HEAD) before and after:
<details>
```
# before (bun 1.4.0 / main)
body GET HEAD
Response("hi") 200 CL=2 200 CL=2
Response("") 200 CL=0 200 CL=0
Response(null) 200 CL=0 200 CL=0
Response(Uint8Array(0)) 200 CL=0 200 CL=0
Response(new Blob([])) 200 CL=0 200 CL=0
Bun.file(empty) 204 CL=0 200 CL=0 <<<
Bun.file(full) 200 CL=10 200 CL=10
"hi" + CL:999 200 CL=2 200 CL=999 <<<
null + CL:999 200 CL=0 200 CL=999
null status 204 204 CL=0 204 CL=0 <<<
"body" status 204 204 CL=0 204 CL=4 <<<
null status 304 304 CL=0 304 CL=0
null status 205 205 CL=0 205 CL=0
null status 101 101 CL=0 101 CL=0 <<<
# after
Bun.file(empty) 200 CL=0 200 CL=0
"hi" + CL:999 200 CL=2 200 CL=2
null + CL:999 200 CL=0 200 CL=999 (#15355, unchanged)
null status 204 204 CL=- 204 CL=-
"body" status 204 204 CL=- 204 CL=-
null status 304 304 CL=0 304 CL=0
null status 205 205 CL=0 205 CL=0
null status 101 101 CL=- 101 CL=-
# routes vs fetch handler, Response("data", { status }), before -> after
204 routes GET 204 CL=4 body="data" -> 204 CL=- body=""
204 routes HEAD 204 CL=4 -> 204 CL=-
205 routes GET 205 CL=4 body="data" -> 205 CL=0 body=""
304 routes GET 304 CL=4 body="data" -> 304 CL=0 body=""
```
</details>
The new `describe("response framing")` in `serve.test.ts` has 24
assertions that fail on the unmodified build and 7 deliberate positive
controls (205/304 keep `Content-Length: 0` on the `fetch` path; the
null-body escape hatch still forwards the user header).
`bun-serve-file.test.ts` (67), `bun-serve-static.test.ts` (34),
`bun-serve-routes.test.ts` (41), `node-http.test.ts` (127),
`proxy-stress-adversarial.test.ts` (151), `serve-http3.test.ts` (45) all
pass.
Fixes #20676
Related: #32794 fixes the fourth framing divergence the same fuzzer
found (`Bun.file(p).slice(a, b)` treated as `a..EOF`). Different root
cause, but both touch `render_metadata`, so whichever lands second needs
a small rebase.
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
fad4e32 to
5c8f6ee
Compare
Bun.file(p).slice(a, b) served from a Bun.serve fetch handler reported the wrong framing: GET invented a 206 + Content-Range the client never asked for, and HEAD reported Content-Length = fileSize - a. - Blob::resolve_size / resolved_size: the File arm widened a concrete slice size to max_size - offset; the Bytes arm already guarded against this. Extract the shared clamp_view_to_store helper and use it in both arms of both functions. This fixes HEAD's Content-Length, Response(slice).body reading past the end of the slice, and structuredClone mutating the source blob's size. - RequestContext::do_sendfile: resolve the blob's size to the clamped sendfile length instead of the stat size, so the slice is the HTTP entity and render_metadata frames it as a plain 200. - render_metadata: needs_content_range now only comes from a resolved incoming Range header, so the remain < blob.size() and has_content_range compensations are dead; remove them. - Blob serialization: add the blob's size (format version 4) so a file slice's length survives structuredClone / bun:jsc serialize.
new Response(Bun.file(f).slice(start, end)) is now a plain 200 whose entity is the slice, so rewrite the serve.test.ts suite that pinned the old 206 + Content-Range behavior to assert the new framing (and add the Content-Length / Content-Range checks it was missing). An empty slice is a 204, never a 0-byte 206. Update docs/runtime/http/routing.mdx: slicing sets Content-Length to the slice's length, and incoming Range headers are handled natively by returning the whole file, so drop the parse-Range-yourself example.
The version-4 blob wire format wrote self.size.get() after the serializer's resolve_size() call. For an fd-backed blob that pins the resolve result of the sending process into the clone, and an fd number is meaningless outside it: the cross-process structuredClone test got size 0 back because the intermediate process could not stat the fd. Capture the size before resolve_size() runs. A slice's concrete length still round-trips; an unresolved Bun.file(p) / Bun.file(fd) keeps the MAX_SIZE unknown-size sentinel on the wire so the receiver resolves it lazily against its own path or fd, exactly as before version 4.
…32800 8f6a7c6 (#32800) removed render_metadata's 200-to-204 rewrite for an empty body, so an empty Bun.file().slice() is now a plain 200 with Content-Length: 0 on both GET and HEAD. That also makes GET and HEAD fully agree for an empty slice, so the two separate empty-slice tests collapse into the GET/HEAD parity matrix.
094c3ee to
9be8e38
Compare
|
Heads-up on an overlap: #33360 touches the same A file store's Bun.file("/proc/version").slice(0, 10); // clamp_view_to_store(0, 10, 0) -> size 0The No action needed now; whichever of the two lands second can take that arm from the other. The rest of this PR (HEAD |
…e() via sendfile (#36862) Found by the outbound-request-body fuzzer (ledger #11440). ### Repro ```js // file is >= 32 KiB so the sendfile fast path is taken require("fs").writeFileSync("/tmp/f.bin", Buffer.alloc(65536)); using server = Bun.serve({ port: 0, async fetch(req) { console.log("CL", req.headers.get("content-length")); await req.arrayBuffer(); return new Response("ok"); } }); await fetch(server.url, { method: "POST", body: Bun.file("/tmp/f.bin").slice(10, 110) }); ``` ``` CL 65536 <- should be 100 (hangs: fetch() never settles) ``` The 100 slice bytes arrive correctly (right offset, right count); only the `Content-Length` header is wrong, so the origin waits for 65436 more bytes that never come. Cliff is exactly at a 32 KiB backing file; slice size is irrelevant. ### Cause `src/runtime/webcore/fetch.rs`'s sendfile setup computed `content_size` as the whole file's `stat.st_size` for regular files, discarding the slice's own size: ```rust let blob_size = if bun_sys::S::ISREG(stat.st_mode as u32) { stat_size // <- ignores the slice window } else { original_size.min(stat_size) }; ``` `remain` was then separately clamped to the slice window, so `sendfile(2)` wrote the right bytes while `HTTPRequestBody::Sendfile(sf).len()` (= `sf.content_size`) produced the wrong `Content-Length`. ### Fix After the existing `remain` clamp for regular files, set `content_size = remain`; that is exactly the byte count `sendfile` will write. The now-redundant `blob_size` branch is dropped. ### Verification `test/js/bun/http/fetch-file-upload.test.ts` gains a `describe` covering slice uploads across the 32 KiB boundary (32767 / 32768 / 64 KiB / 1 MiB files) plus an open-ended `slice(10)`. All four boundary cases and the open-ended slice time out on `main` and pass with this change. The existing whole-file sendfile roundtrip test in the same file continues to pass. Related: #32794 fixes the same bug class on the `Bun.serve` response side; this is the `fetch()` client upload side. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/fetch-file-upload.test.ts <!-- robobun:evidence:end -->
…e() via sendfile (oven-sh#36862) Found by the outbound-request-body fuzzer (ledger oven-sh#11440). ### Repro ```js // file is >= 32 KiB so the sendfile fast path is taken require("fs").writeFileSync("/tmp/f.bin", Buffer.alloc(65536)); using server = Bun.serve({ port: 0, async fetch(req) { console.log("CL", req.headers.get("content-length")); await req.arrayBuffer(); return new Response("ok"); } }); await fetch(server.url, { method: "POST", body: Bun.file("/tmp/f.bin").slice(10, 110) }); ``` ``` CL 65536 <- should be 100 (hangs: fetch() never settles) ``` The 100 slice bytes arrive correctly (right offset, right count); only the `Content-Length` header is wrong, so the origin waits for 65436 more bytes that never come. Cliff is exactly at a 32 KiB backing file; slice size is irrelevant. ### Cause `src/runtime/webcore/fetch.rs`'s sendfile setup computed `content_size` as the whole file's `stat.st_size` for regular files, discarding the slice's own size: ```rust let blob_size = if bun_sys::S::ISREG(stat.st_mode as u32) { stat_size // <- ignores the slice window } else { original_size.min(stat_size) }; ``` `remain` was then separately clamped to the slice window, so `sendfile(2)` wrote the right bytes while `HTTPRequestBody::Sendfile(sf).len()` (= `sf.content_size`) produced the wrong `Content-Length`. ### Fix After the existing `remain` clamp for regular files, set `content_size = remain`; that is exactly the byte count `sendfile` will write. The now-redundant `blob_size` branch is dropped. ### Verification `test/js/bun/http/fetch-file-upload.test.ts` gains a `describe` covering slice uploads across the 32 KiB boundary (32767 / 32768 / 64 KiB / 1 MiB files) plus an open-ended `slice(10)`. All four boundary cases and the open-ended slice time out on `main` and pass with this change. The existing whole-file sendfile roundtrip test in the same file continues to pass. Related: oven-sh#32794 fixes the same bug class on the `Bun.serve` response side; this is the `fetch()` client upload side. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/fetch-file-upload.test.ts <!-- robobun:evidence:end -->
Found by a fuzzer that byte-checks
Bun.servefile responses. ABun.file(p).slice(a, b)body is repeatedly treated asa..end-of-fileinstead ofa..b. There are four manifestations with one shared root: a file slice's concretesizegets overwritten with a value derived from the file size and the offset.Repro
On 1.4.0 and current
main, the request sent noRangeheader, yet:Causes
Blob::resolve_size()/resolved_size()(Blob.rs): theFilearm doessize = max_size - offset, overwriting a slice's concrete length. TheBytesarm directly above it already has the guard, with a comment explaining why. Every unconditionalresolve_size()caller is affected:do_render_head_response: HEAD'sContent-Lengthis 900 (fileSize - offset).Body::to_readable_stream:new Response(Bun.file(p).slice(100, 200)).bodystreams 900 bytes, reading past the end of the slice..sizeto 900.offsetare serialized, never thesize, so a deserializedBun.file(p).slice(100, 200)widens to 900 bytes.RequestContext::do_sendfile:b.size.set(stat_size)overwrites the slice blob's size with the whole file's. That is what makesrender_metadatabelieve the body is a partial file and invent the 206.The static route path (
routes: { "/x": new Response(Bun.file(p).slice(a, b)) }) already treated the slice as the entity; only the fetch handler path was wrong.Fix
Blob.rs: extractclamp_view_to_store()(the logic theBytesarm already had) and use it in both arms ofresolve_sizeandresolved_size. A concrete slice size is authoritative; only the unknown-size sentinel resolves to the remainder of the store.sizeand bump the format version from 3 to 4. Only a slice's concrete length is pinned; an unresolvedBun.file(p)/Bun.file(fd)keeps the unknown-size sentinel on the wire so the receiver resolves it lazily against its own path or fd (an fd number is meaningless in another process). Version 3 payloads still deserialize.RequestContext::do_sendfile: resolve the blob's size to the clampedsendfile.remain, the byte count actually sent, so the slice is the HTTP entity.Content-Lengthis the slice's length andrender_metadataframes it as a plain 200. An empty slice is a plain200withContent-Length: 0, matching 8f6a7c6 (Bun.serve: fix HEAD and 204 response framing #32800).render_metadata: with the blob size resolved correctly,needs_content_rangeis only ever set by a resolved incomingRangeheader, so theremain < blob.size()andhas_content_rangecompensations are dead. Removed, along with the now-redundant flag setter indo_sendfile.Rebased onto #32800
8f6a7c6 (#32800) landed on
mainand fixes the companion framing class from the same fuzzer run (the HEAD / 204 / empty-body paths each derived framing independently of GET). This PR fixes the fourth divergence it mentions (Bun.file(p).slice(a, b)treated asa..EOF); the two are independent root causes but both editrender_metadata, so this branch is rebased onto it. The one conflict was the200 -> 204rewrite for an empty body: #32800 deliberately removes it (an empty body is a valid zero-byte200entity), so this branch keeps that removal, and the empty-slice tests here assert200withContent-Length: 0(which #32800 also makes HEAD agree on).Behavior change, needs a maintainer call
new Response(Bun.file(f).slice(a, b))with no clientRangeheader used to be a206 Partial ContentwithContent-Range: bytes a-(b-1)/*. That was documented (docs/runtime/http/routing.mdx, introduced in 0617896) and covered byserve.test.ts'sshould support Content-Range with Bun.file()suite; both are updated here. Reasons to drop it:Rangeheader. Unsolicited 206es poison caches, and any HEAD-then-GET client sees two contradictory descriptions of the same resource.Rangeheader support. Onmain,new Response(Bun.file(p).slice(100, 200))plusRange: bytes=0-9returns206 Content-Range: bytes 100-199/*with 100 bytes: aContent-Rangefor a different range than the client asked for.Rangeheader yourself and return a slice) is superseded:new Response(Bun.file(p))already resolves the client'sRangeheader natively and responds with a correct 206.This is the only part of the PR that changes documented behavior; the other fixes are unambiguous. Happy to split it out if preferred.
Tests
test/js/bun/http/bun-serve-file.test.ts: a GET/HEAD parity matrix for slices at offset 0, a nonzero offset, open-ended, past EOF, and empty. Also strengthens the existing/slice-escapeassertion.test/js/bun/http/serve.test.ts: re-asserts the existing slice suite for the new framing and adds theContent-Lengthand absent-Content-Rangechecks it never had.test/js/web/fetch/blob.test.ts:Response(slice).bodystreams exactly the slice's bytes.test/js/web/structured-clone-blob-file.test.ts: a file-backed slice round-trips its offset and length (the file sibling of the existing memory-blob test).All of the new and re-asserted tests fail without the
src/changes and pass with them.