fix(http): default reason phrase; root the body chunk across a replaced writeHead

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<String>) {
+        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<String>) {
     } 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<i64>, Vec<i64>)> {
     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

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<String>)
             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(),
             }
         }
     };

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 += "<p>" + tag + ":" + i + "</p>";
+  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<string> {
+  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();
+});

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<string> {
+  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();
+});