diff --git a/crates/perry-ext-http/src/server/response.rs b/crates/perry-ext-http/src/server/response.rs index 937c3117d69..f074aadb2a8 100644 --- a/crates/perry-ext-http/src/server/response.rs +++ b/crates/perry-ext-http/src/server/response.rs @@ -220,7 +220,29 @@ impl ServerResponse { self } } +/// node's `STATUS_CODES[statusCode] || 'unknown'`. +pub(crate) fn status_text(code: u16) -> &'static str { + http::StatusCode::from_u16(code) + .ok() + .and_then(|s| s.canonical_reason()) + .unwrap_or("unknown") +} + impl ResponseState { + /// The message the head carries, fixed when the head is committed (node's + /// `writeHead`): a string `reason` is the message, even an empty one; + /// otherwise `statusMessage ||= STATUS_CODES[statusCode] || 'unknown'`. + pub(crate) fn commit_status_message(&mut self, reason: Option) { + let message = match reason { + Some(reason) => reason, + None => match self.status_message.take() { + Some(current) if !current.is_empty() => current, + _ => status_text(self.status_code).to_string(), + }, + }; + self.status_message = Some(message); + } + pub fn new() -> Self { Self { header_fields: None, @@ -796,11 +818,7 @@ pub unsafe extern "C" fn js_node_http_res_write_head( if status.is_finite() && status > 0.0 { sr.status_code = status as u16; } - if let Some(m) = status_message { - if !m.is_empty() { - sr.status_message = Some(m); - } - } + sr.commit_status_message(status_message); if let Some(headers) = direct_headers { sr.committed_headers = Some(headers); } @@ -848,7 +866,8 @@ extern "C" { /// A program that replaced the response's `writeHead` (on-headers, which /// `compression` uses to add `Vary: Accept-Encoding`) observes it; the /// replacement normally ends in the intrinsic, which commits the head. While -/// `writeHead` is the intrinsic nothing runs here: the caller's native commit +/// `writeHead` is the intrinsic, only its message rule runs here +/// (`statusMessage ||= STATUS_CODES[statusCode]`): the caller's native commit /// is that call. pub(crate) fn implicit_header(handle: i64) { let scope = perry_ffi::TransientRootScope::enter(); @@ -873,6 +892,11 @@ pub(crate) fn implicit_header(handle: i64) { if unsafe { js_value_is_closure(method.to_bits() as i64) } == 0 || unsafe { super::response_methods::is_intrinsic_write_head(method) } { + // The intrinsic `writeHead(this.statusCode)` the caller commits + // natively: it carries no reason. + if let Some(sr) = response_state_mut(owner.get()) { + sr.commit_status_message(None); + } return; } let this = perry_ffi::JsThis::from_f64(handle_to_pointer_f64(owner.get())); @@ -896,7 +920,7 @@ fn finish_heap_head(handle: i64, status: f64, reason: Option) { } else { response_state(owner.get()).map_or(200, |sr| sr.status_code) }; - heap::write_head_fields(owner.get(), code, reason.filter(|m| !m.is_empty())); + heap::write_head_fields(owner.get(), code, reason); if response_state(owner.get()).is_some_and(|s| s.committed_headers.is_none()) { let headers = header_values::snapshot(owner.get()); if let Some(sr) = response_state_mut(owner.get()) { @@ -1115,7 +1139,11 @@ pub extern "C" fn js_node_http_res_add_trailers(handle: i64, headers_value: f64) pub(crate) fn finalize_buffered_end(handle: i64, chunk: f64) -> Option<(Vec, Vec)> { let response_scope = perry_ffi::TransientRootScope::enter(); let response_root = response_scope.root_addr(handle); + // `implicit_header` can run a program's `writeHead` (compression's + // on-headers), and a moving collection there relocates a young chunk. + let chunk_root = response_scope.root_nanbox(chunk); implicit_header(response_root.get()); + let chunk = chunk_root.get(); let v = JsValue::from_bits(chunk.to_bits()); let final_chunk = if v.is_undefined() || v.is_null() { None @@ -1600,8 +1628,11 @@ pub extern "C" fn js_node_http_res_write_with_cb(handle: i64, chunk: f64, callba } return 0; } + // `implicit_header` can run JS that moves a young chunk (see + // `finalize_buffered_end`). + let chunk_root = response_scope.root_nanbox(chunk); implicit_header(response_root.get()); - let bytes = jsvalue_to_body_bytes(chunk); + let bytes = jsvalue_to_body_bytes(chunk_root.get()); // Honor streaming mode (after `res.flushHeaders()` / a prior streamed // `res.write`) exactly like `js_node_http_res_write`: the chunk must go down // the stream channel, NOT into `buffered_body`. Without this, a streamed diff --git a/crates/perry-ext-http/src/server/response_payload.rs b/crates/perry-ext-http/src/server/response_payload.rs index a06d15eb68f..e9ba4a0f650 100644 --- a/crates/perry-ext-http/src/server/response_payload.rs +++ b/crates/perry-ext-http/src/server/response_payload.rs @@ -354,11 +354,7 @@ pub(crate) fn write_head_fields(handle: i64, code: u16, reason: Option) let current = unsafe { perry_ffi::runtime_get(owner.get(), b"statusMessage") }; match JsValue::from_bits(current.to_bits()).to_owned_string() { Some(m) if !m.is_empty() => m, - _ => http::StatusCode::from_u16(code) - .ok() - .and_then(|s| s.canonical_reason()) - .unwrap_or("unknown") - .to_string(), + _ => super::response::status_text(code).to_string(), } } }; diff --git a/test-files/test_gap_gc_http_body_chunk_across_write_head.ts b/test-files/test_gap_gc_http_body_chunk_across_write_head.ts new file mode 100644 index 00000000000..c59f1e6b7bb --- /dev/null +++ b/test-files/test_gap_gc_http_body_chunk_across_write_head.ts @@ -0,0 +1,60 @@ +import * as http from "node:http"; + +// A program-replaced `writeHead` (what on-headers does for `compression`) +// runs inside the implicit head of `res.end(chunk)` / `res.write(chunk, cb)`. +// It allocates enough garbage that a collection runs there; the chunk the +// response was handed is a young string and must still be the body. +function churn(): number { + let keep = 0; + for (let i = 0; i < 400000; i++) { + const o = { i, s: "v" + i, a: [i, i + 1] }; + keep += o.a.length; + } + return keep; +} + +function payload(tag: string, n: number): string { + let s = ""; + for (let i = 0; i < n; i++) s += "

" + tag + ":" + i + "

"; + return s; +} + +const server = http.createServer((req, res) => { + const original = res.writeHead; + res.writeHead = function (this: http.ServerResponse, ...args: any[]) { + churn(); + return (original as any).apply(this, args); + } as any; + const url = req.url || "/"; + const body = payload(url, 400); + res.setHeader("X-Expected-Length", String(Buffer.byteLength(body))); + if (url.startsWith("/end")) { + res.end(body); + } else { + res.write(body, () => {}); + res.end(); + } +}); + +function get(port: number, path: string): Promise { + return new Promise((resolve) => { + http.get({ host: "127.0.0.1", port, path }, (res) => { + let n = 0; + let text = ""; + res.on("data", (d: Buffer) => { n += d.length; text += d.toString(); }); + res.on("end", () => { + const expected = Number(res.headers["x-expected-length"]); + const ok = n === expected && text === payload(path, 400); + resolve(path + " status=" + res.statusCode + " bytes=" + n + " expected=" + expected + " same=" + ok); + }); + }); + }); +} + +server.listen(0, "127.0.0.1", async () => { + const port = (server.address() as any).port; + for (const path of ["/end1", "/end2", "/end3", "/write1", "/write2", "/write3"]) { + console.log(await get(port, path)); + } + server.close(); +}); diff --git a/test-files/test_gap_http_status_message_default.ts b/test-files/test_gap_http_status_message_default.ts new file mode 100644 index 00000000000..dd6cc9df893 --- /dev/null +++ b/test-files/test_gap_http_status_message_default.ts @@ -0,0 +1,40 @@ +import * as http from "node:http"; +import * as net from "node:net"; + +// Each case sets the status fields the way a framework does, then answers. +type Case = { name: string; handle: (res: http.ServerResponse) => void }; +const cases: Case[] = [ + { name: "plain end", handle: (res) => res.end("x") }, + { name: "empty statusMessage end", handle: (res) => { res.statusCode = 200; res.statusMessage = ""; res.end("x"); } }, + { name: "empty statusMessage write", handle: (res) => { res.statusCode = 201; res.statusMessage = ""; res.write("x"); res.end(); } }, + { name: "empty statusMessage flushHeaders", handle: (res) => { res.statusCode = 200; res.statusMessage = ""; res.flushHeaders(); res.end("x"); } }, + { name: "custom statusMessage flushHeaders", handle: (res) => { res.statusCode = 200; res.statusMessage = "Fine"; res.flushHeaders(); res.end("x"); } }, + { name: "writeHead empty reason", handle: (res) => { res.writeHead(404, ""); res.end("x"); } }, +]; + +let current = 0; +const server = http.createServer((req, res) => { + const i = Number((req.url || "/0").slice(1)); + cases[i].handle(res); + console.log(cases[i].name + " server statusMessage=" + JSON.stringify(res.statusMessage)); +}); + +function rawStatusLine(port: number, i: number): Promise { + return new Promise((resolve) => { + const s = net.connect(port, "127.0.0.1", () => { + s.write("GET /" + i + " HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + }); + let buf = ""; + s.on("data", (d) => { buf += d.toString(); }); + s.on("end", () => resolve(buf.split("\r\n")[0])); + }); +} + +server.listen(0, "127.0.0.1", async () => { + const port = (server.address() as net.AddressInfo).port; + for (current = 0; current < cases.length; current++) { + const line = await rawStatusLine(port, current); + console.log(cases[current].name + " status line=" + JSON.stringify(line)); + } + server.close(); +});