diff --git a/src/runtime/server/FileRoute.rs b/src/runtime/server/FileRoute.rs index 88691374f5f4..281ae42e3c04 100644 --- a/src/runtime/server/FileRoute.rs +++ b/src/runtime/server/FileRoute.rs @@ -34,6 +34,7 @@ pub struct FileRoute { blob: Blob, headers: Headers, status_code: u16, + status_text: Box<[u8]>, // Mutated on every request (`on()` runs `hash()`); FileRoute is reached via // a shared `*const Self` from the route table, so wrap for interior // mutability. `StatHash` is small POD with `Default`, so `Cell` + @@ -85,6 +86,7 @@ impl FileRoute { size_of::() + self.headers.memory_cost() + self.blob.reported_estimated_size.get() + + self.status_text.len() } pub fn last_modified_date(&self) -> JsResult> { @@ -127,6 +129,7 @@ impl FileRoute { blob, headers, status_code: opts.status_code, + status_text: Box::default(), stat_hash: Cell::new(StatHash::default()), })) } @@ -176,6 +179,8 @@ impl FileRoute { *body_value = BodyValue::Blob(blob.dupe()); let headers = headers_from(response.get_init_headers(), &blob); let status_code = response.status_code(); + 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(FileRoute { ref_count: Cell::new(1), @@ -187,6 +192,7 @@ impl FileRoute { blob, headers, status_code, + status_text: Box::from(status_text_slice.slice()), stat_hash: Cell::new(StatHash::default()), })))); } @@ -210,6 +216,7 @@ impl FileRoute { has_content_range_header: false, has_date_header: false, status_code: 200, + status_text: Box::default(), stat_hash: Cell::new(StatHash::default()), })))); } @@ -273,10 +280,10 @@ impl FileRoute { } } - fn write_status_code(&self, status: u16, resp: AnyResponse) { + fn write_status_code(&self, status: u16, status_text: &[u8], resp: AnyResponse) { match resp { - AnyResponse::SSL(r) => write_status::(r, status), - AnyResponse::TCP(r) => write_status::(r, status), + AnyResponse::SSL(r) => write_status::(r, status, status_text), + AnyResponse::TCP(r) => write_status::(r, status, status_text), AnyResponse::H3(r) => { let mut b = bun_core::fmt::ItoaBuf::new(); let s = bun_core::fmt::itoa(&mut b, status); @@ -520,7 +527,12 @@ impl FileRoute { req.set_yield(false); - this.write_status_code(status_code, resp); + let status_text: &[u8] = if status_code == this.status_code { + &this.status_text + } else { + &[] + }; + this.write_status_code(status_code, status_text, resp); if this.has_date_header { resp.mark_wrote_date_header(); } diff --git a/src/runtime/server/HTMLBundle.rs b/src/runtime/server/HTMLBundle.rs index 937785d53a27..9e1daa5ed416 100644 --- a/src/runtime/server/HTMLBundle.rs +++ b/src/runtime/server/HTMLBundle.rs @@ -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, diff --git a/src/runtime/server/HTTPStatusText.rs b/src/runtime/server/HTTPStatusText.rs index 9787914cb369..5c1f09195a4b 100644 --- a/src/runtime/server/HTTPStatusText.rs +++ b/src/runtime/server/HTTPStatusText.rs @@ -11,6 +11,35 @@ pub const fn is_sendable(code: u16) -> bool { matches!(code, 100..=999) } +/// RFC 9112 §4 `reason-phrase`: HTAB / SP / VCHAR (0x21..=0x7E) / obs-text (0x80..=0xFF). +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; + +/// `" "` for uWS `writeStatus`: prefers a valid `status_text`, +/// then [`get`], then an empty reason phrase. +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] +} + pub fn get(code: u16) -> Option<&'static [u8]> { match code { 100 => Some(b"100 Continue"), diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index f85ad8e6a6a9..1e09841e8168 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -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_); @@ -3613,6 +3613,9 @@ 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 borrow of `init.status_text`; that field is not mutated below. + 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()); @@ -3644,7 +3647,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 @@ -3653,9 +3663,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() { @@ -3754,21 +3764,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) { diff --git a/src/runtime/server/StaticRoute.rs b/src/runtime/server/StaticRoute.rs index 36c2d66b857e..3365c72e481d 100644 --- a/src/runtime/server/StaticRoute.rs +++ b/src/runtime/server/StaticRoute.rs @@ -33,6 +33,7 @@ pub struct StaticRoute { pub(super) ref_count: Cell, pub server: Cell>, pub status_code: u16, + pub status_text: Box<[u8]>, pub blob: AnyBlob, pub cached_blob_size: u64, pub has_date: bool, @@ -111,6 +112,7 @@ impl StaticRoute { headers, server: Cell::new(options.server), status_code: options.status_code, + status_text: Box::default(), })) } @@ -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::() + self.blob.memory_cost() + self.headers.memory_cost() + size_of::() + + self.blob.memory_cost() + + self.headers.memory_cost() + + self.status_text.len() } pub fn from_js( @@ -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, @@ -253,6 +261,7 @@ impl StaticRoute { headers, server: Cell::new(None), status_code: response.status_code(), + status_text: Box::from(status_text_slice.slice()), })))); } @@ -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::(r, status), - AnyResponse::TCP(r) => write_status::(r, status), + AnyResponse::SSL(r) => write_status::(r, status, status_text), + AnyResponse::TCP(r) => write_status::(r, status, status_text), AnyResponse::H3(r) => { let mut b = bun_core::fmt::ItoaBuf::new(); let s = bun_core::fmt::itoa(&mut b, status); @@ -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); } @@ -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()); @@ -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); diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index f50f6ce8b23d..f911d51fa6d4 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -111,7 +111,11 @@ pub use server_body::{ }; // ─── write_status ──────────────────────────────────────────────────────────── -pub fn write_status(resp: *mut uws_sys::NewAppResponse, status: u16) { +pub fn write_status( + resp: *mut uws_sys::NewAppResponse, + 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() { @@ -120,16 +124,8 @@ pub fn write_status(resp: *mut uws_sys::NewAppResponse, st // S008: `Response` 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 ──────────────────────────────────────────────────────────────── diff --git a/test/js/bun/http/serve-stream-body-error.test.ts b/test/js/bun/http/serve-stream-body-error.test.ts index c0e5635ea7bd..b90387f136aa 100644 --- a/test/js/bun/http/serve-stream-body-error.test.ts +++ b/test/js/bun/http/serve-stream-body-error.test.ts @@ -314,7 +314,7 @@ test.skipIf(!isASAN)( // The stream errors after "EB" is on the wire, so the body is force-closed // without the terminating 0\r\n\r\n chunk (RFC 9112 section 7). - const expected = Array(6).fill({ status: "HTTP/1.1 597 HM", terminated: false }); + const expected = Array(6).fill({ status: "HTTP/1.1 597 ", terminated: false }); expect({ stderr, results: stdout.trim() ? JSON.parse(stdout) : stdout, exitCode }).toEqual({ stderr: "", results: expected, diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index d6534b96eae8..24b63d3f1da6 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -1418,6 +1418,107 @@ it("does not write body bytes for null body statuses", async () => { } }); +describe("status line reason phrase", () => { + async function rawStatusLine(port: number, path: string): Promise { + const received: Buffer[] = []; + const { resolve, reject, promise } = Promise.withResolvers(); + 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 dir = tempDir("serve-status-text", { "payload.txt": "hello" }); + 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 }), + "/file-known": new Response(Bun.file(join(String(dir), "payload.txt")), { + status: 201, + statusText: "File Custom", + }), + "/file-unknown-bare": new Response(Bun.file(join(String(dir), "payload.txt")), { + 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 = { + "/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 ", + "/file-known": "HTTP/1.1 201 File Custom", + "/file-unknown-bare": "HTTP/1.1 520 ", + }; + const got: Record = {}; + 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()", () => {