node:http/http2 hardening: enforce h2 request pseudo-headers, never report a failed handler as success, and bound/validate the parser paths

packages/bun-uws/src/HttpParser.h

@@ -356,6 +356,13 @@ struct HttpResponseData;
 
                         // Move past comma if present
                         if (pos < value.length() && value[pos] == ',') {
+                            /* llhttp HPE_INVALID_TRANSFER_ENCODING: any list element after "chunked"
+                             * (including "chunked,", "chunked,chunked") is invalid, never framed as
+                             * chunked — https://github.com/nodejs/llhttp (src/llhttp/http.ts). */
+                            if (te.chunked) [[unlikely]] {
+                                te.invalid = true;
+                                return te;
+                            }
                             pos++;
                         }
                     }

src/js/internal/tls.ts

@@ -135,6 +135,13 @@ function secureProtocolToVersionRange(secureProtocol) {
   return null;
 }
 
+// Option-ingestion rule only (node v26.3.0 internal/tls/wrap.js:1368,1686,1762):
+// `!== false`, so undefined keeps verification on. Read-sites of the stored
+// field use plain truthiness there (:490,:845,:1220) — do not reuse this here.
+function normalizeRejectUnauthorized(value) {
+  return value !== false;
+}
+
 let NativeSecureContext;
 
 /**
@@ -182,6 +189,7 @@ export {
   VALID_TLS_ERROR_MESSAGE_TYPES,
   isValidTLSArray,
   isValidTLSItem,
+  normalizeRejectUnauthorized,
   processPfxOptions,
   secureProtocolToVersionRange,
   throwOnInvalidTLSArray,

src/js/node/net.ts

@@ -1013,7 +1013,7 @@ const ServerHandlers: SocketHandler<NetSocket> = {
         data.destroy(error);
       } else if (
         data.isServer &&
-        data._rejectUnauthorized &&
+        data._rejectUnauthorized !== false &&
         /peer did not return a certificate/.test(error?.message)
       ) {
         // Ignore server's authorization errors

src/js/node/tls.ts

@@ -10,6 +10,7 @@ const {
   tlsStringToProtocolVersion,
   secureProtocolToVersionRange,
   processPfxOptions,
+  normalizeRejectUnauthorized,
   validateSecureProtocol,
 } = require("internal/tls");
 const {
@@ -604,6 +605,13 @@ function newNativeSecureContext(options, cached = false) {
       options = { ...options, minVersion, maxVersion };
     }
   }
+  // Node treats any value other than an explicit `false` as "verify"; the native converter
+  // only accepts real booleans, so normalize the falsy-but-not-false spellings Node accepts
+  // (0, "", null) before they can throw or silently disable verification.
+  const rejectUnauthorized = options.rejectUnauthorized;
+  if (rejectUnauthorized !== undefined && typeof rejectUnauthorized !== "boolean") {
+    options = { ...options, rejectUnauthorized: normalizeRejectUnauthorized(rejectUnauthorized) };
+  }
   const ctx = (cached ? NativeSecureContext.intern : NativeSecureContext.createPrivate)(options);
   if (pfxExtraCAs) {
     for (const pem of pfxExtraCAs) ctx.addCACert(pem);

src/jsc/bindings/node/http/JSConnectionsList.cpp

@@ -89,7 +89,9 @@ JSArray* JSConnectionsList::idle(JSGlobalObject* globalObject)
     size_t i = 0;
     while (iter->next(globalObject, item)) {
         JSHTTPParser* parser = dynamicDowncast<JSHTTPParser>(item);
-        if (!parser) {
+        // A close()d parser has no impl but stays in the list (remove() after close()
+        // is a no-op, matching Node); it must be skipped, not dereferenced.
+        if (!parser || !parser->impl()) {
             continue;
         }
 
@@ -143,7 +145,9 @@ JSArray* JSConnectionsList::expired(JSGlobalObject* globalObject, uint64_t heade
     size_t i = 0;
     while (iter->next(globalObject, item)) {
         JSHTTPParser* parser = dynamicDowncast<JSHTTPParser>(item);
-        if (!parser) {
+        // A close()d parser has no impl but stays in the list (remove() after close()
+        // is a no-op, matching Node); it must be skipped, not dereferenced.
+        if (!parser || !parser->impl()) {
             continue;
         }
 

src/runtime/api/bun/h2/connection.rs

@@ -1011,7 +1011,8 @@ impl Connection {
         let cap = (self.enforced_max_header_list_size as usize).max(65536);
         if self.header_block.len().saturating_add(payload.len()) > cap {
             // nghttp2's NGHTTP2_MAX_HEADERSLEN (65536) overflow returns NGHTTP2_ERR_HEADER_COMP,
-            // which node surfaces as a session COMPRESSION_ERROR.
+            // which node surfaces as a session COMPRESSION_ERROR
+            // (test-http2-options-max-headers-exceeds-nghttp2.js).
             self.send_go_away(sink, ErrorCode::CompressionError, b"header block too large");
             return true;
         }
@@ -1066,6 +1067,11 @@ impl Connection {
         let mut saw_connect = false;
         let mut saw_host = false;
         let mut informational = false;
+        // nghttp2 check_path() flags for the RFC 9113 §8.3.1 :path validation.
+        let mut path_regular = false;
+        let mut path_asterisk = false;
+        let mut scheme_http = false;
+        let mut meth_options = false;
         let mut content_length: Option<u64> = None;
         while off < block.len() {
             match self.hpack.decode(&block[off..]) {
@@ -1106,11 +1112,14 @@ impl Connection {
                                 b"protocol" => pseudo::PROTOCOL,
                                 _ => pseudo::UNKNOWN,
                             };
-                            // 8.3.1: requests never carry :status - a server seeing it inbound is
-                            // a malformed block. (The client direction also constrains pseudo
-                            // headers, but inbound PUSH_PROMISE blocks legitimately carry request
-                            // pseudo-headers, so that check needs the push context first.)
-                            let wrong_direction = self.is_server && rest == b"status";
+                            // RFC 9113 §8.3.1/§8.3.2: :status only in response blocks, request
+                            // pseudo-headers only in request blocks. Key on `is_request` (not
+                            // is_server) — a client-received PUSH_PROMISE is a request block.
+                            let wrong_direction = if is_request {
+                                bit == pseudo::STATUS
+                            } else {
+                                bit != pseudo::STATUS && bit != pseudo::UNKNOWN
+                            };
                             // RFC 8441 §4: :protocol is only valid when SETTINGS_ENABLE_CONNECT_PROTOCOL
                             // has been enabled by this endpoint. nghttp2 (and so node) checks the
                             // submitted local value here, not the ACKed one — so a request that arrives
@@ -1120,9 +1129,9 @@ impl Connection {
                             let protocol_disabled = self.is_server
                                 && rest == b"protocol"
                                 && self.local_settings.enable_connect_protocol == 0;
-                            // nghttp2 (check_pseudo_header) treats an empty pseudo-header value as
-                            // malformed, so `:path: ""` never counts as a present :path (§8.3.1:
-                            // `:path` "MUST NOT be empty" for http/https).
+                            // RFC 9113 §8.1: pseudo-headers never appear in a trailer section.
+                            // nghttp2 (check_pseudo_header) also treats an empty pseudo-header
+                            // value as malformed, so `:path: ""` never counts as a present :path.
                             if seen_regular
                                 || bit == pseudo::UNKNOWN
                                 || (seen_pseudo & bit) != 0
@@ -1137,15 +1146,43 @@ impl Connection {
                                 informational = true;
                             }
                             seen_pseudo |= bit;
-                            if rest == b"method" && value_b == b"CONNECT" {
-                                saw_connect = true;
+                            // nghttp2 http_request_on_header: per-field flags for check_path()/
+                            // nghttp2_http_on_request_headers below; CONNECT on a pushed (even)
+                            // stream is rejected up front ("we won't allow CONNECT for push").
+                            match rest {
+                                b"method" => {
+                                    if value_b == b"CONNECT" {
+                                        if push_parent != 0 {
+                                            malformed = true;
+                                        }
+                                        saw_connect = true;
+                                    }
+                                    meth_options |= value_b == b"OPTIONS";
+                                }
+                                b"path" => {
+                                    path_regular |= value_b.first() == Some(&b'/');
+                                    path_asterisk |= value_b == b"*";
+                                }
+                                b"scheme" => {
+                                    scheme_http |= value_b.eq_ignore_ascii_case(b"http")
+                                        || value_b.eq_ignore_ascii_case(b"https");
+                                }
+                                _ => {}
                             }
                         } else {
                             seen_regular = true;
                             match name_b {
                                 b"connection" | b"keep-alive" | b"proxy-connection"
                                 | b"transfer-encoding" | b"upgrade" => malformed = true,
-                                b"host" if is_request => saw_host = true,
+                                // nghttp2 http_request_on_header: in request blocks Host is checked
+                                // like :authority (empty/repeated => malformed); in a response it
+                                // is an ordinary field and node delivers it.
+                                b"host" if self.is_server || is_request => {
+                                    if value_b.is_empty() || saw_host {
+                                        malformed = true;
+                                    }
+                                    saw_host = true;
+                                }
                                 b"te" => {
                                     // RFC 9110 10.1.4: field values are case-insensitive.
                                     if !value_b.eq_ignore_ascii_case(b"trailers") {
@@ -1199,11 +1236,9 @@ impl Connection {
             sink.on_stream_reset(target, ErrorCode::StreamClosed.as_u32());
             return false;
         }
-        // RFC 9113 §8.3.1 (nghttp2_http_on_request_headers): a request block needs exactly one
-        // non-empty :method, :scheme and :path plus an :authority or Host; plain CONNECT omits
-        // :scheme/:path and carries :authority; extended CONNECT (:protocol, RFC 8441) requires
-        // :method CONNECT and :authority. Without this a block with an empty or missing :path
-        // reaches JS as a request with an empty url (no compliant peer can produce that shape).
+        // RFC 9113 §8.3.1 / nghttp2_http_on_request_headers: request block = :method+:scheme
+        // +:path + (:authority|Host); plain CONNECT omits :scheme/:path with :authority;
+        // extended CONNECT (RFC 8441) needs :method CONNECT. Applies to HEADERS & PUSH_PROMISE.
         if is_request && !rejected && !malformed {
             use pseudo::{AUTHORITY, METHOD, PATH, PROTOCOL, SCHEME};
             let extended_connect = (seen_pseudo & PROTOCOL) != 0;
@@ -1213,8 +1248,19 @@ impl Connection {
                 (seen_pseudo & (METHOD | SCHEME | PATH)) != (METHOD | SCHEME | PATH)
                     || ((seen_pseudo & AUTHORITY) == 0 && !saw_host)
                     || (extended_connect && (!saw_connect || (seen_pseudo & AUTHORITY) == 0))
+                    // nghttp2 check_path(): under http/https, :path must start with '/'
+                    // (or be '*' for OPTIONS).
+                    || (scheme_http && !(path_regular || (meth_options && path_asterisk)))
             };
-        }
+        } else if !is_trailer && !rejected && !malformed && !informational {
+            // RFC 9113 §8.3.2 (nghttp2_http_on_response_headers): a final response block must
+            // carry exactly :status and no request pseudo-header. wrong_direction above already
+            // rejected a request pseudo per-field; this catches a block with :status omitted.
+            malformed = (seen_pseudo & pseudo::STATUS) == 0;
+        }
+        // RFC 9113 §8.1.1: an inbound request's content-length must be coherent — the declared
+        // value is attached to the stream and, at END_STREAM, must equal the DATA received
+        // (plain CONNECT is exempt).
         if push_parent == 0 && self.is_server && !malformed && !rejected {
             if let Some(s) = self.streams.get_mut(&target) {
                 if !saw_connect && s.content_length.is_none() {
@@ -1229,9 +1275,9 @@ impl Connection {
             }
         }
         if malformed && !rejected {
-            // node (Http2Session::OnInvalidFrame): every locally-rejected invalid frame counts
-            // against maxSessionInvalidFrames; exceeding it tears the session down with
-            // ERR_HTTP2_TOO_MANY_INVALID_FRAMES (same post-increment comparison as node).
+            // nghttp2 session_handle_invalid_stream2 / RFC 9113 §8.4.1: malformed HEADERS or
+            // PUSH_PROMISE → RST_STREAM(PROTOCOL_ERROR) on the target id + invalid-frame count.
+            // node Http2Session::OnInvalidFrame tears down on maxSessionInvalidFrames overflow.
             let count = self.invalid_frame_count;
             self.invalid_frame_count = count.saturating_add(1);
             if count > self.max_invalid_frames {

src/runtime/api/bun/h2_frame_parser.rs

@@ -5139,6 +5139,8 @@ impl H2FrameParser {
         if global.has_exception() {
             return Some(stream);
         }
+        // The `*mut Stream` above stays live across this call into JS.
+        let _dispatch = self.enter_dispatch();
         match callback.call(
             &global,
             ctx_value,
@@ -5498,6 +5500,34 @@ impl H2FrameParser {
         });
     }
 
+    /// Free streams whose legacy lifecycle finished (queued by `free_resources`). Only runs
+    /// at a quiescent point — no JS dispatch on the stack and no in-progress receive()
+    /// borrowing the engine cell; otherwise ids stay queued for the next such point.
+    fn drain_pending_engine_stream_closes(&self) {
+        if self.dispatch_depth.get() != 0 || self.pending_engine_stream_closes.get().is_empty() {
+            return;
+        }
+        let Ok(mut engine_guard) = self.engine.try_borrow_mut() else {
+            return;
+        };
+        let Some(engine) = engine_guard.as_mut() else {
+            return;
+        };
+        self.pending_engine_stream_closes.with_mut(|v| {
+            for id in v.drain(..) {
+                engine.close_stream(id);
+                if let Some(stream) = self.streams.with_mut(|m| m.remove(&id)) {
+                    // SAFETY: sole owner just removed from the map; free_resources already ran;
+                    // dispatch-depth gate above proves no native frame still borrows it; stream
+                    // ids never repeat in a session → frees exactly once.
+                    unsafe {
+                        drop(bun_core::heap::take(stream));
+                    }
+                }
+            }
+        });
+    }
+
     /// Feed inbound bytes through the rewrite engine, buffering the unconsumed tail (design B).
     fn rewrite_read(&self, bytes: &[u8]) {
         bun_output::scoped_log!(H2FrameParser, "rewriteRead {}", bytes.len());
@@ -5560,28 +5590,11 @@ impl H2FrameParser {
                     engine.pending_local_settings_acks.push_back(w);
                 }
             });
-            // Streams whose legacy lifecycle finished since the last batch: evict the engine
-            // entry and free the legacy slot. free_resources already ran for these (it is the
-            // only producer of this queue); duplicate ids are fine — remove() yields None.
-            if self.dispatch_depth.get() == 0 {
-                self.pending_engine_stream_closes.with_mut(|v| {
-                    for id in v.drain(..) {
-                        engine.close_stream(id);
-                        if let Some(stream) = self.streams.with_mut(|m| m.remove(&id)) {
-                            // SAFETY: stream is the heap::alloc'd *mut Stream owned by the
-                            // map entry just removed; free_resources ran when it was queued,
-                            // dispatch_depth == 0 means no caller below us on the stack holds
-                            // a `&mut Stream` across anything that can run user JS (every
-                            // such site arms enter_dispatch), ids never repeat within a
-                            // session, so this frees exactly once.
-                            unsafe {
-                                drop(bun_core::heap::take(stream));
-                            }
-                        }
-                    }
-                });
-            }
         }
+        // Streams whose legacy lifecycle finished since the last batch: evict the engine entry
+        // and free the legacy slot at this quiescent point (the helper enforces the safety
+        // rules; deferred ids are also reclaimed at the next host-call boundary).
+        self.drain_pending_engine_stream_closes();
         if self.rewrite_tail.get().is_empty() {
             let feed = {
                 let mut guard = self.engine.borrow_mut();
@@ -7045,6 +7058,9 @@ impl H2FrameParser {
         callframe: &CallFrame,
     ) -> JsResult<JSValue> {
         bun_output::scoped_log!(H2FrameParser, "rstStream");
+        // Quiescent host-call boundary: reclaim deferred stream closes before this frame
+        // materializes any `*mut Stream`.
+        this.drain_pending_engine_stream_closes();
         let [stream_arg, error_arg] = callframe.arguments_as_array::<2>();
         if callframe.arguments_count() < 2 {
             return Err(global_object.throw(format_args!("Expected stream and code arguments")));
@@ -7912,6 +7928,10 @@ impl H2FrameParser {
             defer_callback_arg,
         ] = args.ptr;
 
+        // Quiescent host-call boundary: reclaim deferred stream closes before this frame
+        // materializes any `*mut Stream`.
+        this.drain_pending_engine_stream_closes();
+
         if !stream_arg.is_number() {
             return Err(global_object.throw(format_args!("Expected stream to be a number")));
         }
@@ -7956,9 +7976,18 @@ impl H2FrameParser {
             }
         };
 
-        let buffer = match StringOrBuffer::from_js_with_encoding(global_object, data_arg, encoding)?
-        {
-            Some(b) => b,
+        // send_data can re-enter JS mid-payload (batch flushes, prior writes' callbacks): pin
+        // + protect ArrayBuffer payloads so they can't be detached under the borrowed slice.
+        // Strings are immutable (zero-copy path); ThreadSafe's Drop releases the pin/protect.
+        let pin_payload = data_arg.is_cell() && data_arg.js_type().is_array_buffer_like();
+        let buffer = match StringOrBuffer::from_js_with_encoding_maybe_async(
+            global_object,
+            data_arg,
+            encoding,
+            pin_payload,
+            true,
+        )? {
+            Some(b) => bun_jsc::ThreadSafe::adopt(b),
             None => {
                 return Err(global_object.throw_invalid_argument_type_value(
                     b"write",
@@ -8347,6 +8376,8 @@ impl H2FrameParser {
         };
         let mut _count: u32 = 0;
         let mut it = StreamResumableIterator::init(this);
+        // The iterator's `*mut Stream`s stay live across the callbacks below.
+        let _dispatch = this.enter_dispatch();
         while let Some(stream) = it.next() {
             // SAFETY: stream is *mut Stream from self.streams; valid while the map entry exists
             let Some(value) = (unsafe { (*stream).js_context.get() }) else {