Repository navigation
Conversation
…tor's position A fetch handler can return Bun.file(fd) for a regular file on a descriptor that the program opened. FileResponseStream read such a body with read(2) when the byte range started at offset 0, so the body started at the position of the descriptor and each response moved it. A descriptor that the program had written through sent Content-Length: 16 and no bytes. A Range request for bytes 0-3 sent the bytes at the position. StartOptions now has a positional flag. do_sendfile sets it for a regular file from Bun.file(fd), and start() then reads at the offset, also when the offset is 0. Static routes, directory routes and Bun.file(path) open a new descriptor for each response and keep read(2).
|
Updated 8:12 PM PT - Oct 16th, 2026
✅ @robobun, your commit 13c61d496db124cae23391210e056a21b5d6f521 passed in 🧪 To try this PR locally: bunx bun-pr 45266That installs a local version of the PR into your bun-45266 --bun |
|
Status: ready for review. CI passed on every lane (build 126929, 181 jobs), Windows and macOS included. Reproduction (Linux x64, release build of main 3d6c7bc): a fetch handler returns The 7 new tests in For a maintainer: the PR takes one decision that nobody made yet. In |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe change adds explicit positional-read control to ChangesFile response reads
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely identifies the main change: Bun.serve reads Bun.file(fd) bodies from the requested offset instead of the descriptor's current position. |
| Description check | |
The description is detailed and covers the problem, fix, verification, scope, limitations, and related issues. It does not use the template headings exactly, but it provides the required information a… |
Full details: Description check
-
Explanation
The description is detailed and covers the problem, fix, verification, scope, limitations, and related issues. It does not use the template headings exactly, but it provides the required information and is substantially complete.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
A response whose body is a stream of the file can be chunked. On Windows the HTMLRewriter row is. The raw client returned the chunk framing as the body. The rows where the server reads a stream now use fetch(), which accepts both framings. A connection that closes before the first chunk fails the in-flight test at once.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review completed
Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.
Still open from earlier reviews (3):
- 🔴
src/runtime/server/FileResponseStream.rs:115—Windows programs that keep writing through a fd while serving it as Bun.file(fd) can now have those writes silently ove… - Also unresolved: 2 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
libuv reads at an offset by saving the position of the handle, reading, and restoring it, on a pool thread. A read or write that the program makes through the descriptor at that moment can land at the wrong offset. On Windows a positional file is now opened again by its path, and the response reads through that handle. The read at an offset on the program's handle stays as the fallback when the file does not open again. The test with two responses in flight now checks the position of the descriptor on every platform.
Problem
new Response(Bun.file(fd))for a descriptor that the program wrote through. The response isContent-Length: 16and 0 bytes.Range: bytes=0-3answers206with the bytes at the position.FileResponseStream::start(src/runtime/server/FileResponseStream.rs:245) reads at the offset only whenoffset > 0. At offset 0 it callsread(2), which starts at the position of the descriptor and moves it.Fix
do_sendfilesets a newStartOptions.positionalfor a regularBun.file(fd), andstartthen reads at the offset, also at 0.startreads through a second handle.fstatand thesendfile(2)backend already sends from the offset.test/js/bun/http/bun-serve-file.test.tsfail on main. Self-reviewed: 30 concerns raised, 28 addressed.Background
FileResponseStreamsends aBun.servefile body:sendfile(2)for plain TCP on Linux from 1 MiB, aBufferedReaderotherwise.preadfor every regular file (2SetFilePointerExper read on Windows) and a head that follows the position (Notes).Downsides
Bun.serveaBun.file(fd)body is an absolute byte range,Bun.stdinincluded. Bun.stdin.stream(), process.stdin: read an unsliced file from its current offset #41687 proposes the opposite for.stream().Bun.file(fd)slice still declares the whole file's size, so curl andfetchstill fail. Bun.serve: write the Content-Length of a file response from the window it streams #44826 fixes that. Land this PR first or together..text. Static routes andBun.file(path)keep their syscalls.Notes
Where this comes from. #44826 (found by a fuzzer of
Bun.servefile responses) lists this as a limit it leaves open. #33873 made the sameFileResponseStreamchange in July, together with a change toBun.file(fd).text(). The stale bot closed it with no review. This PR takes only theBun.servepart and sets the flag only for a descriptor that the server did not open. A program on a released version can passBun.file(path).Who reaches it. A handler that returns
Bun.file(fd)for a regular file when the descriptor is not at byte 0: the program wrote or read through it, it is served a second time, or it isBun.stdinfrom a file. It needs the reader backend: under 1 MiB on Linux plain HTTP, every size over TLS, on macOS and on Windows.Observed (Linux x64, plain HTTP, one 16-byte file
0123456789ABCDEF). v1.3.12 and v1.3.13 are the official binaries. main is a release build of 3d6c7bc, and the last column is the same commit with this change.Bun.file(fd), descriptor written through,fetch()ECONNRESETECONNRESETBun.file(fd)at byte 3,Range: bytes=0-3bytes 0-3/16,3456bytes 0-3/16,0123Bun.file(fd).slice(0, 4)at byte 3, curl0123789A0123fetch()ECONNRESETECONNRESETECONNRESETECONNRESETThe failing
fetch()and curl cells wait for the idle timeout first. More rows, main against this PR, with a raw client that sendsConnection: close:slice(0, 4)three times on one descriptor0123,4567,89AB0123,0123,0123Bun.file(fd)twicesendfile)new Response(Bun.stdin), a 19-byte file on stdin, two requestsContent-Length: 19, then 0 bytesWhat is a regression. Only the whole-file body on plain HTTP on POSIX: v1.3.12 sent it with
sendfile(2)at any size. Over TLS v1.3.12 read the file into memory from the position and framed what it read: a descriptor at byte 3 gotContent-Length: 13and 13 bytes. v1.3.13 and main sendContent-Length: 16and those 13 bytes. This PR sends all 16, so for TLS it defines the behavior and does not restore it.Rangesupport arrived with #29202, so the Range row is a defect of that support. Aslice(0, n)of a descriptor was mis-framed in v1.3.12 and still is.Why a slice is a byte range of the file.
BunFile.slicedocumentsendas "absolute offset in bytes (relative to 0)" (packages/bun-types/bun.d.ts:2684).docs/runtime/http/routing.mdx:248says "To send part of a file, use theslice(start, end)method" withstart = 0. The test "Range header cannot escape a Bun.file().slice(0, n) window via fetch handler" pins the slice as a window for a path. For a whole file no text says what the position means. That is the decision in Downsides.Considered.
Content-Length= size minus position). A program has no API to put a descriptor back at byte 0, so one descriptor could serve one response. Two responses in flight share the position: the 16 MiB row above. ARangereply and thesendfile(2)backend need absolute bytes in any case.Bun.file(fd)body down the stream path, whichclone()andtee()already take and which reads at the offset on main. It gives upsendfile(2)andRange, and Bun.stdin.stream(), process.stdin: read an unsliced file from its current offset #41687 changes that path to the position.preadfor every regular file. It changes the syscalls of static routes (serve-file-slice-read-error.test.tscounts 3readcalls) and costs 2SetFilePointerExper read on Windows.With #41687. On main a handler that returns a response after
clone(), atee()branch,pipeThrough()orHTMLRewriter.transform()over the sameBun.file(fd)already sends the bytes from byte 0, because the server then reads a stream of the file. The new table test has those rows beside the rows that this PR fixes, so the two readers must agree. #41687 makes that stream read from the position when the offset is 0. If both PRs land as they are,return responseand the same response afterclone()send different bytes. The decision in Downsides settles which one changes.With #44826. The diffs do not overlap in
src/. If #44826 lands alone, aBun.file(fd)slice at byte 0 turns from a failed response into a well-framed one with the wrong bytes (Content-Length: 4over4567). So this PR lands first or together. The test helperwireResponsehere is the one from #44826, byte for byte, so the test files merge. The PR that lands second adds theContent-Lengthassertion for aBun.file(fd)slice.Measurements (release builds, base 3d6c7bc, Linux x64).
Bun.file(path)andBun.file(path).slice(0, n):read10,pread640,fstat10 on both builds.Bun.file(fd):read10 with 1 of 10 bodies complete on main,pread6410 with 10 of 10 complete here.objdumpof the symbols):FileResponseStream::start288 to 290, nothing per read.do_sendfile967 to 971,FileRoute::on775 to 777,DirectoryRoute::on1440 to 1439..text80,509,029 to 80,509,285 bytes on Linux x64. The stripped binary has the same file size. The function that only Windows compiles is not in these numbers.size_of::<StartOptions>()96 to 104 bytes.Bun.file(fd)body costs one path lookup, one open, twofstatand one close per response, and its reads have no offset. A static route pays nothing. Process Monitor on Windows can measure it.Who reads what on main (a
Bun.file(fd)of a regular file)..text(),.bytes(),.arrayBuffer(),.json(), offset 0ReadFileUV)slice(a, b)witha > 0lseektoa, thenread: right bytes, moves the position.stream().stream()with #41687 as it stands, offset 0Bun.servebody, offset 0, mainsendfilefrom 1 MiB: at the offset)Bun.servebody, this PRpread), position not movedThe Windows column is from the code. I ran nothing on Windows or macOS myself. The new tests pass on the Windows and macOS lanes of CI build 126929.
Same idiom, not changed here.
ReadFile(src/runtime/webcore/blob/read_file.rs:736): the.text()family reads from the position at offset 0. To tellslice(0, n)from a whole file whose.sizewas read, a Blob needs a flag that main does not have. fix(io): Fix staleBlobcache and truncation inBun.write#25702 got a change request for such a flag. Fora > 0thelseekmoves the position of the caller, which needs no decision and is a separate change.NodeFS::read_file_with_options(src/runtime/node/node_fs.rs:7080): aBun.file(fd)as aFormDatapart or afetchupload body reads from the position. With the descriptor at byte 3, aslice(0, 4)part uploads3456on main and with this PR.respondWithFDat offset 0 (src/js/node/http2.ts:3116): node:http2: port node's respondWithFD() and respondWithFile() response paths #43564.Bun.write(dst, Bun.file(fd).slice(0, n)): Honor Bun.file().slice() bounds in Bun.write #41209.Limits.
read(2)at offset 0. No test pins that half of the flag: the response of an eventfd never ends on main.readSyncorwriteSyncby the program at that moment can land at the wrong offset. Sostartopens the file again by its path (get_fd_path), checks that it is the same file (volume and file index), and reads through that handle. That also covers aslice(a, b)body witha > 0, which had this exposure on main. If the file does not open again (no shared read access, or its name is gone), the read falls back to the libuv read at the offset. A region that the program locked withLockFilecannot be read through the second handle.pread(FUSE withFOPEN_NONSEEKABLE) now fails withESPIPEat offset 0, whereread(2)worked. By code, not run..stream()and aslice(a, b)body witha > 0already read such a file withpreadon main.open("/dev/fd/N")duplicates descriptor N (fd(4)), soBun.file("/dev/fd/N")shares the position of N. Not covered, not run.positionaltoo.Self-review. Two of the 30 concerns are taken in part. The tests stay in their own block and do not move into the table that #44826 adds, because that table is not on main. The stream rows are a response after
clone(),tee(),pipeThrough()andHTMLRewriter. A row that returns the clone itself is left out: the type ofResponse.clone()in the test setup is not the type a handler returns.After review. The first version read through libuv at the offset on Windows too. A review showed that a write by the program during a response could then land at the wrong offset, so Windows got the second handle. The stream rows of the table test now use
fetch(), because on Windows theHTMLRewriterresponse is chunked. The Windows code is type-checked forx86_64-pc-windows-msvc, and CI build 126929 passed on every lane with it (181 jobs).Suites run on a debug ASAN build:
bun-serve-file.test.ts,serve-file-slice-read-error.test.ts(still 3readcalls),bun-file-fd-read.test.ts,bun-stdin-slice.test.ts(with a 60 s timeout: its tests take 5 to 10 s on that build),body-clone.test.ts,bun-serve-static.test.ts,test/regression/issue/20965.test.ts. The 7 new tests also fail on a release build of main and pass on a release build of this PR.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/bun-serve-file.test.ts