The bug and how it manifests

In ClientHttp2Session.request(), this.#connections is a reference counter that tracks live streams on the session. When the counter reaches zero on a closed session, the session self-destructs. The counter is incremented inside the streamStart native callback, which fires only when this.#parser.request() successfully initiates a stream. A catch block wrapping the entire request() body unconditionally executes this.#connections-- for any exception thrown anywhere inside it, on the assumption that every exception means a previously-started stream failed. The new CONNECT validation throws violate that assumption.

The specific code path

The PR adds an else block (the plain-CONNECT branch) containing three throw statements:

} else {
  if (\!authority) throw $ERR_HTTP2_CONNECT_AUTHORITY();
  if (headers[":scheme"] \!== undefined) throw $ERR_HTTP2_CONNECT_SCHEME();
  if (headers[":path"] \!== undefined) throw $ERR_HTTP2_CONNECT_PATH();
}

These throws happen before this.#parser.request() is ever reached. The streamStart callback therefore never fires, so this.#connections is never incremented. Yet the catch block catches these throws and decrements this.#connections unconditionally, taking it from 0 to -1, then -2, and so on with each subsequent validation failure.

Why existing code does not prevent it

The session-destruction guards (in close() and the streamEnd callback) use strict equality: if (this.#connections === 0 && this.#closed). A negative value never satisfies === 0, so a session that accumulates CONNECT validation errors can never self-destroy, even after all legitimate streams have ended and the session has been explicitly closed. This same flaw exists for other pre-existing early throws (e.g., ERR_HTTP2_INVALID_STREAM, ERR_INVALID_ARG_TYPE), but those are programming-error paths unlikely to be hit in normal use.

Impact

The PR promotes this from a theoretical edge case to an ordinary user-facing scenario. Forgetting :authority on a CONNECT request is a common mistake; accidentally including :scheme or :path is equally plausible. Each such mistake silently corrupts the session's connection counter. A production client that issues several invalid CONNECT requests before closing the session will hold a leaked session handle indefinitely, accumulating file descriptors and memory.

Step-by-step proof

1. const client = http2.connect('http://127.0.0.1:' + port) — this.#connections === 0
2. client.request({ ':method': 'CONNECT' }) — enters else branch, \!authority is true → throws ERR_HTTP2_CONNECT_AUTHORITY; catch runs this.#connections-- → this.#connections === -1
3. client.request({ ':method': 'CONNECT', ':authority': 'h', ':scheme': 'https' }) → throws ERR_HTTP2_CONNECT_SCHEME; catch runs this.#connections-- → this.#connections === -2
4. client.close() — checks if (this.#connections === 0) → -2 === 0 is false → destroy() is never called
5. The session and its underlying socket remain open indefinitely.

How to fix it

The cleanest fix is to move the CONNECT pseudo-header validation above the try block so the catch handler never sees these errors. Alternatively, the catch block can guard the decrement: if (this.#connections > 0) this.#connections--. Either approach ensures the counter stays at 0 when no stream was ever started and the destruction guard can fire normally.

What the bug is and how it manifests

At line 3851, the condition distinguishing plain CONNECT from extended CONNECT (RFC 8441) is:

if (\!isConnect || headers[":protocol"] \!== undefined) {

The strict \!== undefined check treats any non-undefined value — including empty string ''" and null— as "protocol is present", routing the request through the extended-CONNECT / non-CONNECT branch. Node.js instead uses the falsy check!headers[HTTP2_HEADER_PROTOCOL]`, so empty string and null are treated as "no protocol" and the plain-CONNECT validation branch is taken.

The specific code path that triggers it

When a caller invokes client.request({ ':method': 'CONNECT', ':protocol': '' }):

1. isConnect = true, so \!isConnect = false.
2. headers[':protocol'] = '' → '' \!== undefined = true.
3. false || true = true → extended-CONNECT / non-CONNECT branch is taken.
4. Bun silently defaults :authority from the session, and sets default :scheme and :path values. No error is thrown.
5. Node.js: \!headers[HTTP2_HEADER_PROTOCOL] = \!'' = true → plain-CONNECT branch → if (\!authority) throw ERR_HTTP2_CONNECT_AUTHORITY().
6. Bun and Node.js diverge. The same applies for ':protocol': null (null \!== undefined = true in Bun; \!null = true in Node.js).

Why existing code doesn't prevent it

The PR already fixed the :authority strict-undefined check to use a falsy check (\!authority) in commit cf9998a, and the :scheme guard in the non-CONNECT branch uses \!headers[':scheme'] (falsy). The :protocol check at line 3851 is the one remaining place that uses strict-undefined equality, making it inconsistent with the pattern deliberately adopted elsewhere in this same PR.

What the impact would be

An application passing ':protocol': '' (which is not a valid RFC 8441 protocol identifier) would have Bun silently produce a malformed request rather than throw an error. The practical impact is low because empty string is never a legitimate :protocol value, but it creates a genuine Node.js compatibility divergence that would be observable in tests verifying error-path behavior.

How to fix it

Change line 3851 from:

if (\!isConnect || headers[":protocol"] \!== undefined) {

to:

if (\!isConnect || headers[":protocol"]) {

This matches Node.js's falsy check and is consistent with how the other pseudo-header checks in the same block are written.

Step-by-step proof

1. Connect a client: const client = http2.connect('http://127.0.0.1:' + port)
2. Call client.request({ ':method': 'CONNECT', ':protocol': '' }) (no :authority).
3. Bun: isConnect = true, '' \!== undefined = true → condition is true → non-CONNECT/extended-CONNECT branch → :authority defaulted from session, :scheme/:path defaults set → no error thrown.
4. Node.js: \!headers[HTTP2_HEADER_PROTOCOL] = \!'' = true → plain-CONNECT branch → \!authority (authority not provided) → throws ERR_HTTP2_CONNECT_AUTHORITY.
5. Bun silently proceeds; Node.js throws. The divergence is observable by any caller that relies on the error-path for input validation.

What the bug is

In the non-CONNECT scheme-defaulting block (the switch at lines 3863-3869), the default: case sets headers[":scheme"] = protocol where protocol is derived from url.protocol || options?.protocol || "https:". The WHATWG URL API always returns the scheme with a trailing colon (e.g. "wss:", "ftp:"). The case "https:" and case "http:" arms explicitly strip the colon, but the default arm assigns the colon-suffixed value verbatim. RFC 9113 §8.3.1 requires the :scheme pseudo-header to follow RFC 3986 §3.1 syntax, which does NOT include the trailing colon (e.g. the correct value is "wss", not "wss:").

The specific code path

let protocol: string = url.protocol || options?.protocol || "https:";
switch (protocol) {
  case "https:": headers[":scheme"] = "https"; break;
  case "http:":  headers[":scheme"] = "http";  break;
  default:       headers[":scheme"] = protocol;   // assigns e.g. "wss:" verbatim
}

Addressing the reachability refutation

One verifier correctly points out that the ClientHttp2Session constructor (lines 3686-3693) validates the session protocol and throws ERR_HTTP2_UNSUPPORTED_PROTOCOL for anything other than "http:" or "https:". Because url.protocol always returns a non-empty string for a valid WHATWG URL, the fallback to options?.protocol is never evaluated. So in practice, protocol in the request() switch can only ever be "http:" or "https:", and the default: arm is dead code today. This is accurate.

Why the bug is still worth flagging

The default arm being unreachable is a consequence of a runtime guard elsewhere, not of the code being correct. If the constructor validation were relaxed (e.g. to support WebSocket Upgrade over HTTP/2 via extended CONNECT), or if the code were copied to a context without the guard, the default arm would produce RFC-violating :scheme values. The fix is trivially simple (protocol.slice(0, -1)) and aligns the code with its own intent (which is clearly to produce a bare scheme name, as demonstrated by the explicit case arms). The PR directly touched and restructured this switch, making it an appropriate time to address the latent issue.

Pre-existing status

Git history confirms the old code was default: scheme = protocol — identical behavior. The PR preserved this pattern when moving the block inside the if (!isConnect || ...) guard. The bug predates this PR.

Step-by-step proof (if the guard is relaxed)

1. Suppose a future commit adds "wss:" to the constructor allowed-protocol list.
2. Caller: http2.connect("wss://example.com").request({ ":method": "GET" })
3. url.protocol = "wss:"; method = "GET"; isConnect = false → non-CONNECT branch taken.
4. headers[":scheme"] is undefined → scheme switch entered.
5. "wss:" matches neither "https:" nor "http:" → default: arm: headers[":scheme"] = "wss:".
6. HEADERS frame sent with :scheme: wss: (colon included) — violates RFC 9113 §8.3.1.
7. Correct value should be :scheme: wss.

Fix

Change the default: arm from:

default: headers[":scheme"] = protocol;

to:

default: headers[":scheme"] = protocol.slice(0, -1);