From ec0f11c3ada6a23d3e14c07e8a02d63e51d5d2d0 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 4 Jun 2026 13:07:12 -0700 Subject: [PATCH 1/3] net: throw on an invalid socket handler config instead of aborting --- src/runtime/socket/Handlers.rs | 71 +++++++++++++++++++++------------- test/js/bun/net/socket.test.ts | 26 +++++++++++++ 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/src/runtime/socket/Handlers.rs b/src/runtime/socket/Handlers.rs index df148474690f..5d16c7386b10 100644 --- a/src/runtime/socket/Handlers.rs +++ b/src/runtime/socket/Handlers.rs @@ -286,6 +286,38 @@ impl Handlers { generated: &GeneratedSocketConfigHandlers, is_server: bool, ) -> JsResult { + // Validate the config before constructing a `Handlers`: an invalid + // handler must return an error while there is nothing to clean up. + // A constructed-but-not-yet-`protect()`ed `Handlers` whose `Drop` runs + // `unprotect()` would trip `debug_assert!(protection_count > 0)` (debug + // abort) and unprotect callbacks `protect()` never rooted (release). + macro_rules! validate_callback { + ($field:ident, $name:literal) => {{ + let value = generated.$field; + if !value.is_undefined_or_null() && !value.is_callable() { + return Err(global_object.throw_invalid_arguments(format_args!( + "Expected \"{}\" callback to be a function", + $name + ))); + } + }}; + } + validate_callback!(on_open, "onOpen"); + validate_callback!(on_close, "onClose"); + validate_callback!(on_data, "onData"); + validate_callback!(on_writable, "onWritable"); + validate_callback!(on_timeout, "onTimeout"); + validate_callback!(on_connect_error, "onConnectError"); + validate_callback!(on_end, "onEnd"); + validate_callback!(on_error, "onError"); + validate_callback!(on_handshake, "onHandshake"); + + if !generated.on_data.is_callable() && !generated.on_writable.is_callable() { + return Err(global_object.throw_invalid_arguments(format_args!( + "Expected at least \"data\" or \"drain\" callback" + ))); + } + let mut result = Handlers { on_open: JSValue::ZERO, on_close: JSValue::ZERO, @@ -316,36 +348,23 @@ impl Handlers { protection_count: 0, }; - // inline for (callback_fields) |field| { ... @field(generated, field) ... } + // Callbacks are validated above; assign the ones that are present. macro_rules! assign_callback { - ($field:ident, $name:literal) => {{ - let value = generated.$field; - if value.is_undefined_or_null() { - } else if !value.is_callable() { - return Err(global_object.throw_invalid_arguments(format_args!( - "Expected \"{}\" callback to be a function", - $name - ))); - } else { - result.$field = value; + ($field:ident) => {{ + if generated.$field.is_callable() { + result.$field = generated.$field; } }}; } - assign_callback!(on_open, "onOpen"); - assign_callback!(on_close, "onClose"); - assign_callback!(on_data, "onData"); - assign_callback!(on_writable, "onWritable"); - assign_callback!(on_timeout, "onTimeout"); - assign_callback!(on_connect_error, "onConnectError"); - assign_callback!(on_end, "onEnd"); - assign_callback!(on_error, "onError"); - assign_callback!(on_handshake, "onHandshake"); - - if result.on_data.is_empty() && result.on_writable.is_empty() { - return Err(global_object.throw_invalid_arguments(format_args!( - "Expected at least \"data\" or \"drain\" callback" - ))); - } + assign_callback!(on_open); + assign_callback!(on_close); + assign_callback!(on_data); + assign_callback!(on_writable); + assign_callback!(on_timeout); + assign_callback!(on_connect_error); + assign_callback!(on_end); + assign_callback!(on_error); + assign_callback!(on_handshake); result.with_async_context_if_needed(global_object); result.protect(); Ok(result) diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index ead9e8d856e9..908197daf1c1 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -1558,3 +1558,29 @@ it("node:net connect() reusing a server-accepted handle keeps the listener's han expect(exitCode).toBe(0); void stderr; }); + +it("Bun.listen with an invalid socket handler throws ERR_INVALID_ARG_TYPE instead of aborting", async () => { + // Regression: a non-function handler used to abort the debug process (Drop ran + // unprotect() on a Handlers that protect() never rooted). It must throw a + // catchable error. Run in a subprocess so a regression (abort) shows up as a + // non-zero exit rather than killing the test runner. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `try { + Bun.listen({ port: 0, hostname: "127.0.0.1", socket: { data: 123 } }); + process.exit(2); // did not throw + } catch (e) { + process.stdout.write(String(e.code)); + process.exit(0); + }`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("ERR_INVALID_ARG_TYPE"); + expect(exitCode).toBe(0); +}); From 99e7ea3b75477dfc1ba264103308c746a8c5145c Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 4 Jun 2026 20:28:51 +0000 Subject: [PATCH 2/3] test: cover the missing data/drain validation path in the invalid-handler regression test --- test/js/bun/net/socket.test.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index 908197daf1c1..cdf0973f9248 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -1560,10 +1560,11 @@ it("node:net connect() reusing a server-accepted handle keeps the listener's han }); it("Bun.listen with an invalid socket handler throws ERR_INVALID_ARG_TYPE instead of aborting", async () => { - // Regression: a non-function handler used to abort the debug process (Drop ran - // unprotect() on a Handlers that protect() never rooted). It must throw a - // catchable error. Run in a subprocess so a regression (abort) shows up as a - // non-zero exit rather than killing the test runner. + // Regression: an invalid handlers object used to abort the debug process on + // both validation error paths (Drop ran unprotect() on a Handlers that + // protect() never rooted). It must throw a catchable error. Run in a + // subprocess so a regression (abort) shows up as a non-zero exit rather than + // killing the test runner. await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -1572,8 +1573,13 @@ it("Bun.listen with an invalid socket handler throws ERR_INVALID_ARG_TYPE instea Bun.listen({ port: 0, hostname: "127.0.0.1", socket: { data: 123 } }); process.exit(2); // did not throw } catch (e) { - process.stdout.write(String(e.code)); - process.exit(0); + console.log(e.code + ": " + e.message); + } + try { + Bun.listen({ port: 0, hostname: "127.0.0.1", socket: {} }); + process.exit(2); // did not throw + } catch (e) { + console.log(e.code + ": " + e.message); }`, ], env: bunEnv, @@ -1581,6 +1587,9 @@ it("Bun.listen with an invalid socket handler throws ERR_INVALID_ARG_TYPE instea stderr: "pipe", }); const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stdout).toBe("ERR_INVALID_ARG_TYPE"); + expect(stdout).toBe( + 'ERR_INVALID_ARG_TYPE: Expected "onData" callback to be a function\n' + + 'ERR_INVALID_ARG_TYPE: Expected at least "data" or "drain" callback\n', + ); expect(exitCode).toBe(0); }); From ee7e7358b7ecf2de45a3abc8bd87c33d1a150e01 Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 4 Jun 2026 20:41:13 +0000 Subject: [PATCH 3/3] ci: retrigger