serve: trace every server-level callback from the JS wrapper instead of rooting them as Strong

src/runtime/server/mod.rs

@@ -288,13 +288,6 @@
     /// times due to SNI, so we have to store them.
     pub user_routes: Vec<UserRoute<SSL, DEBUG>>,
 
-    pub on_clienterror: jsc::StrongOptional,
-
-    /// node:http compat: JS callback invoked with the JSNodeHTTPServerSocket
-    /// when a connection is accepted (for TLS, when its handshake completes),
-    /// before any request bytes. Backs `server.emit("connection", ...)`.
-    pub on_connection: jsc::StrongOptional,
-
     pub inspector_server_id: jsc::DebuggerId,
 }
 
@@ -307,8 +300,7 @@
 impl<const SSL: bool, const DEBUG: bool> Drop for NewServer<SSL, DEBUG> {
     fn drop(&mut self) {
         // The remaining owned fields (config, base_url, h3_alt_svc, dev_server,
-        // user_routes, all_closed_promise, on_clienterror, on_connection) drop
-        // automatically.
+        // user_routes, all_closed_promise) drop automatically.
         if let Some(p) = self.plugins.take() {
             // SAFETY: `plugins` carries the `heap::alloc` provenance from
             // `ServePlugins::init`; this releases the server's counted ref.
@@ -1429,6 +1421,66 @@
         }
     }
 
+    pub(crate) fn js_on_client_error_get_cached(server_js: JSValue) -> Option<JSValue> {
+        match (SSL, DEBUG) {
+            (false, false) => route_list_cached::http::on_client_error_get_cached(server_js),
+            (true, false) => route_list_cached::https::on_client_error_get_cached(server_js),
+            (false, true) => route_list_cached::debug_http::on_client_error_get_cached(server_js),
+            (true, true) => route_list_cached::debug_https::on_client_error_get_cached(server_js),
+        }
+    }
+
+    pub(crate) fn js_gc_on_client_error_set(
+        server_js: JSValue,
+        global: &JSGlobalObject,
+        callback: JSValue,
+    ) {
+        match (SSL, DEBUG) {
+            (false, false) => {
+                route_list_cached::http::on_client_error_set_cached(server_js, global, callback)
+            }
+            (true, false) => {
+                route_list_cached::https::on_client_error_set_cached(server_js, global, callback)
+            }
+            (false, true) => {
+                route_list_cached::debug_http::on_client_error_set_cached(server_js, global, callback)
+            }
+            (true, true) => {
+                route_list_cached::debug_https::on_client_error_set_cached(server_js, global, callback)
+            }
+        }
+    }
+
+    pub(crate) fn js_on_connection_get_cached(server_js: JSValue) -> Option<JSValue> {
+        match (SSL, DEBUG) {
+            (false, false) => route_list_cached::http::on_connection_get_cached(server_js),
+            (true, false) => route_list_cached::https::on_connection_get_cached(server_js),
+            (false, true) => route_list_cached::debug_http::on_connection_get_cached(server_js),
+            (true, true) => route_list_cached::debug_https::on_connection_get_cached(server_js),
+        }
+    }
+
+    pub(crate) fn js_gc_on_connection_set(
+        server_js: JSValue,
+        global: &JSGlobalObject,
+        callback: JSValue,
+    ) {
+        match (SSL, DEBUG) {
+            (false, false) => {
+                route_list_cached::http::on_connection_set_cached(server_js, global, callback)
+            }
+            (true, false) => {
+                route_list_cached::https::on_connection_set_cached(server_js, global, callback)
+            }
+            (false, true) => {
+                route_list_cached::debug_http::on_connection_set_cached(server_js, global, callback)
+            }
+            (true, true) => {
+                route_list_cached::debug_https::on_connection_set_cached(server_js, global, callback)
+            }
+        }
+    }
+
     /// Wrap an already-heap-allocated server pointer in its JS object.
     /// Ownership transfers to the C++ wrapper (freed via `finalize`).
     pub fn ptr_to_js(this: *mut Self, global: &JSGlobalObject) -> JSValue {
@@ -1874,7 +1926,7 @@
         }
 
         // owned-field cleanup (all_closed_promise / user_routes /
-        // config / on_clienterror / h3_alt_svc / dev_server / plugins) is
+        // config / h3_alt_svc / dev_server / plugins) is
         // handled by the heap::take drop below — see `impl Drop for NewServer`.
         if Self::HAS_H3 {
             if let Some(h3a) = this_ref.h3_app.take() {
@@ -1940,8 +1992,6 @@
             flags: ServerFlags::default(),
             plugins: None,
             user_routes: Vec::new(),
-            on_clienterror: jsc::StrongOptional::empty(),
-            on_connection: jsc::StrongOptional::empty(),
             inspector_server_id: jsc::DebuggerId::init(0),
         }));
 
@@ -2903,22 +2953,23 @@
 // them here instead of redeclaring with a divergent `req` pointer type.
 use server_body::{Bun__ServerRouteList__callRoute, Bun__ServerRouteList__create};
 
