Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/runtime/server/FileRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl FileRoute {

// `fd_owned` tracks whether this function is still responsible for
// closing the file descriptor and releasing the route ref. Every
// non-streaming return — bodiless status codes (304/204/205/307/308),
// non-streaming return — null-body status codes (1xx/204/205/304),
// HEAD, non-streamable files, and the two JS-exception early-return
// paths below — hits this defer, so neither the fd nor the route ref
// (or the server's pending_requests counter) can leak regardless of
Expand Down Expand Up @@ -504,10 +504,17 @@ impl FileRoute {
resp.write_mark();
this.write_headers(resp);

// Bodiless statuses end before the range switch so a 304 emits no
// Null-body statuses end before the range switch so a 304 emits no
// Content-Range. FileResponseStream ships via sendfile/write(), so a
// null-body status must never start it; 307/308 routes skip it too.
if HTTPStatusText::is_null_body(status_code) || matches!(status_code, 307 | 308) {
// null-body status must never start it. 307/308 are ordinary
// body-bearing statuses (RFC 9110 §15.4) and fall through to stream
// the file, same as StaticRoute and the fetch-handler path.
if HTTPStatusText::is_null_body(status_code) {
// 205 is the one null-body status RFC 9112 §6.3 does NOT
// self-terminate, so a keep-alive client needs Content-Length.
if status_code == 205 && !resp.state().has_written_content_length_header() {
resp.write_header_int(b"content-length", 0);
}
resp.end_without_body(resp.should_close_connection());
return;
}
Comment thread
robobun marked this conversation as resolved.
Expand Down
105 changes: 105 additions & 0 deletions test/js/bun/http/bun-serve-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { afterAll, beforeAll, describe, expect, it, mock, test } from "bun:test"
import { bunEnv, bunExe, isASAN, isWindows, rmScope, tempDir, tempDirWithFiles } from "harness";
import { mkfifo } from "mkfifo";
import { unlinkSync } from "node:fs";
import { connect } from "node:net";
import { join } from "node:path";

const LARGE_SIZE = 1024 * 1024 * 8;
Expand Down Expand Up @@ -1187,3 +1188,107 @@ test("file route serves a burst of concurrent requests after reloads", async ()
const a = await fetch(`${server.url}a`).then(r => r.text());
expect(a).toBe("a-new");
});

// FileRoute used to end 205/307/308 via end_without_body, which writes no
// Content-Length; RFC 9112 §6.3 does not self-terminate those, so HTTP/1.1
// keep-alive clients blocked waiting for body framing. 307/308 now stream the
// file body like StaticRoute and the fetch-handler path do (RFC 9110 §15.4
// permits a redirect body); 205 stays bodiless per RFC 9110 §15.3.6 and
// writes Content-Length: 0.
test("file route 205/307/308 responses are framed on HTTP/1.1 keep-alive", async () => {
using dir = tempDir("serve-file-bodiless-status", {
"f.txt": "hello",
});
const file = () => Bun.file(join(String(dir), "f.txt"));
await using server = Bun.serve({
port: 0,
routes: {
"/204": new Response(file(), { status: 204 }),
"/205": new Response(file(), { status: 205 }),
"/304": new Response(file(), { status: 304 }),
"/307": new Response(file(), { status: 307, headers: { Location: "/204" } }),
"/308": new Response(file(), { status: 308, headers: { Location: "/204" } }),
},
fetch: req =>
new URL(req.url).pathname === "/handler-307"
? new Response(file(), { status: 307, headers: { Location: "/204" } })
: new Response("fallback"),
});

async function rawGet(path: string) {
const { promise, resolve, reject } = Promise.withResolvers<string>();
const sock = connect(server.port, "127.0.0.1", () => {
sock.write(`GET ${path} HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n`);
});
let buf = "";
sock.on("data", d => {
buf += d.toString("latin1");
if (buf.includes("\r\n\r\n")) {
sock.end();
resolve(buf);
}
});
sock.on("close", () => resolve(buf));
sock.on("error", reject);
return promise;
}

const framing = async (path: string) => {
const raw = await rawGet(path);
const head = raw.split("\r\n\r\n")[0];
return {
contentLength: /^content-length:\s*(\S+)/im.exec(head)?.[1] ?? null,
connectionClose: /^connection:\s*close/im.test(head),
};
};

// 307/308 ship the file body (same Content-Length as the fetch-handler path).
for (const path of ["/307", "/308", "/handler-307"]) {
expect({ path, ...(await framing(path)) }).toEqual({
path,
contentLength: "5",
connectionClose: false,
});
}
// 205 is a null-body status but not self-terminating: Content-Length: 0.
expect(await framing("/205")).toEqual({ contentLength: "0", connectionClose: false });
// 204/304 are self-terminating and stay header-only (RFC 9110 §8.6 forbids
// Content-Length on 204).
for (const path of ["/204", "/304"]) {
expect({ path, ...(await framing(path)) }).toEqual({
path,
contentLength: null,
connectionClose: false,
});
}

// Two back-to-back 307s on a keep-alive connection: the second resolving
// proves the first's framing was complete. Skipped on Windows: pipelining
// a Bun.file() route there hits a pre-existing bug (connection closes with
// zero bytes, independent of status; reproduces on main with status 200).
if (!isWindows) {
const { promise, resolve, reject } = Promise.withResolvers<string>();
const sock = connect(server.port, "127.0.0.1", () => {
sock.write("GET /307 HTTP/1.1\r\nHost: x\r\n\r\nGET /307 HTTP/1.1\r\nHost: x\r\n\r\n");
});
let buf = "";
sock.on("data", d => {
buf += d.toString("latin1");
if ((buf.match(/HTTP\/1\.1 307/g) || []).length === 2) {
sock.end();
resolve(buf);
}
});
sock.on("close", () => resolve(buf));
sock.on("error", reject);
expect((await promise).match(/HTTP\/1\.1 307/g)?.length).toBe(2);
Comment thread
claude[bot] marked this conversation as resolved.
}

// And fetch (redirect:"manual") must complete rather than time out.
const res = await fetch(`${server.url}307`, { redirect: "manual" });
expect({
status: res.status,
contentLength: res.headers.get("content-length"),
body: await res.text(),
}).toEqual({ status: 307, contentLength: "5", body: "hello" });
});
Loading