Bun.serve: serve full content for Bun.file() bodies whose stat size is 0 (procfs) - #34242
Bun.serve: serve full content for Bun.file() bodies whose stat size is 0 (procfs)#34242robobun wants to merge 5 commits into
Conversation
new Response(Bun.file("/proc/self/status")) served 200 Content-Length: 0
with an empty body, while Bun.file(p).text() and the .stream() route both
returned the ~1.5 KB content. procfs/sysfs regular files report st_size == 0
by design but are readable; do_sendfile trusted the stat, clamped remain to
0, wrote Content-Length: 0, and ended with no body.
For an unsliced Bun.file() body whose stat size is 0, fall back to a
read-to-EOF BufferedReader path (no upfront Content-Length, length: None)
so the body is actually read. FileRoute (static routes) had the same
clamping and gets the same fallback. FileResponseStream.on_reader_done now
terminates the response with resp.end() when BufferedReader reports EOF
without ever delivering a chunk, so a genuinely empty file still serves
correctly.
|
Updated 12:31 PM PT - Jul 15th, 2026
❌ @robobun, your commit 91e1e08 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 34242That installs a local version of the PR into your bun-34242 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
Neither suggested issue is a match:
Leaving the PR without |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
WalkthroughZero-stat-size regular files such as procfs files now stream to EOF without inferred range or content-length framing. Empty-chunk EOF responses complete directly, and Linux regression coverage verifies procfs and genuine zero-byte file behavior. ChangesZero-size file streaming
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Not a duplicate of #33360. Same filesystem quirk, disjoint code paths:
The repro here uses a fresh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/server/FileRoute.rs`:
- Around line 438-444: Update the stream_to_eof condition in FileRoute’s
file-serving logic to require S::ISREG(mode) in addition to the existing
FileType::File, zero-size, unsliced checks. Preserve the current behavior for
actual regular files while preventing zero-size block devices and other
non-regular files from being read unboundedly.
In `@src/runtime/server/RequestContext.rs`:
- Around line 1825-1836: The zero-stat regular-file classification currently
only affects GET/sendfile handling, while HEAD still derives and advertises a
zero content length. Reuse the stream_to_eof classification in
do_render_head_response when resolving the Blob size so unsliced procfs/sysfs
files omit the derived Content-Length consistently, and add a HEAD regression
assertion for this case.
In `@test/js/bun/http/serve.test.ts`:
- Around line 2028-2049: Update the response-header assertions in both the
fetch-handler path and static-route path to require that
res.headers.get("content-length") is absent rather than merely nonzero. Keep the
existing status and body-content assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a6ec4e2-371e-45e1-9b4e-86591467cd96
📒 Files selected for processing (4)
src/runtime/server/FileResponseStream.rssrc/runtime/server/FileRoute.rssrc/runtime/server/RequestContext.rstest/js/bun/http/serve.test.ts
…tat files FileRoute's stream_to_eof now requires S::ISREG(mode) rather than the FileType::File fallback bucket, so a zero-size block device is not read unboundedly. do_render_head_response omits Content-Length for an unsliced file blob that resolves to size 0, matching the GET path which now streams to EOF without a stat-derived length. Tightened the test's Content-Length assertion to null-or-exact-body-length, and added a HEAD assertion.
HEAD cannot distinguish a genuinely empty regular file from a procfs file without reading, and bun-serve-file.test.ts locks down Content-Length: 0 for HEAD on empty files. Revert the do_render_head_response change and exclude HEAD from FileRoute's stream_to_eof so HEAD keeps its existing framing; only GET takes the read-to-EOF fallback.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/http/serve.test.ts`:
- Around line 2003-2063: Add a HEAD request case to the existing procfs serving
test, preferably for the static `/route` path, and assert it returns status 200,
an empty response body, and `Content-Length: 0`. Keep the existing GET and real
empty-file assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8ef054f-962c-4de3-a7b1-beb696623d4e
📒 Files selected for processing (2)
src/runtime/server/FileRoute.rstest/js/bun/http/serve.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/runtime/server/RequestContext.rs:2646-2652— Minor: this guard can't distinguish a procfs file (stat=0, has content) from a genuinely empty regular file (stat=0, no content), so HEAD on a real empty file now omitsContent-Lengthentirely, whereas GET on the same file returnsContent-Length: 0(viaresp.end(b"")inon_reader_done). The same divergence exists inFileRoutevia the!stream_to_eofgate at FileRoute.rs:563. This is RFC 9110 §9.3.2-compliant and unavoidable without reading on HEAD, so it's a reasonable tradeoff — but the test's/emptycase only exercises GET; consider adding aHEAD /emptyassertion to lock down the intended behavior.Extended reasoning...
What changed and how it manifests
Before this PR,
do_render_head_responseon a genuinely empty regular file calledblob.resolve_size()→blob_size == 0→ fell into the else-branch and wrotecontent-length: 0. After this PR,was_unsliced_fileistruefor a freshBun.file(emptyPath)(needs_to_read_file && offset == 0 && size == MAX_SIZE),resolve_size()setsblob_size = 0, and the new guard!(was_unsliced_file && blob_size == 0)skips the header write entirely.end_without_body()then writes only the terminating\r\n, so the HEAD response has noContent-Lengthat all.The same holds in
FileRoute::on: for an unsliced empty regular file,stream_to_eofistrue, so the!stream_to_eofgate at line 563 skips thecontent-lengthwrite, and themethod == HEADbranch callsend_without_bodywith no CL header.GET vs HEAD divergence
GET on the same empty file takes the stream-to-EOF path:
length: Noneis passed toFileResponseStream, the firstread()returns 0 bytes,BufferedReaderskipson_read_chunkfor empty chunks and callson_reader_donedirectly, which (per this PR's other change at FileResponseStream.rs:310-317) now callsresp.end(b"", ...). uWS'send()writesContent-Length: 0for an empty body. So GET returnsContent-Length: 0but HEAD returns no header — a divergence that didn't exist before this PR (both used to returnContent-Length: 0).Step-by-step proof
- User serves
new Response(Bun.file("/tmp/empty"))where/tmp/emptyis a 0-byte regular file. - HEAD request →
do_render_head_response:was_unsliced_file = true(it's a file blob, offset=0, size=MAX_SIZE)resolve_size()stats the file →blob_size = 0!(true && 0 == 0)=false→ skipwrite_header_intend_without_body→ response has noContent-Length
- GET request →
do_sendfile:stream_to_eof = true(is_regular, stat_size=0, offset=0, original_size=MAX_SIZE)length: Nonepassed toFileResponseStream::start- First
read()hits EOF with 0 bytes →on_reader_donefires withoutRESPONSE_DONEset resp.end(b"", ...)→ uWS writesContent-Length: 0
- Result: GET has
Content-Length: 0, HEAD has none.
Why this isn't a merge blocker
The refutation is correct that this is intentional and documented — the code comment at line 2643-2645 cites RFC 9110 §9.3.2, which explicitly permits a server to omit on HEAD any header field "for which a value is determined only while generating the content." At HEAD time the server genuinely cannot know whether a 0-stat file is empty or procfs without reading it, and reading defeats HEAD's purpose. The alternatives are (a) write
CL: 0, which lies for procfs — the very bug this PR fixes; or (b) do a filesystem-type check (statfs, path prefix), which is fragile. Omitting is the only choice that's correct for both cases, and it's spec-sanctioned. The pre-PR behavior was actively wrong for procfs (advertisedCL: 0for a 1.5KB file); trading a technically-correct-but-uninformativeCL: 0on empty files for correctness on procfs is the right call.Practical impact is minimal: genuinely empty files served over HTTP are rare, and clients that use HEAD to probe size must already tolerate absent
Content-Length(chunked/dynamic responses are common). RFC 9110 §8.6 says CL is optional on HEAD.What's actionable
The test's
/emptycase at serve.test.ts:2069-2073 only exercises GET. Since the HEAD behavior on empty files is now different from before (and different from GET), it's worth locking down with an explicit assertion — e.g.,expect((await fetch(new URL("/empty", server.url), { method: "HEAD" })).headers.get("content-length")).toBeNull()— so a future change doesn't accidentally reintroduceCL: 0(which would re-break procfs HEAD) or start reading the file on HEAD. That's a test-coverage nit, not a code fix. - User serves
|
The finding above reviewed 5c9bf80, which was superseded by c192eee before the review posted (the header notes "PR may have been updated during review"):
So there is no GET/HEAD divergence for genuinely empty files at c192eee. The |
|
The diff is green on the tests it touches ( |
There was a problem hiding this comment.
I didn't find bugs, but this touches Bun.serve's file-response streaming across three sites, including a change to FileResponseStream::on_reader_done that swaps the fall-through end_without_body() for resp.end(b"") — reachable by more than just the new procfs path — so it warrants a human look.
Checked: stream_to_eof in both do_sendfile and FileRoute::on is gated on S_ISREG && stat_size == 0 && unsliced, with HEAD excluded in FileRoute.
Checked: sendfile.remain is left at MAX_SIZE under stream_to_eof but unused — needs_content_length is cleared and length: None is passed to the stream.
Checked: on_reader_done was already reachable with !RESPONSE_DONE for pipes/sockets that EOF on first read; the end(b"") swap looks correct there too (no Content-Length was written), but worth confirming.
Checked: Range resolution is skipped for zero-stat files; a Range request now gets a 200 full body instead of 206/416, which seems right for unknown-length content.
Extended reasoning...
Overview
The PR fixes Bun.serve returning empty bodies for Bun.file() responses backed by procfs/sysfs/cgroupfs regular files that report st_size == 0 but yield real content on read(). It touches three files in src/runtime/server/:
RequestContext.rs(do_sendfile): adds astream_to_eofclassifier and gates Content-Length, Content-Range, sendfile-remain clamping, Range resolution, the empty-body early-exit, and thelengthpassed toFileResponseStreamon!stream_to_eof.FileRoute.rs(on): mirrors the same classifier for static routes, additionally excludingmethod == HEAD; threads anis_regularflag out of the stat block to gate onS_ISREG(mode)rather than theFileType::Filefallback.FileResponseStream.rs(on_reader_done): when the reader signals done withoutRESPONSE_DONEset (i.e. EOF at first read, no chunk ever delivered), terminates viaresp.end(b"", ...)instead of falling through tofinish()→end_without_body().
A Linux-only test in serve.test.ts covers the fetch-handler path, the static-route path, and a genuine 0-byte file.
Security risks
None identified. The change narrows to is_regular && stat_size == 0 && offset == 0 && size == MAX_SIZE, so it cannot be used to unbound-read block devices (CodeRabbit's earlier concern, addressed in 5c9bf80). No user-controlled input reaches new parsing or allocation logic. Range handling is disabled for these files, not extended.
Level of scrutiny
High. This is core Bun.serve file-serving logic — a hot, production-critical path shared by both the fetch-handler and static-route sides. The on_reader_done change alters response-termination semantics for a codepath that predates this PR: pipes and sockets that EOF on first read previously reached finish() with !RESPONSE_DONE and called end_without_body(); they now call resp.end(b""). The PR description argues the old behavior left clients hanging when no Content-Length was written, which is plausible, but the swap affects more than the new procfs case and deserves a maintainer's eye on the uWS framing implications.
The PR also went through a design reversal on HEAD (5c9bf80 omitted Content-Length for zero-stat HEAD, c192eee reverted after bun-serve-file.test.ts caught a regression on real empty files). The final position — HEAD keeps stat-derived Content-Length: 0 because it can't distinguish procfs from a real empty file without reading — is reasonable but is a tradeoff a human should ratify.
Other factors
- Test is Linux-only (inherent to procfs); no coverage on macOS/Windows, though the
stream_to_eofbranch is unreachable there absent a zero-stat regular file. - All CodeRabbit threads are resolved; no human reviewer has weighed in yet.
- The bug-hunting system found nothing; I independently traced the
stream_to_eofguards through both call sites and theFileResponseStreamstart options and did not find leaks, double-closes, or unclamped sendfile state escaping into the reader path.
|
On the
Either way the new block only runs when |
1 participant
Repro
Bun.file(P).text()andnew Response(Bun.file(P).stream())both return the full ~1.5 KB; only theResponse(Bun.file)path served empty. procfs, sysfs and cgroupfs regular files reportst_size == 0by design but yield content onread(). Node'shttp+createReadStreamserves the content.Cause
RequestContext::do_sendfilestats the fd, sets the blob size andsendfile.remaintostat_size, writesContent-Length: <stat_size>, and foris_regular && remain == 0ends immediately with an empty body. For a procfs file that isContent-Length: 0and zero reads. The blob is fresh (.sizenever touched); this is the serve path doing its own stat, not the known cached-size issue.Fix
When a regular file stats as 0 bytes and the blob is the unsliced whole file (
offset == 0 && size == MAX_SIZE), skip the stat-based framing: no upfrontContent-Length, noremainclamp, no Range resolution, and handlength: NonetoFileResponseStreamso it reads to EOF via the BufferedReader path. The same condition was added toFileRoute(staticroutes:with aBun.filebody) which had the identical clamp.FileResponseStream::on_reader_donenow terminates the response withresp.end(b"", ...)whenBufferedReadersignals done without ever having delivered a chunk. Previously that fell through toend_without_body(), which with headers already written and noContent-Lengthleaves the client hanging. This keeps a genuinely empty 0-byte file on a real filesystem serving correctly (oneread()hits EOF, uWS writesContent-Length: 0).Verification
The test is Linux-only (procfs). It covers both the fetch-handler path and the static-route path, and asserts a real 0-byte file still serves as empty.
no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts