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
4 changes: 2 additions & 2 deletions src/runtime/server/FileRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@

fn write_status_code(&self, status: u16, resp: AnyResponse) {
match resp {
AnyResponse::SSL(r) => write_status::<true>(r, status),
AnyResponse::TCP(r) => write_status::<false>(r, status),
AnyResponse::SSL(r) => write_status::<true>(r, status, &[]),
AnyResponse::TCP(r) => write_status::<false>(r, status, &[]),

Check warning on line 279 in src/runtime/server/FileRoute.rs

View check run for this annotation

Claude / Claude Code Review

FileRoute does not honor Response statusText (sibling path missed)

`FileRoute` is the sibling of `StaticRoute` here but wasn't given a `status_text` field — `from_js` reads `response.status_code()` but not `get_init_status_text()`, and `write_status_code` hardcodes `&[]`. So after this PR `routes: { "/a": new Response("x", { statusText: "Custom" }) }` emits `Custom` while `routes: { "/b": new Response(Bun.file(p), { statusText: "Custom" }) }` still emits the canned phrase. Consider mirroring the `StaticRoute` change (a `status_text: Box<[u8]>` field populated i
Comment thread
robobun marked this conversation as resolved.
Outdated
AnyResponse::H3(r) => {
let mut b = bun_core::fmt::ItoaBuf::new();
let s = bun_core::fmt::itoa(&mut b, status);
Expand Down
1 change: 1 addition & 0 deletions src/runtime/server/HTMLBundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ impl Route {
blob,
server: Cell::new(Some(server)),
status_code: 200,
status_text: Box::default(),
headers,
cached_blob_size,
has_date: false,
Expand Down
35 changes: 35 additions & 0 deletions src/runtime/server/HTTPStatusText.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,41 @@ pub const fn is_sendable(code: u16) -> bool {
matches!(code, 100..=999)
}

/// RFC 9112 §4: `reason-phrase = 1*( HTAB / SP / VCHAR / obs-text )` where
/// `VCHAR` is `0x21..=0x7E` and `obs-text` is `0x80..=0xFF`. Returns `true`
/// for the empty slice (no phrase, still a legal status line).
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn is_valid_reason_phrase(s: &[u8]) -> bool {
s.iter()
.all(|&c| c == b'\t' || (0x20..=0x7E).contains(&c) || c >= 0x80)
}

pub const STATUS_LINE_BUF: usize = 256;

/// Build the `"<code> <reason>"` string uWS's `writeStatus` expects.
///
/// A non-empty `status_text` that passes [`is_valid_reason_phrase`] is used as
/// the reason (truncated to fit `buf`). Otherwise this falls back to [`get`],
/// then to an empty reason phrase (`"<code> "`), so the placeholder `HM` never
/// reaches the wire.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn format<'a>(buf: &'a mut [u8; STATUS_LINE_BUF], code: u16, status_text: &[u8]) -> &'a [u8] {
let mut itoa = bun_core::fmt::ItoaBuf::new();
let c = bun_core::fmt::itoa(&mut itoa, code);
if !status_text.is_empty() && is_valid_reason_phrase(status_text) {
let msg = &status_text[..status_text.len().min(buf.len() - c.len() - 1)];
let n = c.len() + 1 + msg.len();
buf[..c.len()].copy_from_slice(c);
buf[c.len()] = b' ';
buf[c.len() + 1..n].copy_from_slice(msg);
return &buf[..n];
}
if let Some(canned) = get(code) {
return canned;
}
buf[..c.len()].copy_from_slice(c);
buf[c.len()] = b' ';
&buf[..c.len() + 1]
Comment thread
robobun marked this conversation as resolved.
}

pub fn get(code: u16) -> Option<&'static [u8]> {
match code {
100 => Some(b"100 Continue"),
Expand Down
33 changes: 19 additions & 14 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1860,7 +1860,7 @@ where
fd.close();
}
let mut crbuf = [0u8; RangeRequest::CONTENT_RANGE_BUF];
self.do_write_status(416);
self.do_write_status(416, &[]);
if let Some(response) = self.response_weakref.get() {
if let Some(mut headers_) = response.swap_init_headers() {
self.do_write_headers(&mut headers_);
Expand Down Expand Up @@ -3613,6 +3613,11 @@ where
// an async hop keep the Response rooted via response_protected.
let response: &mut Response = self.response_weakref.get().unwrap();
let mut status = response.status_code();
// +0 bitwise copy; `init.status_text` stays live for the whole request
// (nothing below mutates it), so the borrowed bytes outlive the
// `do_write_status` calls below.
Comment thread
robobun marked this conversation as resolved.
Outdated
let status_text_str = response.get_init_status_text();
let status_text_slice = status_text_str.to_utf8_without_ref();
let mut needs_content_range = self.flags.needs_content_range()
&& (self.sendfile.total > 0 || self.sendfile.remain < self.blob.size());

Expand Down Expand Up @@ -3644,7 +3649,14 @@ where
status = 206;
}

self.do_write_status(status);
self.do_write_status(
status,
if needs_content_range {
&[]
} else {
status_text_slice.slice()
},
);
self.do_write_headers(&mut headers_);
// `HeadersRef` is RAII — its Drop
// already calls `WebCore__FetchHeaders__deref`, so an explicit
Expand All @@ -3653,9 +3665,9 @@ where
drop(headers_);
} else if needs_content_range {
status = 206;
self.do_write_status(status);
self.do_write_status(status, &[]);
} else {
self.do_write_status(status);
self.do_write_status(status, status_text_slice.slice());
}

if let Some(mut cookies) = self.cookies.take() {
Expand Down Expand Up @@ -3754,21 +3766,14 @@ where
}
}

fn do_write_status(&mut self, status: u16) {
fn do_write_status(&mut self, status: u16, status_text: &[u8]) {
debug_assert!(!self.flags.has_written_status());
self.flags.set_has_written_status(true);

// `AnyResponse` is a `Copy` handle; methods take `self` by value.
let Some(resp) = self.resp else { return };
if let Some(text) = HTTPStatusText::get(status) {
resp.write_status(text);
} else {
let mut buf = [0u8; 48];
let mut w = &mut buf[..];
let _ = write!(w, "{} HM", status);
let written = 48 - w.len();
resp.write_status(&buf[..written]);
}
let mut buf = [0u8; HTTPStatusText::STATUS_LINE_BUF];
resp.write_status(HTTPStatusText::format(&mut buf, status, status_text));
}

fn do_write_headers(&mut self, headers: &mut FetchHeaders) {
Expand Down
23 changes: 16 additions & 7 deletions src/runtime/server/StaticRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct StaticRoute {
pub(super) ref_count: Cell<u32>,
pub server: Cell<Option<AnyServer>>,
pub status_code: u16,
pub status_text: Box<[u8]>,
pub blob: AnyBlob,
pub cached_blob_size: u64,
pub has_date: bool,
Expand Down Expand Up @@ -111,6 +112,7 @@ impl StaticRoute {
headers,
server: Cell::new(options.server),
status_code: options.status_code,
status_text: Box::default(),
}))
}

Expand Down Expand Up @@ -142,11 +144,15 @@ impl StaticRoute {
headers: self.headers.clone(),
server: Cell::new(self.server.get()),
status_code: self.status_code,
status_text: self.status_text.clone(),
})))
}