-/// Per-type cached-accessor shims for the `routeList` `WriteBarrier` slot.
-/// `codegen_cached_accessors!` emits `route_list_{get,set}_cached` wrapping
-/// `${T}Prototype__routeList{Get,Set}CachedValue` (generate-classes.ts).
+/// Per-type cached-accessor shims for the server `WriteBarrier` value slots
+/// (`routeList`, `onClientError`, `onConnection`). `codegen_cached_accessors!`
+/// emits `${snake}_{get,set}_cached` wrapping
+/// `${T}Prototype__${prop}{Get,Set}CachedValue` (generate-classes.ts).
 mod route_list_cached {
     pub(super) mod http {
-        bun_jsc::codegen_cached_accessors!("HTTPServer"; routeList);
+        bun_jsc::codegen_cached_accessors!("HTTPServer"; routeList, onClientError, onConnection);
     }
     pub(super) mod https {
-        bun_jsc::codegen_cached_accessors!("HTTPSServer"; routeList);
+        bun_jsc::codegen_cached_accessors!("HTTPSServer"; routeList, onClientError, onConnection);
     }
     pub(super) mod debug_http {
-        bun_jsc::codegen_cached_accessors!("DebugHTTPServer"; routeList);
+        bun_jsc::codegen_cached_accessors!("DebugHTTPServer"; routeList, onClientError, onConnection);
     }
     pub(super) mod debug_https {
-        bun_jsc::codegen_cached_accessors!("DebugHTTPSServer"; routeList);
+        bun_jsc::codegen_cached_accessors!("DebugHTTPSServer"; routeList, onClientError, onConnection);
     }
 }
 
 // ─── extern "C" trampolines ──────────────────────────────────────────────────

src/runtime/server/server.classes.ts

@@ -90,7 +90,7 @@ function generate(name) {
     finalize: true,
     construct: true,
     noConstructor: true,
-    values: ["routeList"],
+    values: ["routeList", "onClientError", "onConnection"],
   });
 }
 export default [

src/runtime/server/server_body.rs

@@ -3498,7 +3498,10 @@ where
         error_code: u8,
         raw_packet: &[u8],
     ) {
-        let Some(callback) = self.on_clienterror.get() else {
+        let Some(server_js) = self.js_value.try_get() else {
+            return;
+        };
+        let Some(callback) = Self::js_on_client_error_get_cached(server_js) else {
             return;
         };
         {
@@ -3546,7 +3549,10 @@ where
     /// `onConnection` callback so `node:http` can emit 'connection' before any
     /// request bytes arrive.
     pub fn on_connection_callback(&mut self, socket: *mut c_void) {
-        let Some(callback) = self.on_connection.get() else {
+        let Some(server_js) = self.js_value.try_get() else {
+            return;
+        };
+        let Some(callback) = Self::js_on_connection_get_cached(server_js) else {
             return;
         };
         let global = self.global();
@@ -3671,8 +3677,7 @@ pub(super) fn server_set_on_client_error_(
                 // SAFETY: as_ returned a non-null *mut to a live server.
                 let this = unsafe { &mut *this };
                 if let Some(app) = this.app {
-                    this.on_clienterror.deinit();
-                    this.on_clienterror = StrongOptional::create(callback, global);
+                    <$T>::js_gc_on_client_error_set(server, global, callback);
                     // uws_sys::App::on_client_error takes the raw C-ABI handler shape;
                     // wrap our typed callback in an extern "C" thunk that slices raw_packet.
                     extern "C" fn thunk(
@@ -3734,8 +3739,7 @@ pub(super) fn server_set_on_connection_(
                 // SAFETY: as_ returned a non-null *mut to a live server.
                 let this = unsafe { &mut *this };
                 if let Some(app) = this.app {
-                    this.on_connection.deinit();
-                    this.on_connection = StrongOptional::create(callback, global);
+                    <$T>::js_gc_on_connection_set(server, global, callback);
                     // uws filters fire with `1` when an HTTP connection is opened
                     // (for TLS, when its handshake completes) and `-1` on close;
                     // only the open notification is forwarded to JS.

test/js/node/http/node-http-uaf.test.ts

@@ -79,3 +79,44 @@ test.concurrent.each([
   });
   expect(JSON.parse(stdout).received).toBeGreaterThan(8 * 1024 * 1024);
 });
+
+test.concurrent("'connection' and 'clientError' callbacks survive GC", async () => {
+  // The server's native struct stores these two node:http callbacks on the JS
+  // wrapper (GC-visited WriteBarrier slots), not in Strong handles. Force GC
+  // between registration and dispatch to prove the wrapper roots them.
+  const src = /* js */ `
+    import http from "node:http";
+    import net from "node:net";
+    import { once } from "node:events";
+
+    let gotConnection = 0;
+    let gotClientError = 0;
+    const server = http.createServer((req, res) => res.end());
+    server.on("connection", () => { gotConnection++; });
+    server.on("clientError", (err, sock) => { gotClientError++; sock.destroy(); });
+    await once(server.listen(0, "127.0.0.1"), "listening");
+
+    for (let i = 0; i < 3; i++) Bun.gc(true);
+
+    const sock = net.connect(server.address().port, "127.0.0.1");
+    sock.on("error", () => {});
+    await once(sock, "connect");
+    for (let i = 0; i < 3; i++) Bun.gc(true);
+    sock.write("!!!garbage!!!\\r\\n\\r\\n");
+    await once(sock, "close");
+    server.close();
+
+    console.log(JSON.stringify({ gotConnection, gotClientError }));
+  `;
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", src],
+    env: bunEnv,
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout: JSON.parse(stdout || "null"), stderr, exitCode }).toEqual({
+    stdout: { gotConnection: 1, gotClientError: 1 },
+    stderr: "",
+    exitCode: 0,
+  });
+});