Bun.serve: serve character-device Bun.file() bodies instead of closing with zero bytes - #34257
Bun.serve: serve character-device Bun.file() bodies instead of closing with zero bytes#34257robobun wants to merge 5 commits into
Conversation
…g with zero bytes
On Linux, returning new Response(Bun.file("/dev/null")) (or any
character device whose file_operations lack .poll, such as /dev/zero or
/dev/full) from a Bun.serve fetch handler closed the connection with no
status line, no headers, and no error() callback. strace showed
epoll_ctl(EPOLL_CTL_ADD, chardev-fd) returning EPERM followed by a
pre-header teardown.
do_sendfile classifies S_ISCHR as (FileType::Pipe, pollable = true) and
hands the fd to FileResponseStream, whose BufferedReader tries to
register it with the main event loop's epoll. For /dev/null-class
devices epoll_ctl returns EPERM; PosixBufferedReader::register_poll
dispatched that through on_reader_error, which FileResponseStream's
fail_with handled by force-closing the socket and discarding any corked
headers. The handler's error() callback is never consulted because
on_file_stream_error only cleans up after the socket is already gone.
PosixBufferedReader::start now recovers from an EPERM on the initial
registration by dropping the never-registered FilePoll and continuing on
the non-pollable (blocking-read) path, the same fallback IOWriter
already uses for EPERM/EINVAL on the writer side. The character devices
epoll rejects are exactly those with no .poll hook, which the kernel
treats as always-readable, so the non-pollable read loop drives them to
EOF without needing readiness notifications.
For the serve path itself, RequestContext::do_sendfile no longer writes
Content-Length for non-regular files (the stat size is meaningless
there) and passes the user's .slice() length through to
FileResponseStream so a sliced /dev/zero stops after that many bytes
instead of streaming forever. FileResponseStream::finish now ends an
immediately-EOF stream with resp.end("") rather than
end_without_body(), so uWS supplies the missing framing and the client
is not left waiting on a headerless body.
|
Updated 9:47 PM PT - Jul 16th, 2026
❌ @Jarred-Sumner, your commit 05fad3d has 4 failures in
🧪 To try this PR locally: bunx bun-pr 34257That installs a local version of the PR into your bun-34257 --bun |
WalkthroughChangesServing non-regular files now handles unsupported poll registration, selects appropriate HTTP framing, emits explicit empty responses at EOF, and adds Linux raw TCP regression coverage for Character-device file serving
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this touches shared I/O infrastructure (PosixBufferedReader::start is used by subprocess/shell/file readers) and changes HTTP response framing, so it's worth a human look.
What was reviewed:
- EPERM fallback in
start(): verifiedclose_impl(.., close_fd=false)releases the never-registeredFilePollwithout closing the fd beforehandleis reset toFd(fd). end_without_body→end(b"")infinish(): checked that the chunked-EOF path is only reached viaon_reader_donefor non-regular files; regular-file EOF still ends viaon_read_chunk.- Ruled out: non-regular file body capped to 0 when JS reads
.sizefirst —original_sizeis captured before the stat-size clamp.
Extended reasoning...
Overview
Four files: PosixBufferedReader gains a try_register_poll split so start() can catch EPERM from epoll_ctl(ADD) on Linux and fall back to the non-pollable read path (mirroring IOWriter::__start). RequestContext::do_sendfile stops emitting Content-Length for non-regular files and threads an explicit .slice() length through as the FileResponseStream cap. FileResponseStream::finish swaps end_without_body for end(b"") so uWS supplies chunked framing when no length header was written. Two new tests cover /dev/null and /dev/zero.slice() via raw TCP + fetch.
Security risks
None identified. No user-controlled parsing, no auth/crypto. The path is server-side response streaming of an fd the handler already opened.
Level of scrutiny
High. PosixBufferedReader::start is on the hot path for every pollable-fd reader in the runtime (subprocess stdout/stderr, shell pipes, FIFO responses), and FileResponseStream::finish / set_needs_content_length(is_regular) change framing for all non-regular file bodies, not just character devices. The reasoning is sound and the existing FIFO backpressure test is claimed to still pass, but a maintainer should confirm the framing change is desired for FIFOs (previously Content-Length: <stat_size>, now chunked) and that the end(b"") swap doesn't regress any path that previously relied on end_without_body semantics.
Other factors
- The EPERM fallback is
#[cfg(linux|android)]-gated but the new tests aredescribe.skipIf(isWindows), so they also run on macOS — worth confirming kqueue doesn't hit an analogous registration failure there (or that /dev/null registers cleanly on Darwin). - The PR notes overlap with #34242 in the same functions; merge order may matter.
- CI build was still in progress at review time; no green build to reference.
macOS kqueue returns EINVAL (not EPERM) when registering a character device like /dev/null for EVFILT_READ, so the same zero-byte-close reproduces there. Extend the fallback to cover EINVAL on all POSIX in addition to EPERM on Linux, matching the existing IOWriter::__start handling.
|
Good catch on macOS: kqueue returns On the FIFO framing change: before this PR, a FIFO body from the fetch handler wrote |
There was a problem hiding this comment.
The macOS EINVAL concern from my earlier comment is addressed in c45a338 — the fallback now matches both branches of IOWriter::__start. I didn't find further issues, but this touches core I/O poll-registration fallback (shared by every PosixBufferedReader::start caller) plus HTTP framing semantics, so it's worth a human look.
Checked: close_impl(.., false) drops the never-registered FilePoll without closing the fd before it's reinstated as PollOrFd::Fd; the end_without_body → end(b"") path in finish() is only reached when no EOF chunk set RESPONSE_DONE, so regular files with a written Content-Length still terminate via on_read_chunk; the new original_size != MAX_SIZE branch only adds a cap for sliced non-regular files and leaves the unsliced None path unchanged.
Extended reasoning...
Overview
Four files: PosixBufferedReader::start in src/io/PipeReader.rs gains an EPERM/EINVAL fallback that drops the failed FilePoll and continues on the non-pollable read path; FileResponseStream::finish swaps end_without_body for end(b""); RequestContext::do_sendfile gates needs_content_length on is_regular and threads an explicit .slice() length through to the stream for non-regular fds; two new tests in bun-serve-file.test.ts cover /dev/null and sliced /dev/zero.
My earlier inline comment flagged that the first revision only handled Linux EPERM and would fail the new tests on macOS (kqueue → EINVAL). c45a338 added the unconditional-POSIX EINVAL arm; the current fd_not_pollable predicate now mirrors both branches of IOWriter::__start.
Security risks
None identified. No user-controlled input reaches the new code paths beyond the fd's stat mode, which was already being classified. The Content-Length change removes a pre-existing framing violation (CL:0 followed by streamed FIFO bytes) rather than introducing one.
Level of scrutiny
High. PosixBufferedReader::start is shared by subprocess pipes, shell readers, and file-response streaming, so widening the set of registration errors that silently fall back to blocking reads affects every consumer. The finish() framing change and the set_needs_content_length(is_regular) change alter wire-level HTTP behavior for all non-regular file bodies (FIFOs included, not just character devices). These are well-reasoned and covered by the existing FIFO backpressure test plus the two new tests, but they are behavior changes in production-critical paths, not mechanical fixes.
Other factors
try_register_pollcallsenable_keeping_process_alivebeforeregister_with_fd; on the fallback path the poll is torn down viaclose_impl(.., close_fd=false). I did not fully trace whether that path balances the keep-alive ref — worth a glance from someone who ownsFilePoll.- CI on c45a338 is still building with unrelated-looking build-cpp failures on aarch64-android and x64-musl-baseline; test jobs haven't reported yet.
- The PR notes overlap with #34242 in the same functions.
There was a problem hiding this comment.
The macOS EINVAL branch is in place now — c45a338 addresses the earlier finding and matches both arms of IOWriter::__start. No further issues found, but this reaches into shared I/O plumbing and changes HTTP framing, so it's worth a human look.
Checked: close_impl(.., false) on the never-registered FilePoll frees the allocation without closing the fd before handle is restored to Fd(fd); the .size-read-before-serve path on non-regular files was examined and does not zero-cap the response; set_needs_content_length(is_regular) only drops the bogus Content-Length: 0 that FIFOs already emitted.
Extended reasoning...
Overview
Four files: PosixBufferedReader::start in src/io/PipeReader.rs gains an EPERM/EINVAL fallback (refactored via a new try_register_poll); RequestContext::do_sendfile stops writing Content-Length for non-regular files and threads the user's .slice() length through as a cap; FileResponseStream::finish swaps end_without_body() for end(b"") so an immediately-EOF chunked stream terminates; two new tests in bun-serve-file.test.ts cover /dev/null and /dev/zero.
Security risks
None identified. No untrusted-input parsing, no auth/crypto surface. The framing change (chunked instead of Content-Length: 0 for non-regular files) is a correctness fix — the old behavior was already an HTTP framing violation when body bytes followed a zero Content-Length.
Level of scrutiny
Medium-high. PosixBufferedReader::start is shared infrastructure — the shell PipeReader, FileReader, subprocess pipes, and FileResponseStream all call it. The fallback is scoped to the initial registration in start() and mirrors an existing pattern in IOWriter::__start, so risk is bounded, but it silently changes error handling for every caller that passes is_pollable=true on a non-pollable fd. The end_without_body → end(b"") swap in finish() and the Content-Length gating on is_regular are user-visible protocol changes for FIFO/socket bodies. The author notes this composes with #34242, which touches overlapping lines.
Other factors
My earlier finding (Linux-only #[cfg] on the EPERM arm leaving macOS broken) was addressed in c45a338; the current fallback matches IOWriter::__start's two-branch shape (EINVAL on all POSIX, EPERM on Linux/Android). I traced close_impl(None, None, false) on the failed-registration path: it deinit_force_unregisters the FilePoll allocation and, with close_fd=false, leaves the fd open before handle is set back to PollOrFd::Fd(fd). A finder raised whether reading .size on a non-regular Bun.file before serving would cap the response at 0 — ruled out. The FIFO backpressure test and the rest of bun-serve-file.test.ts reportedly still pass, but the framing change for FIFOs and the shared-reader fallback are the pieces I'd want a maintainer to sanity-check.
|
CI status: the diff is green. The new character-device tests in Red lanes are unrelated to this change:
Build 73338 was red only on two Ready for review. |
There was a problem hiding this comment.
No issues found on this pass, but this touches shared low-level I/O (PosixBufferedReader::start, used by shell/subprocess/file-response readers) and changes HTTP framing for all non-regular file bodies, so it's worth a human look.
What was reviewed:
- The EPERM/EINVAL fallback now matches both branches of
IOWriter::__start; my earlier macOS comment is addressed. close_impl(.., false)on the failed-registrationFilePolldeinits the poll allocation without closing the underlying fd before it's restored toPollOrFd::Fd(fd).- The
end_without_body()→end(b"")swap infinish()is guarded by!RESPONSE_DONE, so the streamed-EOF path (which alreadyend()s inon_read_chunk) is unaffected; only immediately-EOF non-regular fds hit it. - Candidate "non-regular body capped at 0 if JS read
.sizefirst" was examined and ruled out.
Extended reasoning...
Overview
Four files: PosixBufferedReader::start gains an EPERM/EINVAL recovery path that drops a never-registered FilePoll and continues on the non-pollable read loop (mirroring the existing IOWriter::__start writer-side fallback); RequestContext::do_sendfile stops writing Content-Length for non-regular files and forwards an explicit .slice() length as the stream cap; FileResponseStream::finish switches end_without_body() → end(b"") so an immediately-EOF stream with no Content-Length still emits valid framing; two new tests cover /dev/null and /dev/zero.slice().
Security risks
None identified. The change widens what fds the reader will attempt to read on the non-pollable path, but only for fds the caller already opened and handed in; no new user-controlled input reaches a parser or allocation size.
Level of scrutiny
High. PosixBufferedReader::start is shared by subprocess stdout/stderr, shell pipe readers, and HTTP file responses — a regression here would affect all of them. The set_needs_content_length(is_regular) change also alters framing for FIFO/socket bodies that previously (incorrectly) wrote Content-Length: 0; that's a behavior change for a whole class of responses, not just /dev/null. The register_poll → try_register_poll refactor is behavior-preserving for existing callers (the public register_poll still dispatches on_reader_error on failure), but a maintainer should confirm the enable_keeping_process_alive counter incremented before the failed register_with_fd is correctly balanced by deinit_force_unregister on the fallback path.
Other factors
My prior review flagged the missing macOS EINVAL branch; c45a338 addressed it and the current diff matches both IOWriter::__start arms. CI is reported green on Linux/macOS/Windows including the existing FIFO backpressure test in the same file. The bug-hunting system raised and refuted a ".size read before serving caps at 0" concern. Given the reach of PosixBufferedReader and the framing change, deferring rather than approving.
…nt-Length: 0 (#36243) ### Repro ```js // mkfifo /tmp/p; (sleep 0.3; printf 'PIPEBYTES!' > /tmp/p) & const srv = Bun.serve({ port: 0, fetch: r => new URL(r.url).pathname === '/fifo' ? new Response(Bun.file('/tmp/p')) : new Response('SECOND-RESPONSE') }); // raw keep-alive client: GET /fifo, then GET /plain ``` Wire capture on one keep-alive connection, before: ``` HTTP/1.1 200 OK content-type: application/octet-stream content-disposition: filename="p" content-length: 0 Date: ... PIPEBYTES!HTTP/1.1 200 OK <- pipe body glued onto response 2's status line content-length: 15 ... SECOND-RESPONSE ``` The first response declares `Content-Length: 0` and then still streams the FIFO's bytes after the head. A client that honours the declared length parses `PIPEBYTES!...` as the next response's status line (RFC 9112 6.3), i.e. a framing desync on the keep-alive connection. ### Cause `RequestContext::do_sendfile` classifies `S_ISFIFO`/`S_ISCHR`/`S_ISSOCK` as non-regular (`is_regular = false`), sets the blob size to `min(original_size, st_size) = 0` (a FIFO stats as 0 bytes), then unconditionally sets `needs_content_length = true`, so `render_metadata` writes `content-length: 0` and marks the header as written. `FileResponseStream` is then started with `length: None` and streams the fd to EOF via `resp.write()`; with the Content-Length mark set, uWS emits those bytes raw instead of chunk-framing them. Two framing decisions that disagree. `FileRoute` (static file routes) already gates the Content-Length header on `file_type == File`, so only the fetch-handler path is affected. ### Fix Set `needs_content_length` only for regular files. With no Content-Length marked, uWS's `write()` enters chunked mode on the first body chunk (and `end()` writes a correct `Content-Length: N` when the whole body arrives in the single terminating read). The regular-file sendfile fast path is unchanged. ### Verification ``` $ USE_SYSTEM_BUN=1 bun test test/js/bun/http/bun-serve-file.test.ts -t 'frames the body as chunked' (fail) Response(Bun.file(FIFO)) frames the body as chunked, not Content-Length: 0 { firstHasContentLength: true, firstIsChunked: false, gluedToNextStatusLine: true } $ bun bd test test/js/bun/http/bun-serve-file.test.ts -t 'frames the body as chunked' (pass) Response(Bun.file(FIFO)) frames the body as chunked, not Content-Length: 0 ``` Full `bun-serve-file.test.ts` (104 tests, including the existing FIFO backpressure test) still passes. Related: #34257 changes the same line for a different symptom (character-device epoll `EPERM`); the two compose. <!-- robobun:evidence:begin --> --- **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/bun/http/bun-serve-file.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…nt-Length: 0 (oven-sh#36243) ### Repro ```js // mkfifo /tmp/p; (sleep 0.3; printf 'PIPEBYTES!' > /tmp/p) & const srv = Bun.serve({ port: 0, fetch: r => new URL(r.url).pathname === '/fifo' ? new Response(Bun.file('/tmp/p')) : new Response('SECOND-RESPONSE') }); // raw keep-alive client: GET /fifo, then GET /plain ``` Wire capture on one keep-alive connection, before: ``` HTTP/1.1 200 OK content-type: application/octet-stream content-disposition: filename="p" content-length: 0 Date: ... PIPEBYTES!HTTP/1.1 200 OK <- pipe body glued onto response 2's status line content-length: 15 ... SECOND-RESPONSE ``` The first response declares `Content-Length: 0` and then still streams the FIFO's bytes after the head. A client that honours the declared length parses `PIPEBYTES!...` as the next response's status line (RFC 9112 6.3), i.e. a framing desync on the keep-alive connection. ### Cause `RequestContext::do_sendfile` classifies `S_ISFIFO`/`S_ISCHR`/`S_ISSOCK` as non-regular (`is_regular = false`), sets the blob size to `min(original_size, st_size) = 0` (a FIFO stats as 0 bytes), then unconditionally sets `needs_content_length = true`, so `render_metadata` writes `content-length: 0` and marks the header as written. `FileResponseStream` is then started with `length: None` and streams the fd to EOF via `resp.write()`; with the Content-Length mark set, uWS emits those bytes raw instead of chunk-framing them. Two framing decisions that disagree. `FileRoute` (static file routes) already gates the Content-Length header on `file_type == File`, so only the fetch-handler path is affected. ### Fix Set `needs_content_length` only for regular files. With no Content-Length marked, uWS's `write()` enters chunked mode on the first body chunk (and `end()` writes a correct `Content-Length: N` when the whole body arrives in the single terminating read). The regular-file sendfile fast path is unchanged. ### Verification ``` $ USE_SYSTEM_BUN=1 bun test test/js/bun/http/bun-serve-file.test.ts -t 'frames the body as chunked' (fail) Response(Bun.file(FIFO)) frames the body as chunked, not Content-Length: 0 { firstHasContentLength: true, firstIsChunked: false, gluedToNextStatusLine: true } $ bun bd test test/js/bun/http/bun-serve-file.test.ts -t 'frames the body as chunked' (pass) Response(Bun.file(FIFO)) frames the body as chunked, not Content-Length: 0 ``` Full `bun-serve-file.test.ts` (104 tests, including the existing FIFO backpressure test) still passes. Related: oven-sh#34257 changes the same line for a different symptom (character-device epoll `EPERM`); the two compose. <!-- robobun:evidence:begin --> --- **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/bun/http/bun-serve-file.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2 participants
Repro
straceshowsepoll_ctl(EPOLL_CTL_ADD, <chardev fd>) = -1 EPERMfollowed by the socket being force-closed before any byte is written. Node'shttp+createReadStream("/dev/null")serves an empty 200.Bun.file("/dev/null").text()already returns""correctly because the whole-blob reader runs on the threadpool and never touches the main loop's epoll.Cause
RequestContext::do_sendfileclassifiesS_ISCHRas(FileType::Pipe, pollable = true)and hands the fd toFileResponseStream, whoseBufferedReaderregisters it with the uWS event loop's epoll. For character devices whosefile_operationshave no.pollhook (/dev/null,/dev/zero,/dev/full) Linux returnsEPERMfromepoll_ctl(EPOLL_CTL_ADD).PosixBufferedReader::register_polldispatches that throughon_reader_error, whichFileResponseStream::fail_withhandles byresp.force_close(), discarding any corked headers.on_file_stream_errorinRequestContextonly cleans up after the socket is already gone, so the handler'serror()callback never sees it.FIFOs take the same
(Pipe, true)branch but are epollable, which is why they work.Fix
PosixBufferedReader::startnow handles anEPERMfrom the initial poll registration by dropping the never-registeredFilePolland continuing on the non-pollable read path. The shell'sIOWriter::__startalready does this forEPERM/EINVALon the writer side; this brings the reader to parity. Devices that epoll rejects are exactly those with no.pollhook, whichpoll(2)treats as always-readable (DEFAULT_POLLMASK), so the blocking-read loop drives them to EOF without readiness notifications. TTYs and other pollable character devices keep the epoll path.Two follow-on changes make the serve path produce a well-formed response once the reader succeeds:
RequestContext::do_sendfileno longer writesContent-Lengthfor non-regular files (the stat size is meaningless, and aContent-Length: 0header followed by streamed body bytes is a framing violation). It also passes the user's.slice()length through toFileResponseStreamfor non-regular files soBun.file("/dev/zero").slice(0, n)stops afternbytes instead of streaming indefinitely.FileResponseStream::finishends an immediately-EOF stream withresp.end(b"")rather thanend_without_body(), so uWS supplies the missing framing when noContent-Lengthwas written and the client is not left waiting.Verification
The full
bun-serve-file.test.tssuite (including the existing FIFO backpressure test) still passes.Adjacent to #34242 (procfs regular files with
st_size == 0), which touches overlapping lines inRequestContext.rs/FileResponseStream.rsfor a different stat-size bug; the two compose.no test proof · iteration 5 · 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