From 45a61b60848372728ea8d0d140db25b00b46b515 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Mon, 8 Jun 2026 14:49:29 -0700 Subject: [PATCH 1/4] Remove dead code from HTTP client, h2, and websocket modules --- src/http/h2_client/dispatch.rs | 32 +- src/http/lib.rs | 251 +++++--------- src/http_jsc/websocket_client.rs | 267 +++++++------- src/http_types/h2.rs | 38 ++ src/js/internal/http.ts | 83 +++++ src/js/internal/http/FakeSocket.ts | 66 +--- src/js/node/_http_server.ts | 66 +--- src/runtime/api/bun/h2_frame_parser.rs | 463 +++++++++---------------- 8 files changed, 502 insertions(+), 764 deletions(-) diff --git a/src/http/h2_client/dispatch.rs b/src/http/h2_client/dispatch.rs index e0b19b2a4bc..f0d3e68523b 100644 --- a/src/http/h2_client/dispatch.rs +++ b/src/http/h2_client/dispatch.rs @@ -734,31 +734,9 @@ pub(crate) fn strip_padding(payload: &[u8]) -> Option<&[u8]> { /// hop-by-hop fields. Names from lshpack are already lowercase for table /// hits but a literal can carry anything. pub(crate) fn is_malformed_response_field(name: &[u8]) -> bool { - if name.is_empty() { + if name.is_empty() || !name.iter().all(|&c| wire::is_lower_tchar(c)) { return true; } - for &c in name { - match c { - b'a'..=b'z' - | b'0'..=b'9' - | b'!' - | b'#' - | b'$' - | b'%' - | b'&' - | b'\'' - | b'*' - | b'+' - | b'-' - | b'.' - | b'^' - | b'_' - | b'`' - | b'|' - | b'~' => {} - _ => return true, - } - } matches!( name, b"connection" @@ -770,13 +748,7 @@ pub(crate) fn is_malformed_response_field(name: &[u8]) -> bool { ) } -/// RFC 9113 §8.2.1: a field value MUST NOT contain NUL (0x00), LF (0x0a), or -/// CR (0x0d). HPACK is length-prefixed so these would otherwise pass through -/// verbatim, breaking the no-CR/LF invariant the HTTP/1.1 parser provides and -/// enabling header injection when values are forwarded downstream. -pub fn is_malformed_response_value(value: &[u8]) -> bool { - value.iter().any(|&c| c == 0 || c == b'\r' || c == b'\n') -} +pub use wire::is_malformed_field_value as is_malformed_response_value; pub fn error_code_for(err: bun_core::Error) -> wire::ErrorCode { // bun_core::Error is a NonZeroU16 interned tag; `err!()` yields diff --git a/src/http/lib.rs b/src/http/lib.rs index 82ba943c9e1..d921470a420 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -3630,49 +3630,7 @@ impl<'a> HTTPClient<'a> { if self.flags.protocol != Protocol::Http1_1 { return self.send_progress_update_multiplexed(); } - // reshaped for borrowck — `to_result()` returns an - // `HTTPClientResult<'_>` whose lifetime is tied to `&mut self` (via the - // `body: &mut MutableString` borrow). Holding that result across the - // `is_done` mutations below would require a second live `&mut Self`, - // which PORTING.md §Forbidden flags as aliased `&mut`. Instead: - // snapshot every owned/Copy field out of the result, drop it, mutate - // `self` directly, then rebuild a fresh `HTTPClientResult` for the - // callback from the snapshotted fields + the restored body. - let body = self.state.body_out_str; - // Snapshot the body buffer's CONTENTS by value so that `state.reset()` - // — which calls `body.reset()` and clears the list — doesn't deliver - // an empty body when `is_done`. Restored below before the callback. - let body_snapshot = body_out::take_list(body); - let callback = self.result_callback; - - let ( - has_more, - redirected, - can_stream, - is_http2, - fail, - metadata, - body_size, - certificate_info, - ) = { - let r = self.to_result(); - ( - r.has_more, - r.redirected, - r.can_stream, - r.is_http2, - r.fail, - r.metadata, - r.body_size, - r.certificate_info, - ) - }; // r (and its &mut borrow of self) dropped here - let is_done = !has_more; - - bun_core::scoped_log!(fetch, "progressUpdate {}", is_done); - - if is_done { - self.unregister_abort_tracker(); + self.send_progress_update_inner(|this| { // is_done is response-driven. A server can reply early (HTTP 413) // with keep-alive while request_stage is still .proxy_body or the // tunnel still has buffered encrypted writes. Pooling that tunnel @@ -3685,8 +3643,8 @@ impl<'a> HTTPClient<'a> { // ends on inner-TLS close; ProxyTunnel.onClose fires but the outer // socket is still alive. Pooling that dead wrapper would hang the // next request (proxy.write() → error.ConnectionClosed, swallowed). - let tunnel_poolable = if let Some(t) = self.proxy_tunnel.as_deref() { - self.state.request_stage == RequestStage::Done + let tunnel_poolable = if let Some(t) = this.proxy_tunnel.as_deref() { + this.state.request_stage == RequestStage::Done && t.write_buffer.is_empty() && t.wrapper .as_ref() @@ -3710,12 +3668,12 @@ impl<'a> HTTPClient<'a> { // so for byte-buffer bodies check the unsent slice instead. // Stream/Sendfile are left as-is (they don't track an // unsent slice here). - let request_side_drained = match &self.state.original_request_body { - HTTPRequestBody::Bytes(_) => self.state.request_body.is_empty(), + let request_side_drained = match &this.state.original_request_body { + HTTPRequestBody::Bytes(_) => this.state.request_body.is_empty(), _ => true, }; - if self.is_keep_alive_possible() + if this.is_keep_alive_possible() && !socket.is_closed_or_has_error() && tunnel_poolable && request_side_drained @@ -3724,9 +3682,9 @@ impl<'a> HTTPClient<'a> { // Hand the client's strong ref straight to the pool: `release_socket` // either stores this `RefPtr` in the parked `PooledSocket` or // dereffs it if pooling fails. - let tunnel = self.proxy_tunnel.take(); + let tunnel = this.proxy_tunnel.take(); if let Some(t) = &tunnel { - proxy_tunnel::raw_as_mut(t.as_ptr()).detach_owner(&*self); + proxy_tunnel::raw_as_mut(t.as_ptr()).detach_owner(&*this); } let had_tunnel = tunnel.is_some(); // target_hostname = url.hostname (the CONNECT TCP target at @@ -3735,62 +3693,38 @@ impl<'a> HTTPClient<'a> { // they're distinct values when a Host header override is set. Self::ssl_ctx_mut(ctx).release_socket( socket, - self.flags.did_have_handshaking_error && !self.flags.reject_unauthorized, - self.flags.reject_unauthorized, - self.connected_url.hostname, - self.connected_url.get_port_auto(), - self.tls_props.as_ref(), + this.flags.did_have_handshaking_error && !this.flags.reject_unauthorized, + this.flags.reject_unauthorized, + this.connected_url.hostname, + this.connected_url.get_port_auto(), + this.tls_props.as_ref(), tunnel, - if had_tunnel { self.url.hostname } else { b"" }, + if had_tunnel { this.url.hostname } else { b"" }, if had_tunnel { - self.url.get_port_auto() + this.url.get_port_auto() } else { 0 }, - if had_tunnel || (IS_SSL && self.http_proxy.is_none()) { + if had_tunnel || (IS_SSL && this.http_proxy.is_none()) { // Direct TLS: the handshake verified the peer against // the Host-header override (get_tls_hostname), so the // override hash must be part of the pool key. Matches // the lookup in HTTPContext::connect. - self.proxy_auth_hash() + this.proxy_auth_hash() } else { 0 }, None, ); } else { - if self.proxy_tunnel.is_some() { + if this.proxy_tunnel.is_some() { bun_core::scoped_log!(fetch, "close the tunnel"); - self.close_proxy_tunnel(true); + this.close_proxy_tunnel(true); } GenHttpContext::::close_socket(socket); } - - self.state.reset(); - self.state.response_stage = ResponseStage::Done; - self.state.request_stage = RequestStage::Done; - self.state.stage = Stage::Done; - self.flags.proxy_tunneling = false; bun_core::scoped_log!(fetch, "done"); - } - - // Restore the body bytes that `state.reset()` cleared. - body_out::restore_list(body, body_snapshot); - let async_http = self.parent_async_http(); - // Rebuild the result from snapshotted fields now that all `&mut self` - // mutations are finished — no aliased borrows remain. - let result = HTTPClientResult { - body: body_out::opt_mut(body), - has_more, - redirected, - can_stream, - is_http2, - fail, - metadata, - body_size, - certificate_info, - }; - callback.run(async_http, result); + }); if PRINT_EVERY != 0 { let i = PRINT_EVERY_I.fetch_add(1, Ordering::Relaxed) + 1; @@ -3808,14 +3742,26 @@ impl<'a> HTTPClient<'a> { /// transport, so there is no `ctx`/`socket` to hand back to the pool here. fn send_progress_update_multiplexed(&mut self) { debug_assert!(self.flags.protocol != Protocol::Http1_1); - // reshaped for borrowck — `to_result()` ties `result`'s - // lifetime to `&mut self`, so holding it across the `is_done` mutations - // would require a second live `&mut Self` (aliased UB). Instead snapshot - // every owned/Copy field out of the result, drop it, mutate `self` - // directly, then rebuild a fresh `HTTPClientResult` for the callback. - // See send_progress_update_without_stage_check for the same pattern. + self.send_progress_update_inner(|_| {}); + } + + /// Shared tail of the two progress-update paths: snapshot the result, + /// run `release_transport` once the response is done (the HTTP/1.1 path + /// hands its socket back to the pool there), reset state, and deliver the + /// result to the callback. + fn send_progress_update_inner(&mut self, release_transport: impl FnOnce(&mut Self)) { + // reshaped for borrowck — `to_result()` returns an + // `HTTPClientResult<'_>` whose lifetime is tied to `&mut self` (via the + // `body: &mut MutableString` borrow). Holding that result across the + // `is_done` mutations below would require a second live `&mut Self`, + // which PORTING.md §Forbidden flags as aliased `&mut`. Instead: + // snapshot every owned/Copy field out of the result, drop it, mutate + // `self` directly, then rebuild a fresh `HTTPClientResult` for the + // callback from the snapshotted fields + the restored body. let body = self.state.body_out_str; - // Snapshot the body buffer's CONTENTS by value; restored below. + // Snapshot the body buffer's CONTENTS by value so that `state.reset()` + // — which calls `body.reset()` and clears the list — doesn't deliver + // an empty body when `is_done`. Restored below before the callback. let body_snapshot = body_out::take_list(body); let callback = self.result_callback; @@ -3842,15 +3788,19 @@ impl<'a> HTTPClient<'a> { ) }; // r (and its &mut borrow of self) dropped here let is_done = !has_more; + bun_core::scoped_log!(fetch, "progressUpdate {}", is_done); + if is_done { self.unregister_abort_tracker(); + release_transport(self); self.state.reset(); self.state.response_stage = ResponseStage::Done; self.state.request_stage = RequestStage::Done; self.state.stage = Stage::Done; self.flags.proxy_tunneling = false; } + // Restore the body bytes that `state.reset()` cleared. body_out::restore_list(body, body_snapshot); let async_http = self.parent_async_http(); @@ -4364,6 +4314,44 @@ impl<'a> HTTPClient<'a> { } } + /// Shared tail of the `Location`-header arms in + /// `handle_response_metadata`: parse the rebuilt absolute href, compare + /// origins against the current URL, then swap the href into + /// `self.redirect`. Returns whether the redirect target is same-origin. + fn apply_redirect_url(&mut self, new_href: Vec) -> bool { + // SAFETY: self-borrow — `new_href` is moved into `self.redirect` + // below, which lives as long as `self` (≥ `'a`). + let new_url: URL<'a> = unsafe { URL::parse(&new_href).erase_lifetime() }; + let is_same_origin = strings::eql_case_insensitive_ascii( + strings::without_trailing_slash(new_url.origin), + strings::without_trailing_slash(self.url.origin), + true, + ); + self.url = new_url; + // connected_url still borrows from the previous hop's buffer until + // doRedirect releases the socket, so park it in prev_redirect for + // doRedirect to free instead of leaking it. + debug_assert!(self.prev_redirect.is_empty()); + self.prev_redirect = core::mem::replace(&mut self.redirect, new_href); + is_same_origin + } + + /// Normalize a fully-rebuilt redirect URL through the WHATWG parser and + /// apply it via [`Self::apply_redirect_url`]. + fn normalize_and_apply_redirect_url( + &mut self, + mut string_builder: StringBuilder, + ) -> Result { + debug_assert!(string_builder.cap == string_builder.len); + let input = BunString::borrow_utf8(string_builder.allocated_slice()); + let normalized_url = OwnedString::new(bun_url::href_from_string(&input)); + if normalized_url.tag() == BunStringTag::Dead { + // URL__getHref failed, dont pass dead tagged string to toOwnedSlice. + return Err(err!(RedirectURLInvalid)); + } + Ok(self.apply_redirect_url(normalized_url.to_owned_slice())) + } + pub fn handle_response_metadata( &mut self, response: &mut picohttp::Response, @@ -4658,36 +4646,8 @@ impl<'a> HTTPClient<'a> { let _ = string_builder.append(location); - if cfg!(debug_assertions) { - debug_assert!(string_builder.cap == string_builder.len); - } - - let input = - BunString::borrow_utf8(string_builder.allocated_slice()); - let normalized_url = - OwnedString::new(bun_url::href_from_string(&input)); - if normalized_url.tag() == BunStringTag::Dead { - // URL__getHref failed, dont pass dead tagged string to toOwnedSlice. - return Err(err!(RedirectURLInvalid)); - } - let normalized_url_str = normalized_url.to_owned_slice(); - - // SAFETY: self-borrow — `normalized_url_str` is moved into - // `self.redirect` below, which lives as long as `self` (≥ `'a`). - let new_url: URL<'a> = - unsafe { URL::parse(&normalized_url_str).erase_lifetime() }; - is_same_origin = strings::eql_case_insensitive_ascii( - strings::without_trailing_slash(new_url.origin), - strings::without_trailing_slash(self.url.origin), - true, - ); - self.url = new_url; - // connected_url still borrows from the previous hop's buffer - // until doRedirect releases the socket, so park it in - // prev_redirect for doRedirect to free instead of leaking it. - debug_assert!(self.prev_redirect.is_empty()); - self.prev_redirect = - core::mem::replace(&mut self.redirect, normalized_url_str); + is_same_origin = + self.normalize_and_apply_redirect_url(string_builder)?; } else if location.starts_with(b"//") { let mut string_builder = StringBuilder::default(); @@ -4719,36 +4679,10 @@ impl<'a> HTTPClient<'a> { let _ = string_builder.append(location); - if cfg!(debug_assertions) { - debug_assert!(string_builder.cap == string_builder.len); - } - - let input = - BunString::borrow_utf8(string_builder.allocated_slice()); - let normalized_url = - OwnedString::new(bun_url::href_from_string(&input)); - if normalized_url.tag() == BunStringTag::Dead { - return Err(err!(RedirectURLInvalid)); - } - let normalized_url_str = normalized_url.to_owned_slice(); - - // SAFETY: self-borrow — `normalized_url_str` is moved into - // `self.redirect` below, which lives as long as `self` (≥ `'a`). - let new_url: URL<'a> = - unsafe { URL::parse(&normalized_url_str).erase_lifetime() }; - is_same_origin = strings::eql_case_insensitive_ascii( - strings::without_trailing_slash(new_url.origin), - strings::without_trailing_slash(self.url.origin), - true, - ); - self.url = new_url; - debug_assert!(self.prev_redirect.is_empty()); - self.prev_redirect = - core::mem::replace(&mut self.redirect, normalized_url_str); + is_same_origin = + self.normalize_and_apply_redirect_url(string_builder)?; } else { - let original_url = self.url.clone(); - - let base = BunString::borrow_utf8(original_url.href); + let base = BunString::borrow_utf8(self.url.href); let rel = BunString::borrow_utf8(location); let new_url_ = OwnedString::new(bun_url::join(&base, &rel)); @@ -4756,18 +4690,7 @@ impl<'a> HTTPClient<'a> { return Err(err!(InvalidRedirectURL)); } - let new_url = new_url_.to_owned_slice(); - // SAFETY: self-borrow — `new_url` is moved into `self.redirect` - // below, which lives as long as `self` (≥ `'a`). - self.url = unsafe { URL::parse(&new_url).erase_lifetime() }; - is_same_origin = strings::eql_case_insensitive_ascii( - strings::without_trailing_slash(self.url.origin), - strings::without_trailing_slash(original_url.origin), - true, - ); - debug_assert!(self.prev_redirect.is_empty()); - self.prev_redirect = - core::mem::replace(&mut self.redirect, new_url); + is_same_origin = self.apply_redirect_url(new_url_.to_owned_slice()); } } diff --git a/src/http_jsc/websocket_client.rs b/src/http_jsc/websocket_client.rs index 23aa399797a..a1d6ecdaea2 100644 --- a/src/http_jsc/websocket_client.rs +++ b/src/http_jsc/websocket_client.rs @@ -1717,16 +1717,19 @@ impl WebSocket { this.send_close_with_body(code, None, None, 0); } - pub extern "C" fn init( - outgoing: *mut CppWebSocket, - input_socket: *mut c_void, + /// Allocate a `WebSocket` with `ref_count == 1` and initialize deflate + /// if requested. The initial ref is the I/O-layer ref: the adopted-socket + /// ref in `init` (released by `handle_close`) or the tunnel-connection + /// ref in `init_with_tunnel` (released in `clear_data` when + /// `proxy_tunnel` is detached). The C++ ref paired with + /// `m_connectedWebSocket` is taken later in `finish_init`. + fn new_ws( global_this: &JSGlobalObject, - buffered_data: *mut u8, - buffered_data_len: usize, + outgoing: *mut CppWebSocket, + secure: Option<*mut SslCtx>, + proxy_tunnel: Option>, deflate_params: Option<&websocket_deflate::Params>, - secure_ptr: *mut c_void, - ) -> *mut c_void { - let tcp = input_socket.cast::(); + ) -> *mut Self { // outlives this call. let vm = global_this.bun_vm().as_mut(); let ws = bun_core::heap::into_raw(Box::new(WebSocket:: { @@ -1755,30 +1758,100 @@ impl WebSocket { initial_data_handler: None, // reshaped for borrowck — `vm.event_loop()` returns a // `&'static`-tied borrow that would lock `vm` for the rest of the - // fn; re-derive from `global_this` so `vm` stays usable below. + // fn; re-derive from `global_this` so `vm` stays usable for the + // deflate init below. // SAFETY: bun_vm() never returns null; event_loop ptr is live for VM lifetime. event_loop: global_this.bun_vm().event_loop_mut(), deflate: None, receiving_compressed: false, message_is_compressed: false, - secure: if secure_ptr.is_null() { - None - } else { - Some(secure_ptr.cast::()) - }, - proxy_tunnel: None, + secure, + proxy_tunnel, })); bun_core::scoped_log!(alloc, "new({}) = {:p}", Self::ALLOC_TYPE_NAME, ws); - // SAFETY: ws was just allocated via heap::alloc - let ws_ref = unsafe { &mut *ws }; if let Some(params) = deflate_params { - match WebSocketDeflate::init(*params, vm.rare_data()) { - Ok(deflate) => ws_ref.deflate = Some(deflate), - Err(_) => ws_ref.deflate = None, - } + // SAFETY: ws was just allocated via heap::alloc + unsafe { (*ws).deflate = WebSocketDeflate::init(*params, vm.rare_data()).ok() }; + } + + ws + } + + /// Shared tail of `init` / `init_with_tunnel`: preallocate the frame + /// buffers, ref the event loop, queue any buffered handshake data as a + /// microtask, and take the C++-side ref. + /// + /// # Safety + /// `ws` must be the live `heap::alloc` allocation returned by `new_ws`, + /// with no other `&`/`&mut` borrow of `*ws` live across this call. If + /// `buffered_data_len > 0`, `buffered_data` must be a mimalloc allocation + /// of that length whose ownership transfers to this call (extern-C + /// contract with the upgrade client). + unsafe fn finish_init( + ws: *mut Self, + global_this: &JSGlobalObject, + buffered: Option>, + ) -> *mut c_void { + // SAFETY: caller contract — `ws` is live with no other borrows. + let ws_ref = unsafe { &mut *ws }; + bun_core::handle_oom(ws_ref.send_buffer.ensure_total_capacity(2048)); + bun_core::handle_oom(ws_ref.receive_buffer.ensure_total_capacity(2048)); + ws_ref.poll_ref.r#ref(Self::vm_loop_ctx(global_this)); + + if let Some(buffered_slice) = buffered { + let initial_data = bun_core::heap::into_raw(Box::new(InitialDataHandler:: { + adopted: NonNull::new(ws), + slice: buffered_slice, + // We need to ref the outgoing websocket so that it doesn't get + // finalized before the initial data handler is called. + // SAFETY: `outgoing_websocket` (set by `new_ws` from the + // extern-C `outgoing` argument) is a valid CppWebSocket*; it + // outlives the handler — `handle_without_deinit` drops the + // ref before C++ can finalize. + ws: ws_ref + .outgoing_websocket + .map(|p| unsafe { CppWebSocketRef::new(p) }), + })); + // Backref so `handle_data` can drain the buffered slice ahead of + // fresh socket data, and so `deinit()` can detach from the box if + // teardown races ahead of the microtask drain. + ws_ref.initial_data_handler = NonNull::new(initial_data); + + // Use a higher-priority callback for the initial onData handler + // `queue_microtask_callback` takes an erased + // `(*mut c_void, unsafe extern "C" fn(*mut c_void))`; cast both. + global_this.queue_microtask_callback( + initial_data.cast::(), + InitialDataHandler::::handle, + ); } + // And lastly, ref the new websocket since C++ has a reference to it + ws_ref.ref_(); + + ws.cast::() + } + + pub extern "C" fn init( + outgoing: *mut CppWebSocket, + input_socket: *mut c_void, + global_this: &JSGlobalObject, + buffered_data: *mut u8, + buffered_data_len: usize, + deflate_params: Option<&websocket_deflate::Params>, + secure_ptr: *mut c_void, + ) -> *mut c_void { + let tcp = input_socket.cast::(); + let secure = if secure_ptr.is_null() { + None + } else { + Some(secure_ptr.cast::()) + }; + let ws = Self::new_ws(global_this, outgoing, secure, None, deflate_params); + // outlives this call. + let vm = global_this.bun_vm().as_mut(); + // `adopt_group` takes a closure to write the new socket. let group = { // reshaped for borrowck — `rare_data()` borrows `vm` @@ -1804,8 +1877,8 @@ impl WebSocket { }, ws, // SAFETY: `owner == ws` is a valid live allocation; raw-ptr field - // write avoids materializing a second `&mut` that would alias - // `ws_ref` above. + // write avoids materializing a `&mut WebSocket` around the + // callback. |owner, sock| unsafe { core::ptr::addr_of_mut!((*owner).tcp).write(sock) }, ) { // SAFETY: `ws` is the `heap::alloc` allocation just created @@ -1814,50 +1887,24 @@ impl WebSocket { return core::ptr::null_mut(); } - bun_core::handle_oom(ws_ref.send_buffer.ensure_total_capacity(2048)); - bun_core::handle_oom(ws_ref.receive_buffer.ensure_total_capacity(2048)); - ws_ref.poll_ref.r#ref(Self::vm_loop_ctx(global_this)); - - if buffered_data_len > 0 { + let buffered: Option> = if buffered_data_len > 0 { // SAFETY: buffered_data/len from C++; caller guarantees validity. // The upgrade client allocated this buffer via mimalloc // and transfers ownership to us. // The global allocator is also mimalloc, so `heap::take` // adopts the original allocation (no copy) and `Drop` will `mi_free` it. - let buffered_slice: Box<[u8]> = unsafe { + Some(unsafe { bun_core::heap::take(std::ptr::slice_from_raw_parts_mut( buffered_data, buffered_data_len, )) - }; - let initial_data = bun_core::heap::into_raw(Box::new(InitialDataHandler:: { - adopted: NonNull::new(ws), - slice: buffered_slice, - // We need to ref the outgoing websocket so that it doesn't get - // finalized before the initial data handler is called. - // SAFETY: outgoing is a valid CppWebSocket* (extern-C contract); - // it outlives the handler — `handle_without_deinit` drops the - // ref before C++ can finalize. - ws: NonNull::new(outgoing).map(|p| unsafe { CppWebSocketRef::new(p) }), - })); - // Backref so `handle_data` can drain the buffered slice ahead of - // fresh socket data, and so `deinit()` can detach from the box if - // teardown races ahead of the microtask drain. - ws_ref.initial_data_handler = NonNull::new(initial_data); - - // Use a higher-priority callback for the initial onData handler - // `queue_microtask_callback` takes an erased - // `(*mut c_void, unsafe extern "C" fn(*mut c_void))`; cast both. - global_this.queue_microtask_callback( - initial_data.cast::(), - InitialDataHandler::::handle, - ); - } - - // And lastly, ref the new websocket since C++ has a reference to it - ws_ref.ref_(); - - ws.cast::() + }) + } else { + None + }; + // SAFETY: `ws` is the live allocation created above with no other + // borrows. + unsafe { Self::finish_init(ws, global_this, buffered) } } /// Initialize a WebSocket client that uses a proxy tunnel for I/O. @@ -1882,92 +1929,34 @@ impl WebSocket { NonNull::new(p).expect("extern-C contract: tunnel_ptr is non-null") }; - // ref_count starts at 1: this is the I/O-layer ref, owned by the - // tunnel connection (analogous to the adopted-socket ref in init() - // that handle_close() releases). It is released in clear_data() when - // proxy_tunnel is detached. The ws.ref() below adds the C++ ref - // paired with m_connectedWebSocket. - // outlives this call. - let vm = global_this.bun_vm().as_mut(); - let ws = bun_core::heap::into_raw(Box::new(WebSocket:: { - ref_count: Cell::new(1), - tcp: Socket::::detached(), // No direct socket - using tunnel - outgoing_websocket: NonNull::new(outgoing), - receive_state: ReceiveState::NeedHeader, - receiving_type: Opcode::ResB, - receiving_is_final: true, - ping_frame_bytes: [0u8; 128 + 6], - ping_len: 0, - ping_received: false, - pong_received: false, - close_received: false, - close_frame_buffering: false, - receive_frame: 0, - receive_body_remain: 0, - receive_pending_chunk_len: 0, - receive_buffer: LinearFifo::>::init(), - send_buffer: LinearFifo::>::init(), - global_this: GlobalRef::from(global_this), - poll_ref: KeepAlive::init(), - header_fragment: None, - payload_length_frame_bytes: [0u8; 8], - payload_length_frame_len: 0, - initial_data_handler: None, - // reshaped for borrowck — `vm.event_loop()` returns a - // `&'static`-tied borrow that would lock `vm` for the rest of the - // fn; re-derive from `global_this` so `vm` stays usable below. - // SAFETY: bun_vm() never returns null; event_loop ptr is live for VM lifetime. - event_loop: global_this.bun_vm().event_loop_mut(), - deflate: None, - receiving_compressed: false, - message_is_compressed: false, - secure: None, - proxy_tunnel: Some(tunnel_owned), - })); - bun_core::scoped_log!(alloc, "new({}) = {:p}", Self::ALLOC_TYPE_NAME, ws); - // SAFETY: ws was just allocated via heap::alloc - let ws_ref = unsafe { &mut *ws }; - - if let Some(params) = deflate_params { - match WebSocketDeflate::init(*params, vm.rare_data()) { - Ok(deflate) => ws_ref.deflate = Some(deflate), - Err(_) => ws_ref.deflate = None, - } - } - - bun_core::handle_oom(ws_ref.send_buffer.ensure_total_capacity(2048)); - bun_core::handle_oom(ws_ref.receive_buffer.ensure_total_capacity(2048)); - ws_ref.poll_ref.r#ref(Self::vm_loop_ctx(global_this)); + // No direct socket — `tcp` stays detached and all I/O goes through + // the tunnel. + let ws = Self::new_ws( + global_this, + outgoing, + None, + Some(tunnel_owned), + deflate_params, + ); - if buffered_data_len > 0 { - // SAFETY: see `init()` — adopt the C++ mimalloc-owned buffer - // directly so it is freed (not leaked) when the handler drops. - let buffered_slice: Box<[u8]> = unsafe { + let buffered: Option> = if buffered_data_len > 0 { + // SAFETY: buffered_data/len from C++; caller guarantees validity. + // The upgrade client allocated this buffer via mimalloc + // and transfers ownership to us. + // The global allocator is also mimalloc, so `heap::take` + // adopts the original allocation (no copy) and `Drop` will `mi_free` it. + Some(unsafe { bun_core::heap::take(std::ptr::slice_from_raw_parts_mut( buffered_data, buffered_data_len, )) - }; - let initial_data = bun_core::heap::into_raw(Box::new(InitialDataHandler:: { - adopted: NonNull::new(ws), - slice: buffered_slice, - // SAFETY: outgoing is a valid CppWebSocket* (extern-C contract); - // it outlives the handler — `handle_without_deinit` drops the - // ref before C++ can finalize. - ws: NonNull::new(outgoing).map(|p| unsafe { CppWebSocketRef::new(p) }), - })); - ws_ref.initial_data_handler = NonNull::new(initial_data); - // `queue_microtask_callback` takes an erased - // `(*mut c_void, unsafe extern "C" fn(*mut c_void))`; cast both. - global_this.queue_microtask_callback( - initial_data.cast::(), - InitialDataHandler::::handle, - ); - } - - ws_ref.ref_(); - - ws.cast::() + }) + } else { + None + }; + // SAFETY: `ws` is the live allocation created above with no other + // borrows. + unsafe { Self::finish_init(ws, global_this, buffered) } } /// Handle data received from the proxy tunnel (already decrypted). diff --git a/src/http_types/h2.rs b/src/http_types/h2.rs index 47b3162f084..2b7c50dd978 100644 --- a/src/http_types/h2.rs +++ b/src/http_types/h2.rs @@ -336,3 +336,41 @@ impl Default for FullSettingsPayload { impl FullSettingsPayload { pub(crate) const BYTE_SIZE: usize = 42; } + +// ─── field validation (RFC 9113 §8.2.1) ───────── + +/// RFC 9110 §5.6.2 `tchar`, restricted to lowercase: RFC 9113 §8.2.1 requires +/// HTTP/2 field names to be lowercase, so uppercase tchars are rejected (or +/// normalized) by callers rather than accepted here. +#[inline] +pub const fn is_lower_tchar(c: u8) -> bool { + matches!( + c, + b'a'..=b'z' + | b'0'..=b'9' + | b'!' + | b'#' + | b'$' + | b'%' + | b'&' + | b'\'' + | b'*' + | b'+' + | b'-' + | b'.' + | b'^' + | b'_' + | b'`' + | b'|' + | b'~' + ) +} + +/// RFC 9113 §8.2.1: a field value MUST NOT contain NUL (0x00), LF (0x0a), or +/// CR (0x0d). HPACK is length-prefixed so these would otherwise pass through +/// verbatim, breaking the no-CR/LF invariant the HTTP/1.1 parser provides and +/// enabling header injection when values are forwarded downstream. +#[inline] +pub fn is_malformed_field_value(value: &[u8]) -> bool { + value.iter().any(|&c| matches!(c, 0 | b'\r' | b'\n')) +} diff --git a/src/js/internal/http.ts b/src/js/internal/http.ts index ecd92d4c4f0..c464498e424 100644 --- a/src/js/internal/http.ts +++ b/src/js/internal/http.ts @@ -478,6 +478,88 @@ function filterEnvForProxies(env) { }; } +// Stub members shared by `FakeSocket` (internal/http/FakeSocket.ts) and +// `NodeHTTPServerSocket` (node/_http_server.ts). They are copied onto each +// class's prototype (instead of using a base class) so the prototype chain +// stays `Socket.prototype -> Duplex.prototype`, matching `net.Socket`. +const { constructor: _socketStubConstructor, ...socketStubDescriptors } = Object.getOwnPropertyDescriptors( + class { + declare connecting: boolean; + declare readable: boolean; + declare writable: boolean; + declare writableLength: number; + declare address: () => any; + + connect(_port, _host, _connectListener) { + return this; + } + + get bufferSize() { + return this.writableLength; + } + + get pending() { + return this.connecting; + } + + get readyState() { + if (this.connecting) return "opening"; + if (this.readable) { + return this.writable ? "open" : "readOnly"; + } else { + return this.writable ? "writeOnly" : "closed"; + } + } + + ref() { + return this; + } + + get remoteAddress() { + return this.address()?.address; + } + + set remoteAddress(val) { + // initialize the object so that other properties wouldn't be lost + this.address().address = val; + } + + get remotePort() { + return this.address()?.port; + } + + set remotePort(val) { + // initialize the object so that other properties wouldn't be lost + this.address().port = val; + } + + get remoteFamily() { + return this.address()?.family; + } + + set remoteFamily(val) { + // initialize the object so that other properties wouldn't be lost + this.address().family = val; + } + + resetAndDestroy() {} + + setKeepAlive(_enable = false, _initialDelay = 0) {} + + setNoDelay(_noDelay = true) { + return this; + } + + unref() { + return this; + } + }.prototype, +); + +function installSocketStubs(SocketClass: { prototype: object }) { + Object.defineProperties(SocketClass.prototype, socketStubDescriptors); +} + export { Headers, METHODS, @@ -507,6 +589,7 @@ export { headerStateSymbol, headersSymbol, headersTuple, + installSocketStubs, isAbortError, isTlsSymbol, kAbortController, diff --git a/src/js/internal/http/FakeSocket.ts b/src/js/internal/http/FakeSocket.ts index 3b7c9a08b76..1b7864f1468 100644 --- a/src/js/internal/http/FakeSocket.ts +++ b/src/js/internal/http/FakeSocket.ts @@ -1,4 +1,4 @@ -const { kInternalSocketData, serverSymbol } = require("internal/http"); +const { kInternalSocketData, serverSymbol, installSocketStubs } = require("internal/http"); const { kAutoDestroyed } = require("internal/shared"); const { Duplex } = require("internal/stream"); @@ -24,13 +24,6 @@ var FakeSocket = class Socket extends Duplex { (internalData = this[kInternalSocketData])?.[0]?.[serverSymbol]?.requestIP(internalData[2]) ?? {}); } - get bufferSize() { - return this.writableLength; - } - - connect(_port, _host, _connectListener) { - return this; - } _onTimeout = function () { this.emit("timeout"); }; @@ -55,60 +48,8 @@ var FakeSocket = class Socket extends Duplex { return 80; } - get pending() { - return this.connecting; - } - _read(_size) {} - get readyState() { - if (this.connecting) return "opening"; - if (this.readable) { - return this.writable ? "open" : "readOnly"; - } else { - return this.writable ? "writeOnly" : "closed"; - } - } - - ref() { - return this; - } - - get remoteAddress() { - return this.address()?.address; - } - - set remoteAddress(val) { - // initialize the object so that other properties wouldn't be lost - this.address().address = val; - } - - get remotePort() { - return this.address()?.port; - } - - set remotePort(val) { - // initialize the object so that other properties wouldn't be lost - this.address().port = val; - } - - get remoteFamily() { - return this.address()?.family; - } - - set remoteFamily(val) { - // initialize the object so that other properties wouldn't be lost - this.address().family = val; - } - - resetAndDestroy() {} - - setKeepAlive(_enable = false, _initialDelay = 0) {} - - setNoDelay(_noDelay = true) { - return this; - } - setTimeout(timeout, callback) { const socketData = this[kInternalSocketData]; if (!socketData) return; // sometimes 'this' is Socket not FakeSocket @@ -118,10 +59,6 @@ var FakeSocket = class Socket extends Duplex { return this; } - unref() { - return this; - } - _write(_chunk, _encoding, _callback) {} destroy() { @@ -130,6 +67,7 @@ var FakeSocket = class Socket extends Duplex { } }; +installSocketStubs(FakeSocket); Object.defineProperty(FakeSocket, "name", { value: "Socket" }); export default { diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index c7c54c5582c..76639e5ba4d 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -47,6 +47,7 @@ const { setServerIdleTimeout, setServerCustomOptions, getMaxHTTPHeaderSize, + installSocketStubs, } = require("internal/http"); const NumberIsNaN = Number.isNaN; @@ -963,14 +964,6 @@ const NodeHTTPServerSocket = class Socket extends Duplex { return this[kHandle]?.remoteAddress || null; } - get bufferSize() { - return this.writableLength; - } - - connect(_port, _host, _connectListener) { - return this; - } - _destroy(err, callback) { const handle = this[kHandle]; if (!handle) { @@ -1013,10 +1006,6 @@ const NodeHTTPServerSocket = class Socket extends Duplex { return this[kHandle]?.localAddress?.port; } - get pending() { - return this.connecting; - } - #resumeSocket() { const handle = this[kHandle]; const response = handle?.response; @@ -1044,54 +1033,6 @@ const NodeHTTPServerSocket = class Socket extends Duplex { this.#resumeSocket(); } - get readyState() { - if (this.connecting) return "opening"; - if (this.readable) { - return this.writable ? "open" : "readOnly"; - } else { - return this.writable ? "writeOnly" : "closed"; - } - } - - ref() { - return this; - } - - get remoteAddress() { - return this.address()?.address; - } - - set remoteAddress(val) { - // initialize the object so that other properties wouldn't be lost - this.address().address = val; - } - - get remotePort() { - return this.address()?.port; - } - - set remotePort(val) { - // initialize the object so that other properties wouldn't be lost - this.address().port = val; - } - - get remoteFamily() { - return this.address()?.family; - } - - set remoteFamily(val) { - // initialize the object so that other properties wouldn't be lost - this.address().family = val; - } - - resetAndDestroy() {} - - setKeepAlive(_enable = false, _initialDelay = 0) {} - - setNoDelay(_noDelay = true) { - return this; - } - setTimeout(_timeout, _callback) { return this; } @@ -1102,10 +1043,6 @@ const NodeHTTPServerSocket = class Socket extends Duplex { throw err; } - unref() { - return this; - } - _write(_chunk, _encoding, _callback) { const handle = this[kHandle]; // only enable writting if we can drain @@ -1229,6 +1166,7 @@ function _writeHead(statusCode, reason, obj, response) { updateHasBody(response, statusCode); } +installSocketStubs(NodeHTTPServerSocket); Object.defineProperty(NodeHTTPServerSocket, "name", { value: "Socket" }); function ServerResponse(req, options): void { diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 7e8b9c096c2..299d6d3672c 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -18,6 +18,7 @@ use bun_collections::{ByteVecExt, HashMap as BunHashMap, HiveArrayFallback, VecE use bun_core::MutableString; use bun_core::String as BunString; use bun_http::lshpack; +use bun_http_types::h2::{is_lower_tchar, is_malformed_field_value}; use bun_jsc::AbortSignal; use bun_jsc::ErrorCode as JscErrorCode; use bun_jsc::StringJsc as _; @@ -603,11 +604,6 @@ fn is_valid_request_pseudo_header(name: &[u8]) -> bool { REQUEST_PSEUDO_HEADERS.contains(name) } -#[inline] -fn is_valid_header_value(value: &[u8]) -> bool { - !value.iter().any(|&c| matches!(c, 0 | b'\n' | b'\r')) -} - #[inline] fn is_malformed_field_name(name: &[u8]) -> bool { let rest = match name.split_first() { @@ -615,34 +611,7 @@ fn is_malformed_field_name(name: &[u8]) -> bool { Some((b':', rest)) => rest, Some(_) => name, }; - rest.is_empty() - || !rest.iter().all(|&c| { - matches!( - c, - b'a'..=b'z' - | b'0'..=b'9' - | b'!' - | b'#' - | b'$' - | b'%' - | b'&' - | b'\'' - | b'*' - | b'+' - | b'-' - | b'.' - | b'^' - | b'_' - | b'`' - | b'|' - | b'~' - ) - }) -} - -#[inline] -fn is_malformed_field_value(value: &[u8]) -> bool { - value.iter().any(|&c| c == 0 || c == b'\r' || c == b'\n') + rest.is_empty() || !rest.iter().all(|&c| is_lower_tchar(c)) } bun_core::comptime_string_set! { @@ -5356,30 +5325,47 @@ impl H2FrameParser { Ok(JSValue::UNDEFINED) } - #[bun_jsc::host_fn(method)] - pub(crate) fn get_end_after_headers( - this: &Self, + /// Shared prologue for host fns that take a stream id argument: validates + /// the JS value, optionally rejects id 0 / ids above `MAX_STREAM_ID`, and + /// resolves the live `Stream` pointer in `self.streams`. `not_number_msg` + /// preserves each call site's user-visible error for a non-number argument. + #[inline] + fn stream_from_js_arg( + &self, global_object: &JSGlobalObject, - callframe: &CallFrame, - ) -> JsResult { - let args_list = callframe.arguments_old::<1>(); - if args_list.len < 1 { - return Err(global_object.throw(format_args!("Expected stream argument"))); - } - let stream_arg = args_list.ptr[0]; - + stream_arg: JSValue, + not_number_msg: &str, + ) -> JsResult<*mut Stream> { if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Invalid stream id"))); + return Err(global_object.throw(format_args!("{not_number_msg}"))); } let stream_id = stream_arg.to_u32(); - if stream_id == 0 { + if (CHECK_ZERO && stream_id == 0) || (CHECK_MAX && stream_id > MAX_STREAM_ID) { return Err(global_object.throw(format_args!("Invalid stream id"))); } - let Some(stream) = this.streams.get().get(&stream_id).copied() else { + let Some(stream) = self.streams.get().get(&stream_id).copied() else { return Err(global_object.throw(format_args!("Invalid stream id"))); }; + Ok(stream) + } + + #[bun_jsc::host_fn(method)] + pub(crate) fn get_end_after_headers( + this: &Self, + global_object: &JSGlobalObject, + callframe: &CallFrame, + ) -> JsResult { + let args_list = callframe.arguments_old::<1>(); + if args_list.len < 1 { + return Err(global_object.throw(format_args!("Expected stream argument"))); + } + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Invalid stream id", + )?; // SAFETY: stream is *mut Stream from self.streams; valid while the map entry exists Ok(JSValue::from(unsafe { (*stream).end_after_headers })) @@ -5395,20 +5381,11 @@ impl H2FrameParser { if args_list.len < 1 { return Err(global_object.throw(format_args!("Expected stream argument"))); } - let stream_arg = args_list.ptr[0]; - - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Invalid stream id", + )?; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists let stream = unsafe { &*stream }; @@ -5431,20 +5408,11 @@ impl H2FrameParser { if args_list.len < 1 { return Err(global_object.throw(format_args!("Expected stream argument"))); } - let stream_arg = args_list.ptr[0]; - - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Invalid stream id", + )?; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists let stream = unsafe { &mut *stream }; let state = JSValue::create_empty_object(global_object, 6); @@ -5494,21 +5462,13 @@ impl H2FrameParser { if args_list.len < 2 { return Err(global_object.throw(format_args!("Expected stream and options arguments"))); } - let stream_arg = args_list.ptr[0]; let options = args_list.ptr[1]; - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream_ptr) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream_ptr = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Invalid stream id", + )?; // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck let stream = unsafe { &mut *stream_ptr }; @@ -5607,21 +5567,13 @@ impl H2FrameParser { if args_list.len < 2 { return Err(global_object.throw(format_args!("Expected stream and code arguments"))); } - let stream_arg = args_list.ptr[0]; let error_arg = args_list.ptr[1]; - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 || stream_id > MAX_STREAM_ID { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Invalid stream id", + )?; if !error_arg.is_number() { return Err(global_object.throw(format_args!("Invalid ErrorCode"))); } @@ -5842,20 +5794,11 @@ impl H2FrameParser { ))); } - let stream_arg = args_list.ptr[0]; - - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Expected stream to be a number"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 || stream_id > MAX_STREAM_ID { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Expected stream to be a number", + )?; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists let stream = unsafe { &mut *stream }; @@ -5889,23 +5832,7 @@ impl H2FrameParser { any = true; continue 'begin; } - b'a'..=b'z' - | b'0'..=b'9' - | b'!' - | b'#' - | b'$' - | b'%' - | b'&' - | b'\'' - | b'*' - | b'+' - | b'-' - | b'.' - | b'^' - | b'_' - | b'`' - | b'|' - | b'~' => {} + c if is_lower_tchar(c) => {} b':' => { // only allow pseudoheaders at the beginning if i != 0 || any { @@ -5939,22 +5866,14 @@ impl H2FrameParser { ))); } - let stream_arg = args_list.ptr[0]; let headers_arg = args_list.ptr[1]; let sensitive_arg = args_list.ptr[2]; - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Expected stream to be a number"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 || stream_id > MAX_STREAM_ID { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } - - let Some(stream_ptr) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream_ptr = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Expected stream to be a number", + )?; // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck let stream = unsafe { &mut *stream_ptr }; @@ -6034,7 +5953,7 @@ impl H2FrameParser { value: &[u8], never_index: bool| -> JsResult> { - if !is_valid_header_value(value) { + if is_malformed_field_value(value) { let exception = global_object.to_type_error( bun_jsc::ErrorCode::HTTP2_INVALID_HEADER_VALUE, format_args!("Invalid value for header \"{}\"", BStr::new(validated_name)), @@ -6276,19 +6195,14 @@ impl H2FrameParser { let args = callframe.arguments_undef::<5>(); let [stream_arg, data_arg, encoding_arg, close_arg, callback_arg] = args.ptr; - if !stream_arg.is_number() { - return Err(global_object.throw(format_args!("Expected stream to be a number"))); - } - - let stream_id = stream_arg.to_u32(); - if stream_id == 0 || stream_id > MAX_STREAM_ID { - return Err(global_object.throw(format_args!("Invalid stream id"))); - } + let stream_ptr = this.stream_from_js_arg::( + global_object, + stream_arg, + "Expected stream to be a number", + )?; + // ToBoolean is side-effect free, so reading `close` after the stream + // lookup is observably identical to the previous ordering. let close = close_arg.to_boolean(); - - let Some(stream_ptr) = this.streams.get().get(&stream_id).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck let stream = unsafe { &mut *stream_ptr }; if !stream.can_send_data() { @@ -6425,14 +6339,11 @@ impl H2FrameParser { return Err(global_object.throw(format_args!("Expected stream_id argument"))); } - let stream_id_arg = args_list.ptr[0]; - if !stream_id_arg.is_number() { - return Err(global_object.throw(format_args!("Expected stream_id to be a number"))); - } - - let Some(stream) = this.streams.get().get(&stream_id_arg.to_u32()).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Expected stream_id to be a number", + )?; // SAFETY: stream is *mut Stream from self.streams; valid while the map entry exists Ok(unsafe { (*stream).js_context.get() }.unwrap_or(JSValue::UNDEFINED)) @@ -6451,13 +6362,11 @@ impl H2FrameParser { ); } - let stream_id_arg = args_list.ptr[0]; - if !stream_id_arg.is_number() { - return Err(global_object.throw(format_args!("Expected stream_id to be a number"))); - } - let Some(stream) = this.streams.get().get(&stream_id_arg.to_u32()).copied() else { - return Err(global_object.throw(format_args!("Invalid stream id"))); - }; + let stream = this.stream_from_js_arg::( + global_object, + args_list.ptr[0], + "Expected stream_id to be a number", + )?; let context_arg = args_list.ptr[1]; if !context_arg.is_object() { return Err(global_object.throw(format_args!("Expected context to be an object"))); @@ -6700,131 +6609,15 @@ impl H2FrameParser { return Err(global_object.throw_value(exception)); } - if js_value.js_type().is_array() { - bun_output::scoped_log!(H2FrameParser, "array header {}", BStr::new(name)); - let mut value_iter = js_value.array_iterator(global_object)?; - - if let Some(idx) = single_value_headers_index_of(validated_name) { - if value_iter.len > 1 || single_value_headers[idx] { - if !global_object.has_exception() { - let exception = global_object.to_type_error( - bun_jsc::ErrorCode::HTTP2_HEADER_SINGLE_VALUE, - format_args!( - "Header field \"{}\" must only have a single value", - BStr::new(validated_name) - ), - ); - return Err(global_object.throw_value(exception)); - } - return Ok(JSValue::ZERO); - } - single_value_headers[idx] = true; - } - - while let Some(item) = value_iter.next()? { - if item.is_empty_or_undefined_or_null() { - if !global_object.has_exception() { - return Err(global_object - .err( - JscErrorCode::HTTP2_INVALID_HEADER_VALUE, - format_args!( - "Invalid value for header \"{}\"", - BStr::new(validated_name) - ), - ) - .throw()); - } - return Ok(JSValue::ZERO); - } - - let value_str = match item.to_js_string(global_object) { - Ok(s) => s, - Err(_) => { - global_object.clear_exception(); - return Err(global_object - .err( - JscErrorCode::HTTP2_INVALID_HEADER_VALUE, - format_args!( - "Invalid value for header \"{}\"", - BStr::new(validated_name) - ), - ) - .throw()); - } - }; - - let never_index = - match sensitive_arg.get_truthy(global_object, validated_name)? { - Some(_) => true, - None => sensitive_arg.get_truthy(global_object, name)?.is_some(), - }; - - let value_slice = value_str.to_slice(global_object); - let value = value_slice.slice(); - if !is_valid_header_value(value) { - return Err(global_object - .err( - JscErrorCode::HTTP2_INVALID_HEADER_VALUE, - format_args!( - "Invalid value for header \"{}\"", - BStr::new(validated_name) - ), - ) - .throw()); - } - bun_output::scoped_log!( - H2FrameParser, - "encode header {} {}", - BStr::new(validated_name), - BStr::new(value) - ); - - if let Err(err) = this.encode_header_into_list( - &mut encoded_headers, - validated_name, - value, - never_index, - ) { - if err == bun_core::err!("OutOfMemory") { - return Err(global_object - .throw(format_args!("Failed to allocate header buffer"))); - } - let Some(stream) = this.handle_received_stream_id(stream_id) else { - return Ok(JSValue::js_number(-1.0)); - }; - // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists - let stream = unsafe { &mut *stream }; - if !stream_ctx_arg.is_empty_or_undefined_or_null() - && stream_ctx_arg.is_object() - { - stream.set_context(stream_ctx_arg, global_object); - } - stream.state = StreamState::CLOSED; - stream.rst_code = ErrorCode::COMPRESSION_ERROR.0; - this.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamError, - stream.get_identifier(), - JSValue::js_number(stream.rst_code as f64), - ); - return Ok(JSValue::UNDEFINED); - } - } - } else if !js_value.is_empty_or_undefined_or_null() { - bun_output::scoped_log!(H2FrameParser, "single header {}", BStr::new(name)); - if let Some(idx) = single_value_headers_index_of(validated_name) { - if single_value_headers[idx] { - let exception = global_object.to_type_error( - bun_jsc::ErrorCode::HTTP2_HEADER_SINGLE_VALUE, - format_args!( - "Header field \"{}\" must only have a single value", - BStr::new(validated_name) - ), - ); - return Err(global_object.throw_value(exception)); - } - single_value_headers[idx] = true; - } - let value_str = match js_value.to_js_string(global_object) { + // closure shared by the array and single-value arms; `err_name` + // preserves each arm's user-visible message when string coercion + // fails, and `encode_err_return` its return value on a compression + // error (both match the reference implementation) + let mut encode_value = |item: JSValue, + err_name: &[u8], + encode_err_return: JSValue| + -> JsResult> { + let value_str = match item.to_js_string(global_object) { Ok(s) => s, Err(_) => { global_object.clear_exception(); @@ -6833,7 +6626,7 @@ impl H2FrameParser { JscErrorCode::HTTP2_INVALID_HEADER_VALUE, format_args!( "Invalid value for header \"{}\"", - BStr::new(name) + BStr::new(err_name) ), ) .throw()); @@ -6848,7 +6641,7 @@ impl H2FrameParser { let value_slice = value_str.to_slice(global_object); let value = value_slice.slice(); - if !is_valid_header_value(value) { + if is_malformed_field_value(value) { return Err(global_object .err( JscErrorCode::HTTP2_INVALID_HEADER_VALUE, @@ -6877,23 +6670,87 @@ impl H2FrameParser { .throw(format_args!("Failed to allocate header buffer"))); } let Some(stream) = this.handle_received_stream_id(stream_id) else { - return Ok(JSValue::js_number(-1.0)); + return Ok(Some(JSValue::js_number(-1.0))); }; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists let stream = unsafe { &mut *stream }; - stream.state = StreamState::CLOSED; if !stream_ctx_arg.is_empty_or_undefined_or_null() && stream_ctx_arg.is_object() { stream.set_context(stream_ctx_arg, global_object); } + stream.state = StreamState::CLOSED; stream.rst_code = ErrorCode::COMPRESSION_ERROR.0; this.dispatch_with_extra( JSH2FrameParser::Gc::onStreamError, stream.get_identifier(), JSValue::js_number(stream.rst_code as f64), ); - return Ok(JSValue::js_number(stream_id as f64)); + return Ok(Some(encode_err_return)); + } + Ok(None) + }; + + if js_value.js_type().is_array() { + bun_output::scoped_log!(H2FrameParser, "array header {}", BStr::new(name)); + let mut value_iter = js_value.array_iterator(global_object)?; + + if let Some(idx) = single_value_headers_index_of(validated_name) { + if value_iter.len > 1 || single_value_headers[idx] { + if !global_object.has_exception() { + let exception = global_object.to_type_error( + bun_jsc::ErrorCode::HTTP2_HEADER_SINGLE_VALUE, + format_args!( + "Header field \"{}\" must only have a single value", + BStr::new(validated_name) + ), + ); + return Err(global_object.throw_value(exception)); + } + return Ok(JSValue::ZERO); + } + single_value_headers[idx] = true; + } + + while let Some(item) = value_iter.next()? { + if item.is_empty_or_undefined_or_null() { + if !global_object.has_exception() { + return Err(global_object + .err( + JscErrorCode::HTTP2_INVALID_HEADER_VALUE, + format_args!( + "Invalid value for header \"{}\"", + BStr::new(validated_name) + ), + ) + .throw()); + } + return Ok(JSValue::ZERO); + } + + if let Some(ret) = encode_value(item, validated_name, JSValue::UNDEFINED)? { + return Ok(ret); + } + } + } else if !js_value.is_empty_or_undefined_or_null() { + bun_output::scoped_log!(H2FrameParser, "single header {}", BStr::new(name)); + if let Some(idx) = single_value_headers_index_of(validated_name) { + if single_value_headers[idx] { + let exception = global_object.to_type_error( + bun_jsc::ErrorCode::HTTP2_HEADER_SINGLE_VALUE, + format_args!( + "Header field \"{}\" must only have a single value", + BStr::new(validated_name) + ), + ); + return Err(global_object.throw_value(exception)); + } + single_value_headers[idx] = true; + } + if let Some(ret) = + encode_value(js_value, name, JSValue::js_number(stream_id as f64))? + { + return Ok(ret); } } } From 5ceae995decb9d40719b61215d7693d78ea39023 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:12:53 +0000 Subject: [PATCH 2/4] ci: retrigger From 75452354d53793d93876202b6592aa90e1c381cb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:31:29 +0000 Subject: [PATCH 3/4] Add coverage for h2 header validation and server socket stubs --- test/js/node/http/node-http.test.ts | 44 ++++++++++++++ test/js/node/http2/node-http2.test.js | 84 +++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index 6c3b2a1d25a..3e0d0af8152 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -2034,6 +2034,50 @@ it("native server socket handle accessors return undefined for non-socket receiv expect(exitCode).toBe(0); }, 15_000); +it("server request socket exposes the net.Socket compatibility surface", async () => { + const { promise, resolve, reject } = Promise.withResolvers>(); + const server = createServer((req, res) => { + try { + const s = req.socket; + resolve({ + readyState: s.readyState, + pending: s.pending, + connecting: s.connecting, + bufferSizeEqualsWritableLength: s.bufferSize === s.writableLength, + refReturnsThis: s.ref() === s, + unrefReturnsThis: s.unref() === s, + setNoDelayReturnsThis: s.setNoDelay() === s, + remoteAddressType: typeof s.remoteAddress, + remotePortType: typeof s.remotePort, + remoteFamilyIsIP: s.remoteFamily === "IPv4" || s.remoteFamily === "IPv6", + }); + } catch (err) { + reject(err); + } finally { + res.end("ok"); + } + }); + try { + const url = await listen(server); + const res = await fetch(url); + await res.text(); + expect(await promise).toEqual({ + readyState: "open", + pending: false, + connecting: false, + bufferSizeEqualsWritableLength: true, + refReturnsThis: true, + unrefReturnsThis: true, + setNoDelayReturnsThis: true, + remoteAddressType: "string", + remotePortType: "number", + remoteFamilyIsIP: true, + }); + } finally { + server.close(); + } +}); + it("socket handle write keeps buffered data intact when encoding coercion re-enters write", async () => { // Argument conversion for the native socket write can run arbitrary JS (an encoding // object's toString). If that JS calls write() again on the same socket, both the diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 84b0ee3cd44..6d3077f9f87 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -1569,6 +1569,90 @@ it("sensitive headers should work", async () => { } }); +describe("client request header validation", () => { + let server; + let url; + let lastHeaders; + beforeAll(async () => { + server = http2.createServer(); + server.on("stream", (stream, headers) => { + lastHeaders = headers; + stream.respond({ ":status": 200 }, { endStream: true }); + }); + await new Promise(resolve => server.listen(0, resolve)); + url = `http://localhost:${server.address().port}`; + }); + afterAll(() => { + server.close(); + }); + + function requestError(headers) { + const client = http2.connect(url); + client.on("error", () => {}); + try { + client.request({ ":path": "/", ...headers }); + return null; + } catch (err) { + return err; + } finally { + client.close(); + } + } + + it("rejects a control character in a single header value", () => { + for (const bad of ["a\rb", "a\nb", "a\u0000b"]) { + const err = requestError({ "x-bad": bad }); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); + expect(err.message).toBe('Invalid value for header "x-bad"'); + } + }); + + it("rejects a control character in an array header value", () => { + const err = requestError({ "x-arr": ["good", "bad\u0000"] }); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); + expect(err.message).toBe('Invalid value for header "x-arr"'); + }); + + it("rejects an invalid character in a header name", () => { + const err = requestError({ "bad header": "v" }); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("ERR_INVALID_HTTP_TOKEN"); + }); + + it("rejects multiple values for a single-value header", () => { + const err = requestError({ "content-type": ["text/plain", "text/html"] }); + expect(err).toBeInstanceOf(TypeError); + expect(err.code).toBe("ERR_HTTP2_HEADER_SINGLE_VALUE"); + expect(err.message).toBe('Header field "content-type" must only have a single value'); + }); + + it("lowercases header names and accepts tchar names and array values", async () => { + const client = http2.connect(url); + const { promise, resolve, reject } = Promise.withResolvers(); + client.on("error", reject); + const req = client.request({ + ":path": "/", + "X-Mixed-CASE": "ok", + "x-multi": ["a", "b"], + "x-t0k3n!#$%&'*+-.^_`|~": "ok", + }); + req.on("response", resolve); + req.on("error", reject); + req.end(); + try { + const res = await promise; + expect(res[":status"]).toBe(200); + expect(lastHeaders["x-mixed-case"]).toBe("ok"); + expect(lastHeaders["x-multi"]).toBe("a, b"); + expect(lastHeaders["x-t0k3n!#$%&'*+-.^_`|~"]).toBe("ok"); + } finally { + client.close(); + } + }); +}); + it("http2 session.goaway() validates input types", async done => { const { mustCall } = createCallCheckCtx(done); const server = http2.createServer((req, res) => { From 601934fb0247e400b7a90689e57c008feb81da3e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:45:10 +0000 Subject: [PATCH 4/4] Move dedupe coverage into dedicated test files --- .../node-http-server-socket-surface.test.ts | 51 +++++++++++ test/js/node/http/node-http.test.ts | 44 --------- .../node-http2-header-validation.test.ts | 90 +++++++++++++++++++ test/js/node/http2/node-http2.test.js | 84 ----------------- 4 files changed, 141 insertions(+), 128 deletions(-) create mode 100644 test/js/node/http/node-http-server-socket-surface.test.ts create mode 100644 test/js/node/http2/node-http2-header-validation.test.ts diff --git a/test/js/node/http/node-http-server-socket-surface.test.ts b/test/js/node/http/node-http-server-socket-surface.test.ts new file mode 100644 index 00000000000..a6567110c6c --- /dev/null +++ b/test/js/node/http/node-http-server-socket-surface.test.ts @@ -0,0 +1,51 @@ +import { expect, test } from "bun:test"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; + +// The net.Socket compatibility members shared by the server socket and +// FakeSocket (installSocketStubs in src/js/internal/http.ts). Every assertion +// here also holds for a request socket under real Node. +test("server request socket exposes the net.Socket compatibility surface", async () => { + const { promise, resolve, reject } = Promise.withResolvers>(); + const server = createServer((req, res) => { + try { + const s = req.socket; + resolve({ + readyState: s.readyState, + pending: s.pending, + connecting: s.connecting, + bufferSizeEqualsWritableLength: s.bufferSize === s.writableLength, + refReturnsThis: s.ref() === s, + unrefReturnsThis: s.unref() === s, + setNoDelayReturnsThis: s.setNoDelay() === s, + remoteAddressType: typeof s.remoteAddress, + remotePortType: typeof s.remotePort, + remoteFamilyIsIP: s.remoteFamily === "IPv4" || s.remoteFamily === "IPv6", + }); + } catch (err) { + reject(err); + } finally { + res.end("ok"); + } + }); + try { + await new Promise(resolveListen => server.listen(0, "127.0.0.1", resolveListen)); + const { port } = server.address() as AddressInfo; + const res = await fetch(`http://127.0.0.1:${port}/`); + await res.text(); + expect(await promise).toEqual({ + readyState: "open", + pending: false, + connecting: false, + bufferSizeEqualsWritableLength: true, + refReturnsThis: true, + unrefReturnsThis: true, + setNoDelayReturnsThis: true, + remoteAddressType: "string", + remotePortType: "number", + remoteFamilyIsIP: true, + }); + } finally { + server.close(); + } +}); diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index 3e0d0af8152..6c3b2a1d25a 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -2034,50 +2034,6 @@ it("native server socket handle accessors return undefined for non-socket receiv expect(exitCode).toBe(0); }, 15_000); -it("server request socket exposes the net.Socket compatibility surface", async () => { - const { promise, resolve, reject } = Promise.withResolvers>(); - const server = createServer((req, res) => { - try { - const s = req.socket; - resolve({ - readyState: s.readyState, - pending: s.pending, - connecting: s.connecting, - bufferSizeEqualsWritableLength: s.bufferSize === s.writableLength, - refReturnsThis: s.ref() === s, - unrefReturnsThis: s.unref() === s, - setNoDelayReturnsThis: s.setNoDelay() === s, - remoteAddressType: typeof s.remoteAddress, - remotePortType: typeof s.remotePort, - remoteFamilyIsIP: s.remoteFamily === "IPv4" || s.remoteFamily === "IPv6", - }); - } catch (err) { - reject(err); - } finally { - res.end("ok"); - } - }); - try { - const url = await listen(server); - const res = await fetch(url); - await res.text(); - expect(await promise).toEqual({ - readyState: "open", - pending: false, - connecting: false, - bufferSizeEqualsWritableLength: true, - refReturnsThis: true, - unrefReturnsThis: true, - setNoDelayReturnsThis: true, - remoteAddressType: "string", - remotePortType: "number", - remoteFamilyIsIP: true, - }); - } finally { - server.close(); - } -}); - it("socket handle write keeps buffered data intact when encoding coercion re-enters write", async () => { // Argument conversion for the native socket write can run arbitrary JS (an encoding // object's toString). If that JS calls write() again on the same socket, both the diff --git a/test/js/node/http2/node-http2-header-validation.test.ts b/test/js/node/http2/node-http2-header-validation.test.ts new file mode 100644 index 00000000000..2c5f4b4c001 --- /dev/null +++ b/test/js/node/http2/node-http2-header-validation.test.ts @@ -0,0 +1,90 @@ +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import http2 from "node:http2"; + +// Client-side header validation in the HTTP/2 frame parser: field names must +// be lowercase tchars, field values must not contain NUL/CR/LF (RFC 9113 +// section 8.2.1), and single-value headers must not repeat. Covers both the +// single-value and array encoding paths. +describe("client request header validation", () => { + let server: http2.Http2Server; + let url: string; + let lastHeaders: http2.IncomingHttpHeaders; + beforeAll(async () => { + server = http2.createServer(); + server.on("stream", (stream, headers) => { + lastHeaders = headers; + stream.respond({ ":status": 200 }, { endStream: true }); + }); + await new Promise(resolve => server.listen(0, resolve)); + url = `http://localhost:${(server.address() as { port: number }).port}`; + }); + afterAll(() => { + server.close(); + }); + + function requestError(headers: Record) { + const client = http2.connect(url); + client.on("error", () => {}); + try { + client.request({ ":path": "/", ...headers }); + return null; + } catch (err) { + return err as Error & { code?: string }; + } finally { + client.close(); + } + } + + it("rejects a control character in a single header value", () => { + for (const bad of ["a\rb", "a\nb", "a\u0000b"]) { + const err = requestError({ "x-bad": bad }); + expect(err).toBeInstanceOf(TypeError); + expect(err!.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); + expect(err!.message).toBe('Invalid value for header "x-bad"'); + } + }); + + it("rejects a control character in an array header value", () => { + const err = requestError({ "x-arr": ["good", "bad\u0000"] }); + expect(err).toBeInstanceOf(TypeError); + expect(err!.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); + expect(err!.message).toBe('Invalid value for header "x-arr"'); + }); + + it("rejects an invalid character in a header name", () => { + const err = requestError({ "bad header": "v" }); + expect(err).toBeInstanceOf(TypeError); + expect(err!.code).toBe("ERR_INVALID_HTTP_TOKEN"); + }); + + it("rejects multiple values for a single-value header", () => { + const err = requestError({ "content-type": ["text/plain", "text/html"] }); + expect(err).toBeInstanceOf(TypeError); + expect(err!.code).toBe("ERR_HTTP2_HEADER_SINGLE_VALUE"); + expect(err!.message).toBe('Header field "content-type" must only have a single value'); + }); + + it("lowercases header names and accepts tchar names and array values", async () => { + const client = http2.connect(url); + const { promise, resolve, reject } = Promise.withResolvers(); + client.on("error", reject); + const req = client.request({ + ":path": "/", + "X-Mixed-CASE": "ok", + "x-multi": ["a", "b"], + "x-t0k3n!#$%&'*+-.^_`|~": "ok", + }); + req.on("response", resolve); + req.on("error", reject); + req.end(); + try { + const res = await promise; + expect(res[":status"]).toBe(200); + expect(lastHeaders["x-mixed-case"]).toBe("ok"); + expect(lastHeaders["x-multi"]).toBe("a, b"); + expect(lastHeaders["x-t0k3n!#$%&'*+-.^_`|~"]).toBe("ok"); + } finally { + client.close(); + } + }); +}); diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 6d3077f9f87..84b0ee3cd44 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -1569,90 +1569,6 @@ it("sensitive headers should work", async () => { } }); -describe("client request header validation", () => { - let server; - let url; - let lastHeaders; - beforeAll(async () => { - server = http2.createServer(); - server.on("stream", (stream, headers) => { - lastHeaders = headers; - stream.respond({ ":status": 200 }, { endStream: true }); - }); - await new Promise(resolve => server.listen(0, resolve)); - url = `http://localhost:${server.address().port}`; - }); - afterAll(() => { - server.close(); - }); - - function requestError(headers) { - const client = http2.connect(url); - client.on("error", () => {}); - try { - client.request({ ":path": "/", ...headers }); - return null; - } catch (err) { - return err; - } finally { - client.close(); - } - } - - it("rejects a control character in a single header value", () => { - for (const bad of ["a\rb", "a\nb", "a\u0000b"]) { - const err = requestError({ "x-bad": bad }); - expect(err).toBeInstanceOf(TypeError); - expect(err.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); - expect(err.message).toBe('Invalid value for header "x-bad"'); - } - }); - - it("rejects a control character in an array header value", () => { - const err = requestError({ "x-arr": ["good", "bad\u0000"] }); - expect(err).toBeInstanceOf(TypeError); - expect(err.code).toBe("ERR_HTTP2_INVALID_HEADER_VALUE"); - expect(err.message).toBe('Invalid value for header "x-arr"'); - }); - - it("rejects an invalid character in a header name", () => { - const err = requestError({ "bad header": "v" }); - expect(err).toBeInstanceOf(TypeError); - expect(err.code).toBe("ERR_INVALID_HTTP_TOKEN"); - }); - - it("rejects multiple values for a single-value header", () => { - const err = requestError({ "content-type": ["text/plain", "text/html"] }); - expect(err).toBeInstanceOf(TypeError); - expect(err.code).toBe("ERR_HTTP2_HEADER_SINGLE_VALUE"); - expect(err.message).toBe('Header field "content-type" must only have a single value'); - }); - - it("lowercases header names and accepts tchar names and array values", async () => { - const client = http2.connect(url); - const { promise, resolve, reject } = Promise.withResolvers(); - client.on("error", reject); - const req = client.request({ - ":path": "/", - "X-Mixed-CASE": "ok", - "x-multi": ["a", "b"], - "x-t0k3n!#$%&'*+-.^_`|~": "ok", - }); - req.on("response", resolve); - req.on("error", reject); - req.end(); - try { - const res = await promise; - expect(res[":status"]).toBe(200); - expect(lastHeaders["x-mixed-case"]).toBe("ok"); - expect(lastHeaders["x-multi"]).toBe("a, b"); - expect(lastHeaders["x-t0k3n!#$%&'*+-.^_`|~"]).toBe("ok"); - } finally { - client.close(); - } - }); -}); - it("http2 session.goaway() validates input types", async done => { const { mustCall } = createCallCheckCtx(done); const server = http2.createServer((req, res) => {