diff --git a/changelog.d/10238-http-listen-arguments.md b/changelog.d/10238-http-listen-arguments.md new file mode 100644 index 0000000000..5773a0d3dd --- /dev/null +++ b/changelog.d/10238-http-listen-arguments.md @@ -0,0 +1,5 @@ +Fix dynamic HTTP, HTTPS, and HTTP/2 `server.listen()` argument handling. The handle dispatcher passed a stack buffer shaped like an array to the managed-array accessor, which returned `NaN` for both the requested port and completion callback. This could bind the default port and leave the caller waiting for a callback that was lost (#10137). + +Share listen-overload parsing between real runtime arrays and borrowed argument values, then pass the parsed arguments directly to the existing server implementations. Remove the same fabricated-array pattern from `Bun.serve`. Real arrays retain the offset-aware accessor, including arrays whose dense queue prefix has been shifted away; no runtime/codegen array layout changes are needed. + +Tests cover borrowed port/host/backlog/callback overloads and a real shifted argument array. The original HTTP/2 settings/ping/close callback fixture, which timed out before the fix, now matches Node. diff --git a/crates/perry-ext-http/src/server/bun_server.rs b/crates/perry-ext-http/src/server/bun_server.rs index bf1de40321..e4e8e30ff3 100644 --- a/crates/perry-ext-http/src/server/bun_server.rs +++ b/crates/perry-ext-http/src/server/bun_server.rs @@ -125,13 +125,6 @@ lazy_static! { Mutex::new(HashMap::new()); } -#[repr(C)] -struct InlineArgsHeader { - length: u32, - capacity: u32, - args: [u64; 1], -} - struct ClosureCallResult { value: f64, thrown: Option, @@ -286,12 +279,8 @@ pub unsafe extern "C" fn js_bun_serve(options: f64) -> i64 { crate::server::ensure_gc_scanner_registered(); let handle = register_handle(server); - let args = InlineArgsHeader { - length: 1, - capacity: 1, - args: [options.get().to_bits()], - }; - crate::server::server::js_node_http_server_listen(handle, &args as *const _ as i64); + let parsed = crate::server::types::parse_listen_values(std::iter::once(options.get())); + crate::server::server::listen_http_server(handle, parsed); if !get_handle::(handle) .map(|server| server.listening) .unwrap_or(false) diff --git a/crates/perry-ext-http/src/server/handle_dispatch.rs b/crates/perry-ext-http/src/server/handle_dispatch.rs index 35eab6317a..0f1bf6fb82 100644 --- a/crates/perry-ext-http/src/server/handle_dispatch.rs +++ b/crates/perry-ext-http/src/server/handle_dispatch.rs @@ -17,8 +17,8 @@ //! feature, which `optimized_libs.rs` already auto-activates whenever //! `node:http` / `node:https` / `node:http2` is imported) calls //! `js_ext_http_server_is_handle`; on a hit it forwards to -//! `js_ext_http_server_dispatch_method`, which routes to the same -//! `js_node_http_server_*` externs that the static native_table path uses. +//! `js_ext_http_server_dispatch_method`, which routes to the same server +//! implementations that the static native_table path uses. //! //! Issue #2153. @@ -40,7 +40,6 @@ struct ErrorHeader { } extern "C" { - fn js_node_http_server_listen(server_handle: i64, args_array: i64); fn js_node_http_server_close(server_handle: i64, callback: i64); fn js_node_http_server_close_all_connections(handle: i64); fn js_node_http_server_close_idle_connections(handle: i64); @@ -62,7 +61,6 @@ extern "C" { fn js_node_http_server_set_timeout_method(handle: i64, msecs: f64, callback: i64) -> i64; fn js_node_http_server_ref(handle: i64) -> i64; fn js_node_http_server_unref(handle: i64) -> i64; - fn js_node_https_server_listen(server_handle: i64, args_array: i64) -> i64; fn js_node_https_server_close(server_handle: i64, callback: i64); fn js_node_https_server_close_all_connections(handle: i64); fn js_node_https_server_close_idle_connections(handle: i64); @@ -75,7 +73,6 @@ extern "C" { fn js_node_https_server_set_timeout_method(handle: i64, msecs: f64, callback: i64) -> i64; fn js_node_https_server_ref(handle: i64) -> i64; fn js_node_https_server_unref(handle: i64) -> i64; - fn js_node_http2_server_listen(server_handle: i64, args_array: i64) -> i64; fn js_node_http2_server_close(server_handle: i64, callback: i64); fn js_node_http2_server_address_json(handle: i64) -> *mut StringHeader; fn js_node_http2_server_on( @@ -234,19 +231,6 @@ fn http_server_method_bytes(name: &str) -> Option<&'static [u8]> { } } -/// Build a transient `ArrayHeader`-shaped buffer carrying NaN-boxed args. -/// `js_node_http_server_listen` reads its `args_array` arg as a raw -/// `*const ArrayHeader`; the codegen's `NA_VARARGS` path packs one for the -/// direct dispatch, so we mimic that layout here. The buffer lives only -/// for the duration of the call. -#[repr(C)] -struct InlineArgsHeader { - length: u32, - capacity: u32, - // up to 8 packed u64 args follow inline - args: [u64; 8], -} - /// Dispatch a method on a registered `HttpServer` handle. Method name is a /// UTF-8 ptr+len; args are NaN-boxed f64s (the perry-runtime /// `js_native_call_method` shape). Returns NaN-boxed undefined for methods @@ -281,22 +265,13 @@ pub unsafe extern "C" fn js_ext_http_server_dispatch_method( match method.as_str() { "listen" => { - let n = args.len().min(8); - let mut inline = InlineArgsHeader { - length: n as u32, - capacity: n as u32, - args: [0; 8], - }; - for i in 0..n { - inline.args[i] = args[i].to_bits(); - } - let args_array = &inline as *const _ as i64; + let parsed = crate::server::types::parse_listen_values(args.iter().take(8).copied()); if is_h2 { - js_node_http2_server_listen(handle, args_array); + crate::server::http2_server::listen_http2_server(handle, parsed); } else if is_https { - js_node_https_server_listen(handle, args_array); + crate::server::https_server::listen_https_server(handle, parsed); } else { - js_node_http_server_listen(handle, args_array); + crate::server::server::listen_http_server(handle, parsed); } // Node returns the server for chaining (`createServer(...).listen(p).address()`). self_ref diff --git a/crates/perry-ext-http/src/server/http2_server.rs b/crates/perry-ext-http/src/server/http2_server.rs index 6da36ab36d..2b149a274e 100644 --- a/crates/perry-ext-http/src/server/http2_server.rs +++ b/crates/perry-ext-http/src/server/http2_server.rs @@ -491,8 +491,17 @@ pub unsafe extern "C" fn js_node_http2_create_server(first_arg: f64, second_arg: /// / `parse_listen_args` for the overload resolution. Issue #2041. #[no_mangle] pub unsafe extern "C" fn js_node_http2_server_listen(server_handle: i64, args_array: i64) -> i64 { + listen_http2_server( + server_handle, + crate::server::types::parse_listen_args(args_array), + ) +} + +pub(super) unsafe fn listen_http2_server( + server_handle: i64, + parsed: crate::server::types::ListenArgs, +) -> i64 { // Returns `server_handle` for chainability (#2129). - let parsed = crate::server::types::parse_listen_args(args_array); let opts_f64 = parsed.opts; let port = extract_port(opts_f64, 443); let host = parsed diff --git a/crates/perry-ext-http/src/server/https_server.rs b/crates/perry-ext-http/src/server/https_server.rs index 5d58eae7f8..a10d72dd10 100644 --- a/crates/perry-ext-http/src/server/https_server.rs +++ b/crates/perry-ext-http/src/server/https_server.rs @@ -420,8 +420,17 @@ pub(crate) fn set_ticket_keys(server_handle: i64, value: f64) { /// for the overload resolution. Issue #2041. #[no_mangle] pub unsafe extern "C" fn js_node_https_server_listen(server_handle: i64, args_array: i64) -> i64 { + listen_https_server( + server_handle, + crate::server::types::parse_listen_args(args_array), + ) +} + +pub(super) unsafe fn listen_https_server( + server_handle: i64, + parsed: crate::server::types::ListenArgs, +) -> i64 { // Returns `server_handle` for chainability (#2129). - let parsed = crate::server::types::parse_listen_args(args_array); let opts_f64 = parsed.opts; let port = extract_port(opts_f64, 443); let host = parsed diff --git a/crates/perry-ext-http/src/server/server.rs b/crates/perry-ext-http/src/server/server.rs index c6191c6cbc..92e6ef3d13 100644 --- a/crates/perry-ext-http/src/server/server.rs +++ b/crates/perry-ext-http/src/server/server.rs @@ -777,10 +777,19 @@ fn spawn_rr_inject_loop( /// and the (single) function callback wherever it lands. Issue #2041. #[no_mangle] pub unsafe extern "C" fn js_node_http_server_listen(server_handle: i64, args_array: i64) -> i64 { + listen_http_server( + server_handle, + crate::server::types::parse_listen_args(args_array), + ) +} + +pub(super) unsafe fn listen_http_server( + server_handle: i64, + parsed: crate::server::types::ListenArgs, +) -> i64 { // Returns `server_handle` so `createServer(...).listen(...).on(...)` chains // correctly. Pre-#2129 this was `-> ()` and chained sites broke at runtime // with `undefined.on is not a function`. - let parsed = crate::server::types::parse_listen_args(args_array); let opts_f64 = parsed.opts; let port = extract_port(opts_f64, 3000); let host = parsed diff --git a/crates/perry-ext-http/src/server/types.rs b/crates/perry-ext-http/src/server/types.rs index 0e5c780e0b..e545aeb9ff 100644 --- a/crates/perry-ext-http/src/server/types.rs +++ b/crates/perry-ext-http/src/server/types.rs @@ -149,25 +149,35 @@ pub struct ListenArgs { /// `listen(cb)` form (first and only arg is a function) is handled too. /// /// # Safety -/// `args_array` must be `0`/null or a valid Perry-runtime `ArrayHeader`. +/// `args_array` must be `0`/null or a live, forwarding-resolved, GC-managed +/// Perry-runtime `ArrayHeader`. Borrowed values use `parse_listen_values`. pub unsafe fn parse_listen_args(args_array: i64) -> ListenArgs { - let mut out = ListenArgs { - opts: f64::from_bits(TAG_UNDEFINED), - host: None, - callback: 0, - }; let arr_ptr = args_array as *const ArrayHeader; if arr_ptr.is_null() { - return out; + return parse_listen_values(std::iter::empty()); } // Codegen passes a clean raw pointer; reject a stray NaN-boxed value // rather than dereferencing tag bits as an address. if (args_array as u64) >> 48 != 0 { - return out; + return parse_listen_values(std::iter::empty()); } let len = (*arr_ptr).length as usize; - for i in 0..len { - let bits = perry_ffi::js_array_get(arr_ptr, i as u32).bits(); + parse_listen_values( + (0..len).map(|i| f64::from_bits(perry_ffi::js_array_get(arr_ptr, i as u32).bits())), + ) +} + +/// Resolve listen overloads from values, without requiring an array container. +/// Dynamic handle dispatch already has a borrowed argument slice; it must not +/// disguise stack storage as a managed `ArrayHeader` (#10137). +pub(super) unsafe fn parse_listen_values(values: impl IntoIterator) -> ListenArgs { + let mut out = ListenArgs { + opts: f64::from_bits(TAG_UNDEFINED), + host: None, + callback: 0, + }; + for (i, value) in values.into_iter().enumerate() { + let bits = value.to_bits(); let v = JsValue::from_bits(bits); // The completion callback is the (single) function argument — match it // by value type, not position, so it's picked up wherever it floats. @@ -400,6 +410,62 @@ mod tests { ); } + extern "C" fn listen_test_callback() -> f64 { + f64::from_bits(TAG_UNDEFINED) + } + + #[test] + fn listen_borrowed_values_preserve_port_host_backlog_and_callback() { + let scope = perry_ffi::TransientRootScope::enter(); + let callback = scope.root_nanbox(f64::from_bits( + JsValue::from_object_ptr(perry_runtime::closure::js_closure_alloc( + listen_test_callback as *const u8, + 0, + )) + .bits(), + )); + let host = scope.root_nanbox(f64::from_bits( + JsValue::from_string_ptr(perry_ffi::alloc_string("127.0.0.1").as_raw()).bits(), + )); + unsafe { + // The dynamic dispatcher receives values with no array/GC header. + let parsed = parse_listen_values([0.0, host.get(), 128.0, callback.get()]); + assert_eq!(parsed.opts.to_bits(), 0.0f64.to_bits()); + assert_eq!(extract_port(parsed.opts, 443), 0); + assert_eq!(parsed.host.as_deref(), Some("127.0.0.1")); + assert_eq!(parsed.callback as u64, callback.get().to_bits() & PTR_MASK); + + let parsed = parse_listen_values([0.0, callback.get()]); + assert_eq!(extract_port(parsed.opts, 443), 0); + assert!(parsed.host.is_none()); + assert_eq!(parsed.callback as u64, callback.get().to_bits() & PTR_MASK); + + let parsed = parse_listen_values([callback.get()]); + assert_eq!(parsed.opts.to_bits(), TAG_UNDEFINED); + assert!(parsed.host.is_none()); + assert_eq!(parsed.callback as u64, callback.get().to_bits() & PTR_MASK); + } + } + + #[test] + fn listen_managed_array_keeps_shifted_element_storage() { + unsafe { + let args = make_args(&[ + JsValue::from_number(999.0), + JsValue::from_number(0.0), + JsValue::from_string_ptr(perry_ffi::alloc_string("127.0.0.1").as_raw()), + ]); + let arr = args as *mut perry_runtime::array::ArrayHeader; + let capacity = (*arr).capacity; + assert_eq!(perry_runtime::array::js_array_shift_f64(arr), 999.0); + assert_eq!((*arr).capacity, capacity - 1, "queue offset must be live"); + let parsed = parse_listen_args(args); + assert_eq!(extract_port(parsed.opts, 443), 0); + assert_eq!(parsed.host.as_deref(), Some("127.0.0.1")); + assert_eq!(parsed.callback, 0); + } + } + /// Encode `bytes` (len ≤ 5) as an inline SSO `SHORT_STRING_TAG` /// NaN-box, mirroring the runtime's `JSValue::try_short_string`: /// tag 0x7FF9, length in bits 40..=47, data little-endian in bits