Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions src/runtime/server/FileRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,14 @@ impl FileRoute {
// 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) {
// 205/307/308 are not self-terminating under RFC 9112 §6.3, so a
// keep-alive client needs Content-Length. 1xx/204/304 are, and
// stay header-only.
if matches!(status_code, 205 | 307 | 308)
&& !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
95 changes: 95 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 { 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,97 @@
const a = await fetch(`${server.url}a`).then(r => r.text());
expect(a).toBe("a-new");
});

// FileRoute ends a bodiless status via end_without_body, which writes no
// Content-Length. For statuses that RFC 9112 §6.3 does NOT self-terminate
// (205/307/308), an HTTP/1.1 keep-alive client would then block waiting for
// the body. Assert each status is framed, and that 307 actually completes
// over keep-alive (the former hang).
test("file route bodiless statuses 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: () => new Response("fallback"),
});

async function rawGet(path: string) {
const { promise, resolve } = 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("error", () => resolve(buf));
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),
};
};

// 205/307/308 must carry Content-Length (they are not self-terminating).
for (const path of ["/205", "/307", "/308"]) {
expect({ path, ...(await framing(path)) }).toEqual({
path,
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.
{
const { promise, resolve } = 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("error", () => resolve(buf));
expect((await promise).match(/HTTP\/1\.1 307/g)?.length).toBe(2);

Check warning on line 1274 in test/js/bun/http/bun-serve-file.test.ts

View check run for this annotation

Claude / Claude Code Review

Raw-socket helpers wire error to resolve and never wire close — regressions hang instead of failing

Both raw-socket helpers wire `error` to `resolve(buf)` instead of `reject`, and neither wires `close` — REVIEW.md ('Tests reviewers reject'): *"Wire EVERY failure event (`error`, `close`, `abort`, process exit) to reject the awaited promise"*. In the pipelined-307 block, if a regression makes the server close after the first 307 (e.g. someone swaps this fix for `Connection: close`), node:net emits `end`→`close` — not `error` — so the promise never settles and the test hangs to the file timeout i
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: "0", body: "" });
});
Loading