pub fn memory_cost(&self) -> usize {
size_of::<StaticRoute>() + self.blob.memory_cost() + self.headers.memory_cost()
size_of::<StaticRoute>()
+ self.blob.memory_cost()
+ self.headers.memory_cost()
+ self.status_text.len()
}

pub fn from_js(
Expand Down Expand Up @@ -245,6 +251,8 @@ impl StaticRoute {

let cached_blob_size = blob.size();
let has_date = headers.get(b"date").is_some();
let status_text_str = response.get_init_status_text();
let status_text_slice = status_text_str.to_utf8_without_ref();
return Ok(Some(bun_core::heap::into_raw(Box::new(StaticRoute {
ref_count: Cell::new(1),
blob,
Expand All @@ -253,6 +261,7 @@ impl StaticRoute {
headers,
server: Cell::new(None),
status_code: response.status_code(),
status_text: Box::from(status_text_slice.slice()),
}))));
}

Expand Down Expand Up @@ -467,10 +476,10 @@ impl StaticRoute {
resp.try_end(bytes, all_bytes.len(), resp.should_close_connection())
}

fn do_write_status(&self, status: u16, resp: AnyResponse) {
fn do_write_status(&self, status: u16, status_text: &[u8], resp: AnyResponse) {
match resp {
AnyResponse::SSL(r) => write_status::<true>(r, status),
AnyResponse::TCP(r) => write_status::<false>(r, status),
AnyResponse::SSL(r) => write_status::<true>(r, status, status_text),
AnyResponse::TCP(r) => write_status::<false>(r, status, status_text),
AnyResponse::H3(r) => {
let mut b = bun_core::fmt::ItoaBuf::new();
let s = bun_core::fmt::itoa(&mut b, status);
Expand Down Expand Up @@ -513,7 +522,7 @@ impl StaticRoute {
}

fn render_metadata(&self, resp: AnyResponse) {
self.do_write_status(self.status_code, resp);
self.do_write_status(self.status_code, &self.status_text, resp);
self.do_write_headers(resp);
}

Expand All @@ -526,7 +535,7 @@ impl StaticRoute {
Method::GET => Self::on(this, resp),
Method::HEAD => Self::on_head(this, resp),
_ => {
(*this).do_write_status(405, resp); // Method not allowed
(*this).do_write_status(405, &[], resp); // Method not allowed
resp.write_header(b"Allow", b"GET, HEAD");
resp.write_header_int(b"Content-Length", 0);
resp.end_without_body(resp.should_close_connection());
Expand Down Expand Up @@ -622,7 +631,7 @@ impl StaticRoute {
server.on_pending_request();
resp.timeout(server.config().idle_timeout);
}
(*this).do_write_status(status, resp);
(*this).do_write_status(status, &[], resp);
(*this).do_write_headers(resp);
if !HTTPStatusText::is_null_body(status) {
resp.write_header_int(b"Content-Length", 0);
Expand Down
18 changes: 7 additions & 11 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@ pub use server_body::{
};

// ─── write_status ────────────────────────────────────────────────────────────
pub fn write_status<const SSL: bool>(resp: *mut uws_sys::NewAppResponse<SSL>, status: u16) {
pub fn write_status<const SSL: bool>(
resp: *mut uws_sys::NewAppResponse<SSL>,
status: u16,
status_text: &[u8],
) {
// The route handlers (`StaticRoute`/`FileRoute`) call here from completion
// paths where the request may already be aborted/detached, so no-op on null.
if resp.is_null() {
Expand All @@ -120,16 +124,8 @@ pub fn write_status<const SSL: bool>(resp: *mut uws_sys::NewAppResponse<SSL>, st
// S008: `Response<SSL>` is a ZST opaque — safe `*mut → &mut` deref
// (non-null checked above).
let resp = bun_opaque::opaque_deref_mut(resp);
if let Some(text) = HTTPStatusText::get(status) {
resp.write_status(text);
} else {
use std::io::Write as _;
let mut buf = [0u8; 48];
let mut cursor = &mut buf[..];
write!(cursor, "{} HM", status).expect("unreachable");
let written = 48 - cursor.len();
resp.write_status(&buf[..written]);
}
let mut buf = [0u8; HTTPStatusText::STATUS_LINE_BUF];
resp.write_status(HTTPStatusText::format(&mut buf, status, status_text));
}

// ─── AnyRoute ────────────────────────────────────────────────────────────────
Expand Down
91 changes: 91 additions & 0 deletions test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,97 @@ it("does not write body bytes for null body statuses", async () => {
}
});

describe("status line reason phrase", () => {
async function rawStatusLine(port: number, path: string): Promise<string> {
const received: Buffer[] = [];
const { resolve, reject, promise } = Promise.withResolvers<void>();
await using connection = await Bun.connect({
hostname: "127.0.0.1",
port,
socket: {
data(_s, data) {
received.push(data);
},
end() {
resolve();
},
error(_s, error) {
reject(error);
},
close() {
resolve();
},
},
});
connection.write(`GET ${path} HTTP/1.1\r\nHost: 127.0.0.1\r\nConnection: close\r\n\r\n`);
connection.flush();
await promise;
return Buffer.concat(received).toString("latin1").split("\r\n")[0];
}

it("writes the Response statusText and never the HM placeholder", async () => {
using server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
routes: {
"/static-known": new Response("x", { status: 201, statusText: "Static Custom" }),
"/static-unknown": new Response("x", { status: 599, statusText: "Edge Cache Error" }),
"/static-unknown-bare": new Response("x", { status: 520 }),
},
fetch(req) {
switch (new URL(req.url).pathname) {
case "/known-bare":
return new Response("x", { status: 201 });
case "/unknown-bare":
return new Response("x", { status: 599 });
case "/unknown-custom":
return new Response("x", { status: 520, statusText: "Edge" });
case "/known-custom":
return new Response("x", { status: 404, statusText: "Nope" });
case "/injection":
return new Response("x", { status: 200, statusText: "OK\r\nX-Injected: 1" });
case "/injection-unknown":
return new Response("x", { status: 599, statusText: "x\r\ny" });
default:
return new Response("404", { status: 404 });
}
},
});

const cases: Record<string, string> = {
"/known-bare": "HTTP/1.1 201 Created",
"/unknown-bare": "HTTP/1.1 599 ",
"/unknown-custom": "HTTP/1.1 520 Edge",
"/known-custom": "HTTP/1.1 404 Nope",
// A statusText containing CR/LF is discarded (response splitting defence)
// and falls through to the default phrase.
"/injection": "HTTP/1.1 200 OK",
"/injection-unknown": "HTTP/1.1 599 ",
"/static-known": "HTTP/1.1 201 Static Custom",
"/static-unknown": "HTTP/1.1 599 Edge Cache Error",
"/static-unknown-bare": "HTTP/1.1 520 ",
};
const got: Record<string, string> = {};
for (const path of Object.keys(cases)) {
got[path] = await rawStatusLine(server.port, path);
}
expect(got).toEqual(cases);
});

it("fetch() sees the custom reason phrase", async () => {
using server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
fetch() {
return new Response("x", { status: 499, statusText: "Client Closed Request" });
},
});
const res = await fetch(server.url);
expect(res.status).toBe(499);
expect(res.statusText).toBe("Client Closed Request");
});
});

// Response.error() is a WHATWG network error: its status is 0, which has no
// representation in an HTTP status line. It must never be written to the socket.
describe("Response.error()", () => {
Expand Down
Loading