From 9219bff91ecd26db615b1d09ef4686f003991096 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 11:05:14 +0000 Subject: [PATCH 1/7] Close the connection when a file response ends short of its Content-Length Bun.serve writes Content-Length for a Bun.file body from the fstat it takes when the response starts. When the file shrinks before the body is fully sent, the read hits EOF short of that length. FileResponseStream ended the response there as if the body were whole, so the request was retired as a complete keep-alive response and the client waited for the missing bytes until the idle timeout (forever with idleTimeout: 0). FileResponseStream now tracks the bytes still owed on a fixed-length body. An EOF before that balance reaches zero force-closes the socket, on the sendfile path (sendfile returns 0) and on the reader path (an EOF chunk, or the reader's done callback with no final chunk). --- src/runtime/server/FileResponseStream.rs | 97 +++++++++++++++++++----- test/js/bun/http/bun-serve-file.test.ts | 72 +++++++++++++++++- 2 files changed, 146 insertions(+), 23 deletions(-) diff --git a/src/runtime/server/FileResponseStream.rs b/src/runtime/server/FileResponseStream.rs index 6c092fcfb2c0..6495cd79d167 100644 --- a/src/runtime/server/FileResponseStream.rs +++ b/src/runtime/server/FileResponseStream.rs @@ -47,6 +47,9 @@ pub(crate) struct FileResponseStream { mode: Cell, reader: JsCell, sendfile: JsCell, + /// Bytes still owed on a fixed-length body (the caller wrote this many + /// into Content-Length). `None` streams to EOF. + remaining: Cell>, state: Cell, } @@ -61,7 +64,6 @@ pub enum Mode { struct Sendfile { #[cfg(any(target_os = "linux", target_os = "android"))] socket_fd: Fd, - remain: u64, offset: u64, #[cfg(any(target_os = "linux", target_os = "android"))] has_set_on_writable: bool, @@ -77,7 +79,6 @@ impl Default for Sendfile { Self { #[cfg(any(target_os = "linux", target_os = "android"))] socket_fd: Fd::INVALID, - remain: 0, offset: 0, #[cfg(any(target_os = "linux", target_os = "android"))] has_set_on_writable: false, @@ -133,6 +134,10 @@ pub(crate) enum StreamOwner { enum StreamEnd { Complete, Abort, + /// The file ended before the length the caller committed to (it shrank + /// after the `fstat`). The socket is force-closed first; the owner only + /// has to release the request. + Truncated, Error(sys::Error), } @@ -147,7 +152,7 @@ impl StreamOwner { on_abort, on_error, } => match end { - StreamEnd::Complete => on_complete(ctx, resp), + StreamEnd::Complete | StreamEnd::Truncated => on_complete(ctx, resp), StreamEnd::Abort => on_abort.unwrap_or(on_complete)(ctx, resp), StreamEnd::Error(err) => on_error(ctx, resp, err), }, @@ -180,6 +185,7 @@ impl FileResponseStream { }), reader: JsCell::new(BufferedReader::init::()), sendfile: JsCell::new(Sendfile::default()), + remaining: Cell::new(opts.length), state: Cell::new(State::default()), })); // SAFETY: `this` is the live allocation above; the guard's ref defers @@ -214,7 +220,6 @@ impl FileResponseStream { #[cfg(any(target_os = "linux", target_os = "android"))] socket_fd: opts.resp.get_native_handle(), offset: opts.offset, - remain: opts.length.expect("can_sendfile gates None"), #[cfg(any(target_os = "linux", target_os = "android"))] has_set_on_writable: false, }); @@ -314,8 +319,13 @@ impl FileResponseStream { let resp = self.resp.get(); resp.timeout(self.idle_timeout.get()); + let remaining = self.charge_remaining(chunk.len() as u64); if state == ReadState::Eof { + if remaining.is_some_and(|n| n > 0) { + self.end_truncated(); + return false; + } self.insert_state(State::RESPONSE_DONE); self.detach_resp(); resp.end(chunk, resp.should_close_connection()); @@ -406,8 +416,8 @@ impl FileResponseStream { fn on_sendfile(&self) -> bool { bun_output::scoped_log!( FileResponseStream, - "onSendfile remain={} offset={}", - self.sendfile.get().remain, + "onSendfile remain={:?} offset={}", + self.remaining.get(), self.sendfile.get().offset ); if self.state.get().contains(State::RESPONSE_DONE) { @@ -433,8 +443,9 @@ impl FileResponseStream { #[cfg(any(target_os = "linux", target_os = "android"))] loop { - let (errno, sent, remain) = self.sendfile.with_mut(|sf| { - let adjusted = sf.remain.min(i32::MAX as u64); + let remain = self.remaining.get().expect("can_sendfile gates None"); + let (errno, sent) = self.sendfile.with_mut(|sf| { + let adjusted = remain.min(i32::MAX as u64); let mut off: i64 = i64::try_from(sf.offset).expect("int cast"); // SAFETY: both fds are valid open file descriptors owned by `self`; // `off` is a stack local. @@ -451,16 +462,23 @@ impl FileResponseStream { u64::try_from((off - i64::try_from(sf.offset).expect("int cast")).max(0)) .unwrap(); sf.offset = u64::try_from(off).expect("int cast"); - sf.remain = sf.remain.saturating_sub(sent); - (errno, sent, sf.remain) + (errno, sent) }); + let remain = self + .charge_remaining(sent) + .expect("can_sendfile gates None"); match errno { sys::E::SUCCESS => { - if remain == 0 || sent == 0 { + if remain == 0 { self.end_sendfile(); return false; } + if sent == 0 { + // EOF before the committed length: the file shrank. + self.end_truncated(); + return false; + } return self.arm_sendfile_writable(); } sys::E::EINTR => continue, @@ -544,13 +562,39 @@ impl FileResponseStream { self.finish(); } + /// Subtracts `sent` from the bytes still owed; returns the new balance + /// (`None` when the body has no fixed length). + fn charge_remaining(&self, sent: u64) -> Option { + let remaining = self.remaining.get().map(|n| n.saturating_sub(sent)); + self.remaining.set(remaining); + remaining + } + fn fail_with(&self, err: sys::Error) { + self.close_with(StreamEnd::Error(err)); + } + + /// The headers already promised a Content-Length that the file no longer + /// backs. A clean end would retire the request as a complete keep-alive + /// response, and the client would wait for the missing bytes until the + /// idle timeout. Closing the connection is the one signal left that the + /// body is short. + fn end_truncated(&self) { + bun_output::scoped_log!( + FileResponseStream, + "endTruncated remaining={:?}", + self.remaining.get() + ); + self.close_with(StreamEnd::Truncated); + } + + fn close_with(&self, end: StreamEnd) { if !self.state.get().contains(State::RESPONSE_DONE) { self.insert_state(State::RESPONSE_DONE | State::ERRORED); self.detach_resp(); let resp = self.resp.get(); resp.force_close(); - self.deliver(resp, StreamEnd::Error(err)); + self.deliver(resp, end); } self.finish(); } @@ -579,15 +623,14 @@ impl FileResponseStream { self.insert_state(State::FINISHED); if !self.state.get().contains(State::RESPONSE_DONE) { - self.insert_state(State::RESPONSE_DONE); - self.detach_resp(); - let resp = self.resp.get(); - resp.end_without_body(resp.should_close_connection()); - self.deliver(resp, StreamEnd::Complete); - // This end runs uncorked (reader callbacks), so no cork or parser - // gate will run the close check; do it here, after `on_complete` - // like `end_sendfile`, so the callbacks see a live socket. - resp.close_if_done_and_marked(); + // The reader reports an EOF with no final chunk through + // `on_reader_done`, so this is where a short file lands. The nested + // `finish()` returns at the `FINISHED` guard above. + if self.remaining.get().is_some_and(|n| n > 0) { + self.end_truncated(); + } else { + self.end_complete_without_body(); + } } // Release the owner ref from `heap::into_raw` in `start()`. Every entry @@ -597,6 +640,18 @@ impl FileResponseStream { unsafe { Self::deref(self.as_ptr()) }; } + fn end_complete_without_body(&self) { + self.insert_state(State::RESPONSE_DONE); + self.detach_resp(); + let resp = self.resp.get(); + resp.end_without_body(resp.should_close_connection()); + self.deliver(resp, StreamEnd::Complete); + // This end runs uncorked (reader callbacks), so no cork or parser + // gate will run the close check; do it here, after `on_complete` + // like `end_sendfile`, so the callbacks see a live socket. + resp.close_if_done_and_marked(); + } + fn event_loop(&self) -> EventLoopHandle { EventLoopHandle::init(self.vm.get().event_loop().cast::<()>()) } diff --git a/test/js/bun/http/bun-serve-file.test.ts b/test/js/bun/http/bun-serve-file.test.ts index cc553cc7ba59..3adc696ce441 100644 --- a/test/js/bun/http/bun-serve-file.test.ts +++ b/test/js/bun/http/bun-serve-file.test.ts @@ -1,9 +1,11 @@ import type { Server } from "bun"; import { afterAll, beforeAll, describe, expect, it, mock, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isLinux, isWindows, rmScope, rss, tempDir, tempDirWithFiles } from "harness"; +import { bunEnv, bunExe, isASAN, isLinux, isWindows, rmScope, rss, tempDir, tempDirWithFiles, tls } from "harness"; import { mkfifo } from "mkfifo"; -import { closeSync, openSync, unlinkSync, utimesSync, writeFileSync, writeSync } from "node:fs"; +import { closeSync, openSync, truncateSync, unlinkSync, utimesSync, writeFileSync, writeSync } from "node:fs"; +import { connect as netConnect } from "node:net"; import { join } from "node:path"; +import { connect as tlsConnect } from "node:tls"; const LARGE_SIZE = 1024 * 1024 * 8; const files = { @@ -1454,6 +1456,72 @@ test("file routes frame a slice that reaches or starts past EOF by the bytes the }); }); +// The server writes Content-Length from the fstat it takes when the response +// starts. When the file shrinks before the body is fully sent, the read hits +// EOF short of that length. The response used to end there as if the body +// were whole, and the connection stayed open: the client waited for the +// missing bytes until the idle timeout (forever with idleTimeout: 0). A body +// that ends short of the committed length must close the connection. Plain +// TCP on Linux takes the sendfile path; TLS (and every other platform) takes +// the buffered reader path. +describe.concurrent.each([ + ["plain", false], + ["tls", true], +])("Response(Bun.file) whose file shrinks mid-body over %s", (_label, useTls) => { + test("closes the connection instead of leaving the client waiting for the missing bytes", async () => { + using dir = tempDir("serve-file-shrinks", {}); + const filePath = join(String(dir), "big.bin"); + // Far more than the loopback socket buffers hold, so the file is still + // streaming when it shrinks. + const SIZE = 32 * 1024 * 1024; + writeFileSync(filePath, Buffer.alloc(SIZE, 67)); + + await using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + ...(useTls ? { tls } : {}), + fetch: () => new Response(Bun.file(filePath)), + }); + + const { promise: closed, resolve: onClose } = Promise.withResolvers(); + let head = ""; + let contentLength = -1; + let bodyBytes = 0; + let shrunk = false; + + const socket = useTls + ? tlsConnect({ host: "127.0.0.1", port: server.port, rejectUnauthorized: false }) + : netConnect({ host: "127.0.0.1", port: server.port }); + socket.on("error", () => {}); + socket.on("close", () => onClose()); + socket.on("data", (chunk: Buffer) => { + if (contentLength < 0) { + head += chunk.toString("latin1"); + const headEnd = head.indexOf("\r\n\r\n"); + if (headEnd < 0) return; + contentLength = Number(/^content-length:\s*(\d+)/im.exec(head.slice(0, headEnd))![1]); + chunk = Buffer.from(head.slice(headEnd + 4), "latin1"); + } + bodyBytes += chunk.length; + if (bodyBytes > 0 && !shrunk) { + // The head is on the wire with the full size and the body has + // started: shrink the file under the server. + shrunk = true; + truncateSync(filePath, 4096); + } + }); + socket.once(useTls ? "secureConnect" : "connect", () => { + socket.write("GET /big HTTP/1.1\r\nHost: x\r\n\r\n"); + }); + + // The server must hang up on its own. The client sends nothing more, so + // a server that treats the short body as complete never closes. + await closed; + expect({ contentLength, bodyShort: bodyBytes < contentLength }).toEqual({ contentLength: SIZE, bodyShort: true }); + }); +}); + // A request that declares a body arms the request-body (onData) callback on // the uWS response before the fetch handler runs. uWS keeps a single shared // userdata slot per response, so when the handler returns a file response From b0deecb5af7c5c107c338edc00f7b8f9a25a68aa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:30:24 +0000 Subject: [PATCH 2/7] ci: retrigger From f2586979879ce9892e5e17eb9d7877ee14419b81 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:44:58 +0000 Subject: [PATCH 3/7] Trim the doc comments in FileResponseStream --- src/runtime/server/FileResponseStream.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/runtime/server/FileResponseStream.rs b/src/runtime/server/FileResponseStream.rs index 6495cd79d167..fb853ab63df0 100644 --- a/src/runtime/server/FileResponseStream.rs +++ b/src/runtime/server/FileResponseStream.rs @@ -47,8 +47,7 @@ pub(crate) struct FileResponseStream { mode: Cell, reader: JsCell, sendfile: JsCell, - /// Bytes still owed on a fixed-length body (the caller wrote this many - /// into Content-Length). `None` streams to EOF. + /// Bytes still owed against the Content-Length the caller wrote; `None` streams to EOF. remaining: Cell>, state: Cell, @@ -134,9 +133,7 @@ pub(crate) enum StreamOwner { enum StreamEnd { Complete, Abort, - /// The file ended before the length the caller committed to (it shrank - /// after the `fstat`). The socket is force-closed first; the owner only - /// has to release the request. + /// EOF before the committed length (the file shrank after the `fstat`). Truncated, Error(sys::Error), } @@ -562,8 +559,6 @@ impl FileResponseStream { self.finish(); } - /// Subtracts `sent` from the bytes still owed; returns the new balance - /// (`None` when the body has no fixed length). fn charge_remaining(&self, sent: u64) -> Option { let remaining = self.remaining.get().map(|n| n.saturating_sub(sent)); self.remaining.set(remaining); @@ -574,11 +569,8 @@ impl FileResponseStream { self.close_with(StreamEnd::Error(err)); } - /// The headers already promised a Content-Length that the file no longer - /// backs. A clean end would retire the request as a complete keep-alive - /// response, and the client would wait for the missing bytes until the - /// idle timeout. Closing the connection is the one signal left that the - /// body is short. + /// The committed Content-Length can no longer be honoured, so a close is + /// the only signal left that tells the client the body is short. fn end_truncated(&self) { bun_output::scoped_log!( FileResponseStream, @@ -623,9 +615,8 @@ impl FileResponseStream { self.insert_state(State::FINISHED); if !self.state.get().contains(State::RESPONSE_DONE) { - // The reader reports an EOF with no final chunk through - // `on_reader_done`, so this is where a short file lands. The nested - // `finish()` returns at the `FINISHED` guard above. + // An EOF with no final chunk arrives through `on_reader_done` and + // lands here; the nested `finish()` returns at the guard above. if self.remaining.get().is_some_and(|n| n > 0) { self.end_truncated(); } else { From ca1d2594cce305d4933e61b08e2ff190d3aa9644 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:25:33 +0000 Subject: [PATCH 4/7] FileResponseStream: decide complete or short in one place Every end of a file body that is not an abort or an error now goes through complete(). It ends the response when the body reached its committed length, and closes the connection when bytes are still owed. The sendfile end, the reader's end with a final chunk and the reader's end without one no longer each carry their own check. --- src/runtime/server/FileResponseStream.rs | 69 +++++++++++------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/runtime/server/FileResponseStream.rs b/src/runtime/server/FileResponseStream.rs index fb853ab63df0..7f20b865cf4a 100644 --- a/src/runtime/server/FileResponseStream.rs +++ b/src/runtime/server/FileResponseStream.rs @@ -133,7 +133,7 @@ pub(crate) enum StreamOwner { enum StreamEnd { Complete, Abort, - /// EOF before the committed length (the file shrank after the `fstat`). + /// The file ended before the committed length. Truncated, Error(sys::Error), } @@ -316,17 +316,10 @@ impl FileResponseStream { let resp = self.resp.get(); resp.timeout(self.idle_timeout.get()); - let remaining = self.charge_remaining(chunk.len() as u64); + self.charge_remaining(chunk.len() as u64); if state == ReadState::Eof { - if remaining.is_some_and(|n| n > 0) { - self.end_truncated(); - return false; - } - self.insert_state(State::RESPONSE_DONE); - self.detach_resp(); - resp.end(chunk, resp.should_close_connection()); - self.deliver(resp, StreamEnd::Complete); + self.complete(|resp, close| resp.end(chunk, close)); return false; } @@ -467,15 +460,10 @@ impl FileResponseStream { match errno { sys::E::SUCCESS => { - if remain == 0 { + if remain == 0 || sent == 0 { self.end_sendfile(); return false; } - if sent == 0 { - // EOF before the committed length: the file shrank. - self.end_truncated(); - return false; - } return self.arm_sendfile_writable(); } sys::E::EINTR => continue, @@ -521,11 +509,12 @@ impl FileResponseStream { if self.state.get().contains(State::RESPONSE_DONE) { return; } - self.insert_state(State::RESPONSE_DONE); - self.detach_resp(); + let ended = + self.complete(|resp, close| resp.end_send_file(self.sendfile.get().offset, close)); + if !ended { + return; + } let resp = self.resp.get(); - resp.end_send_file(self.sendfile.get().offset, resp.should_close_connection()); - self.deliver(resp, StreamEnd::Complete); // `end_send_file` bypasses every shouldCloseConnection() gate: it does // not go through internalEnd, and the onWritable gate is skipped // because this frame returns `false` to it. Run the gate here — after @@ -565,6 +554,21 @@ impl FileResponseStream { remaining } + /// The one way a body ends as complete; `end` hands it to uWS. A body that + /// still owes bytes closes the connection instead, and this returns false. + fn complete(&self, end: impl FnOnce(AnyResponse, bool)) -> bool { + if self.remaining.get().is_some_and(|n| n > 0) { + self.end_truncated(); + return false; + } + self.insert_state(State::RESPONSE_DONE); + self.detach_resp(); + let resp = self.resp.get(); + end(resp, resp.should_close_connection()); + self.deliver(resp, StreamEnd::Complete); + true + } + fn fail_with(&self, err: sys::Error) { self.close_with(StreamEnd::Error(err)); } @@ -615,12 +619,13 @@ impl FileResponseStream { self.insert_state(State::FINISHED); if !self.state.get().contains(State::RESPONSE_DONE) { - // An EOF with no final chunk arrives through `on_reader_done` and - // lands here; the nested `finish()` returns at the guard above. - if self.remaining.get().is_some_and(|n| n > 0) { - self.end_truncated(); - } else { - self.end_complete_without_body(); + // A close from `complete` re-enters `finish()`, which returns at the guard above. + let ended = self.complete(|resp, close| resp.end_without_body(close)); + if ended { + // This end runs uncorked (reader callbacks), so no cork or parser + // gate will run the close check; do it here, after `on_complete` + // like `end_sendfile`, so the callbacks see a live socket. + self.resp.get().close_if_done_and_marked(); } } @@ -631,18 +636,6 @@ impl FileResponseStream { unsafe { Self::deref(self.as_ptr()) }; } - fn end_complete_without_body(&self) { - self.insert_state(State::RESPONSE_DONE); - self.detach_resp(); - let resp = self.resp.get(); - resp.end_without_body(resp.should_close_connection()); - self.deliver(resp, StreamEnd::Complete); - // This end runs uncorked (reader callbacks), so no cork or parser - // gate will run the close check; do it here, after `on_complete` - // like `end_sendfile`, so the callbacks see a live socket. - resp.close_if_done_and_marked(); - } - fn event_loop(&self) -> EventLoopHandle { EventLoopHandle::init(self.vm.get().event_loop().cast::<()>()) } From 4e996be56deb754e1c17dec2004ff21362e8eff7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:25:34 +0000 Subject: [PATCH 5/7] Test every way a Bun.file body ends short of its Content-Length The file in the shrink tests is now a 256 MiB length with no bytes written. The server sends until the socket stops taking bytes, so a file that the loopback buffers can hold is sent whole before the test can shrink it, and the wait for a close never ends. Each case also checks that the response was still in flight when the file shrank. New cases: a file route, a directory route, and a sysfs attribute, whose stat size is a whole page while a read returns a few bytes. The last one ends the body with a final short chunk, which no other case reaches. The servers, sockets and files are released from a hook, because a hang is the failure mode and a hung test body releases nothing. --- test/js/bun/http/bun-serve-file.test.ts | 196 ++++++++++++++++++------ 1 file changed, 148 insertions(+), 48 deletions(-) diff --git a/test/js/bun/http/bun-serve-file.test.ts b/test/js/bun/http/bun-serve-file.test.ts index 3adc696ce441..252cc798662a 100644 --- a/test/js/bun/http/bun-serve-file.test.ts +++ b/test/js/bun/http/bun-serve-file.test.ts @@ -2,7 +2,17 @@ import type { Server } from "bun"; import { afterAll, beforeAll, describe, expect, it, mock, test } from "bun:test"; import { bunEnv, bunExe, isASAN, isLinux, isWindows, rmScope, rss, tempDir, tempDirWithFiles, tls } from "harness"; import { mkfifo } from "mkfifo"; -import { closeSync, openSync, truncateSync, unlinkSync, utimesSync, writeFileSync, writeSync } from "node:fs"; +import { + closeSync, + openSync, + readFileSync, + statSync, + truncateSync, + unlinkSync, + utimesSync, + writeFileSync, + writeSync, +} from "node:fs"; import { connect as netConnect } from "node:net"; import { join } from "node:path"; import { connect as tlsConnect } from "node:tls"; @@ -1456,69 +1466,159 @@ test("file routes frame a slice that reaches or starts past EOF by the bytes the }); }); -// The server writes Content-Length from the fstat it takes when the response -// starts. When the file shrinks before the body is fully sent, the read hits -// EOF short of that length. The response used to end there as if the body -// were whole, and the connection stayed open: the client waited for the -// missing bytes until the idle timeout (forever with idleTimeout: 0). A body -// that ends short of the committed length must close the connection. Plain -// TCP on Linux takes the sendfile path; TLS (and every other platform) takes -// the buffered reader path. -describe.concurrent.each([ - ["plain", false], - ["tls", true], -])("Response(Bun.file) whose file shrinks mid-body over %s", (_label, useTls) => { - test("closes the connection instead of leaving the client waiting for the missing bytes", async () => { - using dir = tempDir("serve-file-shrinks", {}); - const filePath = join(String(dir), "big.bin"); - // Far more than the loopback socket buffers hold, so the file is still - // streaming when it shrinks. - const SIZE = 32 * 1024 * 1024; - writeFileSync(filePath, Buffer.alloc(SIZE, 67)); - - await using server = Bun.serve({ - port: 0, - hostname: "127.0.0.1", - idleTimeout: 0, - ...(useTls ? { tls } : {}), - fetch: () => new Response(Bun.file(filePath)), - }); - - const { promise: closed, resolve: onClose } = Promise.withResolvers(); +// Bun.serve writes Content-Length for a Bun.file body from the fstat it takes +// when the response starts. When the file then yields fewer bytes, the +// response used to end as if the body were whole and the connection stayed +// open: the client waited for the missing bytes until the idle timeout +// (forever with idleTimeout: 0). A body that ends short of the committed +// length must close the connection. +// +// The end of the file reaches the server in three shapes, each handled in its +// own place: sendfile(2) returns 0 (plain TCP on Linux, bodies of 1 MiB and +// up), the reader reports the end with no data (the file now ends before the +// reader's offset), or the reader delivers a last short chunk with the end. +describe("Bun.file body that ends short of its Content-Length", () => { + // A wait that never ends is how these tests fail, and then nothing scoped + // to a test body is released. This hook still runs. + const resources = new DisposableStack(); + afterAll(() => resources.dispose()); + + // One raw GET. `closed` settles when the server hangs up, and `seen` says + // how far the response got. `onBodyStarted` runs once, with the first body + // bytes; its `settle` ends the wait when the test cannot go on. + function getUntilClosed( + server: ReturnType, + useTls: boolean, + pathname: string, + onBodyStarted: (settle: () => void) => void = () => {}, + ) { + const seen = { contentLength: -1, bodyBytes: 0 }; + const { promise: closed, resolve, reject } = Promise.withResolvers(); + let waiting = true; + const settle = () => { + waiting = false; + resolve(); + }; let head = ""; - let contentLength = -1; - let bodyBytes = 0; - let shrunk = false; + let bodyStarted = false; + const port = server.port!; const socket = useTls - ? tlsConnect({ host: "127.0.0.1", port: server.port, rejectUnauthorized: false }) - : netConnect({ host: "127.0.0.1", port: server.port }); + ? tlsConnect({ host: "127.0.0.1", port, rejectUnauthorized: false }) + : netConnect({ host: "127.0.0.1", port }); + resources.defer(() => socket.destroy()); + // Runs before the server stops: say where a wait that never ended stood. + resources.defer(() => { + if (waiting) { + console.error("the server did not close the connection:", { + pathname, + useTls, + ...seen, + pendingRequests: server.pendingRequests, + }); + } + }); + + // The close under test is a reset, which the socket reports as an error first. socket.on("error", () => {}); - socket.on("close", () => onClose()); + socket.on("close", settle); socket.on("data", (chunk: Buffer) => { - if (contentLength < 0) { + if (seen.contentLength < 0) { head += chunk.toString("latin1"); const headEnd = head.indexOf("\r\n\r\n"); if (headEnd < 0) return; - contentLength = Number(/^content-length:\s*(\d+)/im.exec(head.slice(0, headEnd))![1]); + seen.contentLength = Number(/^content-length:\s*(\d+)/im.exec(head.slice(0, headEnd))?.[1]); chunk = Buffer.from(head.slice(headEnd + 4), "latin1"); } - bodyBytes += chunk.length; - if (bodyBytes > 0 && !shrunk) { - // The head is on the wire with the full size and the body has - // started: shrink the file under the server. - shrunk = true; - truncateSync(filePath, 4096); + seen.bodyBytes += chunk.length; + if (seen.bodyBytes > 0 && !bodyStarted) { + bodyStarted = true; + try { + onBodyStarted(settle); + } catch (error) { + waiting = false; + reject(error); + } } }); socket.once(useTls ? "secureConnect" : "connect", () => { - socket.write("GET /big HTTP/1.1\r\nHost: x\r\n\r\n"); + socket.write(`GET ${pathname} HTTP/1.1\r\nHost: x\r\n\r\n`); }); + return { closed, seen }; + } + + // The server sends until the socket stops taking bytes, so a file that the + // loopback buffers can hold is sent whole before anything shrinks. This + // length is far beyond them. Only the length is set: no bytes are written. + const SHRINKING_FILE_SIZE = 256 * 1024 * 1024; + + describe.concurrent.each([ + { via: "fetch handler", pathname: "/big.bin", transport: "plain" }, + { via: "fetch handler", pathname: "/big.bin", transport: "tls" }, + { via: "file route", pathname: "/route", transport: "plain" }, + { via: "directory route", pathname: "/dir/big.bin", transport: "tls" }, + ] as const)("from a $via over $transport, when the file shrinks mid-body", ({ pathname, transport }) => { + test("closes the connection instead of leaving the client waiting for the missing bytes", async () => { + const dir = resources.use(tempDir("serve-file-shrinks", {})); + const filePath = join(String(dir), "big.bin"); + writeFileSync(filePath, ""); + truncateSync(filePath, SHRINKING_FILE_SIZE); + + const server = resources.use( + Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + ...(transport === "tls" ? { tls } : {}), + routes: { + "/route": new Response(Bun.file(filePath)), + "/dir/*": { dir: String(dir) }, + }, + fetch: () => new Response(Bun.file(filePath)), + }), + ); + + // The client sends nothing after its request, so a server that treats + // the short body as complete never closes. + let inFlightWhenShrunk = false; + const { closed, seen } = getUntilClosed(server, transport === "tls", pathname, settle => { + // The head is on the wire with the full size and the body has started. + inFlightWhenShrunk = server.pendingRequests === 1; + if (!inFlightWhenShrunk) return settle(); + truncateSync(filePath, 4096); + }); + await closed; + + expect({ + inFlightWhenShrunk, + contentLength: seen.contentLength, + bodyShort: seen.bodyBytes < seen.contentLength, + }).toEqual({ inFlightWhenShrunk: true, contentLength: SHRINKING_FILE_SIZE, bodyShort: true }); + }); + }); + + // A sysfs attribute is a regular file whose stat size is a whole page while + // a read returns only its few bytes. The committed length is wrong before + // anything shrinks, and the end of the file arrives with the only chunk. + // Other platforms have no regular file like it. + test.concurrent.skipIf(!isLinux)("closes the connection when the stat size overstates the content", async () => { + const attribute = "/sys/devices/system/cpu/online"; + const statSize = statSync(attribute).size; + expect(statSize).toBeGreaterThan(readFileSync(attribute).length); + + const server = resources.use( + Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch: () => new Response(Bun.file(attribute)), + }), + ); - // The server must hang up on its own. The client sends nothing more, so - // a server that treats the short body as complete never closes. + const { closed, seen } = getUntilClosed(server, false, "/online"); await closed; - expect({ contentLength, bodyShort: bodyBytes < contentLength }).toEqual({ contentLength: SIZE, bodyShort: true }); + // The reset can reach the client before the head, so there may be no Content-Length to compare with. + expect(seen.bodyBytes).toBeLessThan(statSize); }); }); From 2fb5bbdfaf3f8b0da9af40d62ce60f8a632b1646 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:43:00 +0000 Subject: [PATCH 6/7] End a short file body in order and take the connection down after it A body that still owes bytes is now ended with the connection marked to close, the way a Connection: close response ends: uWS sends the bytes that exist, then shuts the socket down once they have drained. The earlier revision reset the connection, which dropped bytes still in the socket buffer and the head of a response that had not left yet. uws_res_end_sendfile took close_connection and ignored it. It now sets the close mark, so the sendfile end can ask for the same teardown. --- src/runtime/server/FileResponseStream.rs | 56 ++++++------------------ src/uws_sys/libuwsockets.cpp | 10 +++++ 2 files changed, 24 insertions(+), 42 deletions(-) diff --git a/src/runtime/server/FileResponseStream.rs b/src/runtime/server/FileResponseStream.rs index 7f20b865cf4a..d1834a982ae0 100644 --- a/src/runtime/server/FileResponseStream.rs +++ b/src/runtime/server/FileResponseStream.rs @@ -47,7 +47,7 @@ pub(crate) struct FileResponseStream { mode: Cell, reader: JsCell, sendfile: JsCell, - /// Bytes still owed against the Content-Length the caller wrote; `None` streams to EOF. + /// Bytes of `StartOptions::length` not sent yet; `None` streams to EOF. remaining: Cell>, state: Cell, @@ -133,8 +133,6 @@ pub(crate) enum StreamOwner { enum StreamEnd { Complete, Abort, - /// The file ended before the committed length. - Truncated, Error(sys::Error), } @@ -149,7 +147,7 @@ impl StreamOwner { on_abort, on_error, } => match end { - StreamEnd::Complete | StreamEnd::Truncated => on_complete(ctx, resp), + StreamEnd::Complete => on_complete(ctx, resp), StreamEnd::Abort => on_abort.unwrap_or(on_complete)(ctx, resp), StreamEnd::Error(err) => on_error(ctx, resp, err), }, @@ -509,11 +507,7 @@ impl FileResponseStream { if self.state.get().contains(State::RESPONSE_DONE) { return; } - let ended = - self.complete(|resp, close| resp.end_send_file(self.sendfile.get().offset, close)); - if !ended { - return; - } + self.complete(|resp, close| resp.end_send_file(self.sendfile.get().offset, close)); let resp = self.resp.get(); // `end_send_file` bypasses every shouldCloseConnection() gate: it does // not go through internalEnd, and the onWritable gate is skipped @@ -554,43 +548,24 @@ impl FileResponseStream { remaining } - /// The one way a body ends as complete; `end` hands it to uWS. A body that - /// still owes bytes closes the connection instead, and this returns false. - fn complete(&self, end: impl FnOnce(AnyResponse, bool)) -> bool { - if self.remaining.get().is_some_and(|n| n > 0) { - self.end_truncated(); - return false; - } + /// The one way a body ends without an abort or an error; `end` hands it to uWS. + /// A body that still owes bytes is not one the connection can be reused after. + fn complete(&self, end: impl FnOnce(AnyResponse, bool)) { self.insert_state(State::RESPONSE_DONE); self.detach_resp(); let resp = self.resp.get(); - end(resp, resp.should_close_connection()); + let short = self.remaining.get().is_some_and(|n| n > 0); + end(resp, short || resp.should_close_connection()); self.deliver(resp, StreamEnd::Complete); - true } fn fail_with(&self, err: sys::Error) { - self.close_with(StreamEnd::Error(err)); - } - - /// The committed Content-Length can no longer be honoured, so a close is - /// the only signal left that tells the client the body is short. - fn end_truncated(&self) { - bun_output::scoped_log!( - FileResponseStream, - "endTruncated remaining={:?}", - self.remaining.get() - ); - self.close_with(StreamEnd::Truncated); - } - - fn close_with(&self, end: StreamEnd) { if !self.state.get().contains(State::RESPONSE_DONE) { self.insert_state(State::RESPONSE_DONE | State::ERRORED); self.detach_resp(); let resp = self.resp.get(); resp.force_close(); - self.deliver(resp, end); + self.deliver(resp, StreamEnd::Error(err)); } self.finish(); } @@ -619,14 +594,11 @@ impl FileResponseStream { self.insert_state(State::FINISHED); if !self.state.get().contains(State::RESPONSE_DONE) { - // A close from `complete` re-enters `finish()`, which returns at the guard above. - let ended = self.complete(|resp, close| resp.end_without_body(close)); - if ended { - // This end runs uncorked (reader callbacks), so no cork or parser - // gate will run the close check; do it here, after `on_complete` - // like `end_sendfile`, so the callbacks see a live socket. - self.resp.get().close_if_done_and_marked(); - } + self.complete(|resp, close| resp.end_without_body(close)); + // This end runs uncorked (reader callbacks), so no cork or parser + // gate will run the close check; do it here, after `on_complete` + // like `end_sendfile`, so the callbacks see a live socket. + self.resp.get().close_if_done_and_marked(); } // Release the owner ref from `heap::into_raw` in `start()`. Every entry diff --git a/src/uws_sys/libuwsockets.cpp b/src/uws_sys/libuwsockets.cpp index d9dc129a943d..ac3cdeeb6fd2 100644 --- a/src/uws_sys/libuwsockets.cpp +++ b/src/uws_sys/libuwsockets.cpp @@ -1083,6 +1083,11 @@ extern "C" uWS::HttpResponse *uwsRes = (uWS::HttpResponse *)res; auto *data = uwsRes->getHttpResponseData(); data->offset = offset; + /* The head is on the wire, so only the state the close gates read. */ + if (close_connection) + { + data->state |= uWS::HttpResponseData::HTTP_CONNECTION_CLOSE; + } data->state |= uWS::HttpResponseData::HTTP_END_CALLED; data->markDone(uwsRes); uwsRes->resetTimeout(); @@ -1092,6 +1097,11 @@ extern "C" uWS::HttpResponse *uwsRes = (uWS::HttpResponse *)res; auto *data = uwsRes->getHttpResponseData(); data->offset = offset; + /* See the SSL arm above. */ + if (close_connection) + { + data->state |= uWS::HttpResponseData::HTTP_CONNECTION_CLOSE; + } data->state |= uWS::HttpResponseData::HTTP_END_CALLED; data->markDone(uwsRes); uwsRes->resetTimeout(); From 5bd19c1bc5f7870f75ffd6c9dd9ed922d79e2328 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 3 Oct 2026 03:27:50 +0000 Subject: [PATCH 7/7] Test that a short file body arrives, then the connection ends The cases now expect an in-order end: no reset on the client, and for the sysfs attribute the head with Connection: close and the bytes the file has. --- src/runtime/server/FileResponseStream.rs | 2 +- test/js/bun/http/bun-serve-file.test.ts | 100 +++++++++++++---------- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/runtime/server/FileResponseStream.rs b/src/runtime/server/FileResponseStream.rs index d1834a982ae0..bbb62361f122 100644 --- a/src/runtime/server/FileResponseStream.rs +++ b/src/runtime/server/FileResponseStream.rs @@ -549,7 +549,7 @@ impl FileResponseStream { } /// The one way a body ends without an abort or an error; `end` hands it to uWS. - /// A body that still owes bytes is not one the connection can be reused after. + /// A body that still owes bytes takes the connection down once it has drained. fn complete(&self, end: impl FnOnce(AnyResponse, bool)) { self.insert_state(State::RESPONSE_DONE); self.detach_resp(); diff --git a/test/js/bun/http/bun-serve-file.test.ts b/test/js/bun/http/bun-serve-file.test.ts index 252cc798662a..9022594feb1a 100644 --- a/test/js/bun/http/bun-serve-file.test.ts +++ b/test/js/bun/http/bun-serve-file.test.ts @@ -1468,23 +1468,24 @@ test("file routes frame a slice that reaches or starts past EOF by the bytes the // Bun.serve writes Content-Length for a Bun.file body from the fstat it takes // when the response starts. When the file then yields fewer bytes, the -// response used to end as if the body were whole and the connection stayed -// open: the client waited for the missing bytes until the idle timeout -// (forever with idleTimeout: 0). A body that ends short of the committed -// length must close the connection. +// response used to end like a complete one and the connection stayed open: +// the client waited for the missing bytes until the idle timeout (forever +// with idleTimeout: 0). A body that ends short of the committed length must +// take the connection down with it: the bytes that exist, then the end. // -// The end of the file reaches the server in three shapes, each handled in its -// own place: sendfile(2) returns 0 (plain TCP on Linux, bodies of 1 MiB and -// up), the reader reports the end with no data (the file now ends before the -// reader's offset), or the reader delivers a last short chunk with the end. +// The end of the file reaches the server in three shapes: sendfile(2) returns +// 0 (plain TCP on Linux, bodies of 1 MiB and up), the reader reports the end +// with no data (the file now ends before the reader's offset), or the reader +// delivers a last short chunk with the end. describe("Bun.file body that ends short of its Content-Length", () => { // A wait that never ends is how these tests fail, and then nothing scoped // to a test body is released. This hook still runs. const resources = new DisposableStack(); afterAll(() => resources.dispose()); - // One raw GET. `closed` settles when the server hangs up, and `seen` says - // how far the response got. `onBodyStarted` runs once, with the first body + // One raw GET on its own connection. `closed` settles when the server ends + // the connection, and `seen` is what arrived until then (`bodyStart` keeps + // the first 4096 body bytes). `onBodyStarted` runs once, with the first body // bytes; its `settle` ends the wait when the test cannot go on. function getUntilClosed( server: ReturnType, @@ -1492,15 +1493,14 @@ describe("Bun.file body that ends short of its Content-Length", () => { pathname: string, onBodyStarted: (settle: () => void) => void = () => {}, ) { - const seen = { contentLength: -1, bodyBytes: 0 }; + const seen = { head: "", contentLength: -1, bodyBytes: 0, bodyStart: "", error: undefined as string | undefined }; const { promise: closed, resolve, reject } = Promise.withResolvers(); let waiting = true; const settle = () => { waiting = false; resolve(); }; - let head = ""; - let bodyStarted = false; + let received = ""; const port = server.port!; const socket = useTls @@ -1510,29 +1510,36 @@ describe("Bun.file body that ends short of its Content-Length", () => { // Runs before the server stops: say where a wait that never ended stood. resources.defer(() => { if (waiting) { - console.error("the server did not close the connection:", { + console.error("the server did not end the connection:", { pathname, useTls, - ...seen, + contentLength: seen.contentLength, + bodyBytes: seen.bodyBytes, pendingRequests: server.pendingRequests, }); } }); - // The close under test is a reset, which the socket reports as an error first. - socket.on("error", () => {}); + // The server ends the connection in order, so a reset is part of the result. + socket.on("error", (error: NodeJS.ErrnoException) => { + seen.error = error.code ?? error.message; + }); socket.on("close", settle); socket.on("data", (chunk: Buffer) => { if (seen.contentLength < 0) { - head += chunk.toString("latin1"); - const headEnd = head.indexOf("\r\n\r\n"); + received += chunk.toString("latin1"); + const headEnd = received.indexOf("\r\n\r\n"); if (headEnd < 0) return; - seen.contentLength = Number(/^content-length:\s*(\d+)/im.exec(head.slice(0, headEnd))?.[1]); - chunk = Buffer.from(head.slice(headEnd + 4), "latin1"); + seen.head = received.slice(0, headEnd); + seen.contentLength = Number(/^content-length:\s*(\d+)/im.exec(seen.head)?.[1]); + chunk = Buffer.from(received.slice(headEnd + 4), "latin1"); } + const bodyStarted = seen.bodyBytes > 0; seen.bodyBytes += chunk.length; + if (seen.bodyStart.length < 4096) { + seen.bodyStart += chunk.toString("latin1", 0, 4096 - seen.bodyStart.length); + } if (seen.bodyBytes > 0 && !bodyStarted) { - bodyStarted = true; try { onBodyStarted(settle); } catch (error) { @@ -1558,7 +1565,7 @@ describe("Bun.file body that ends short of its Content-Length", () => { { via: "file route", pathname: "/route", transport: "plain" }, { via: "directory route", pathname: "/dir/big.bin", transport: "tls" }, ] as const)("from a $via over $transport, when the file shrinks mid-body", ({ pathname, transport }) => { - test("closes the connection instead of leaving the client waiting for the missing bytes", async () => { + test("ends the connection instead of leaving the client waiting for the missing bytes", async () => { const dir = resources.use(tempDir("serve-file-shrinks", {})); const filePath = join(String(dir), "big.bin"); writeFileSync(filePath, ""); @@ -1579,7 +1586,7 @@ describe("Bun.file body that ends short of its Content-Length", () => { ); // The client sends nothing after its request, so a server that treats - // the short body as complete never closes. + // the short body as complete never ends the connection. let inFlightWhenShrunk = false; const { closed, seen } = getUntilClosed(server, transport === "tls", pathname, settle => { // The head is on the wire with the full size and the body has started. @@ -1593,7 +1600,8 @@ describe("Bun.file body that ends short of its Content-Length", () => { inFlightWhenShrunk, contentLength: seen.contentLength, bodyShort: seen.bodyBytes < seen.contentLength, - }).toEqual({ inFlightWhenShrunk: true, contentLength: SHRINKING_FILE_SIZE, bodyShort: true }); + error: seen.error, + }).toEqual({ inFlightWhenShrunk: true, contentLength: SHRINKING_FILE_SIZE, bodyShort: true, error: undefined }); }); }); @@ -1601,25 +1609,33 @@ describe("Bun.file body that ends short of its Content-Length", () => { // a read returns only its few bytes. The committed length is wrong before // anything shrinks, and the end of the file arrives with the only chunk. // Other platforms have no regular file like it. - test.concurrent.skipIf(!isLinux)("closes the connection when the stat size overstates the content", async () => { - const attribute = "/sys/devices/system/cpu/online"; - const statSize = statSync(attribute).size; - expect(statSize).toBeGreaterThan(readFileSync(attribute).length); + test.concurrent.skipIf(!isLinux)( + "sends what a file has when its stat size overstates it, then ends the connection", + async () => { + const attribute = "/sys/devices/system/cpu/online"; + const content = readFileSync(attribute).toString("latin1"); + const statSize = statSync(attribute).size; + expect(statSize).toBeGreaterThan(content.length); - const server = resources.use( - Bun.serve({ - port: 0, - hostname: "127.0.0.1", - idleTimeout: 0, - fetch: () => new Response(Bun.file(attribute)), - }), - ); + const server = resources.use( + Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch: () => new Response(Bun.file(attribute)), + }), + ); - const { closed, seen } = getUntilClosed(server, false, "/online"); - await closed; - // The reset can reach the client before the head, so there may be no Content-Length to compare with. - expect(seen.bodyBytes).toBeLessThan(statSize); - }); + const { closed, seen } = getUntilClosed(server, false, "/online"); + await closed; + expect({ + contentLength: seen.contentLength, + connection: /^connection:\s*(.+)$/im.exec(seen.head)?.[1], + body: seen.bodyStart, + error: seen.error, + }).toEqual({ contentLength: statSize, connection: "close", body: content, error: undefined }); + }, + ); }); // A request that declares a body arms the request-body (onData) callback on