node:http: make NodeHTTPResponse UpgradeCTX a three-state enum

[robobun]

What

UpgradeCTX in src/runtime/server/NodeHTTPResponse.rs (the upgrade_context: JsCell<UpgradeCTX> field of every NodeHTTPResponse) was a struct of two nullable raw pointers and three Box<[u8]> header copies. Its phases were implicit: context null meant "not an upgrade request", a non-null request meant the uws request was still readable, a null request with a non-null context meant preserve_web_socket_headers_if_needed had already copied the three sec-websocket-* headers into the boxes, and reset() (mem::take) returned it to the null state. upgrade() null-checked context once and request three more times (once per header), and createForJS stored request even when there was no upgrade context, so preserve_web_socket_headers_if_needed copied three headers for every plain request that outlived its handler even though upgrade() can never read them.

// before
pub struct UpgradeCTX {
    pub(crate) context: *mut uws_sys::WebSocketUpgradeContext,
    pub(crate) request: *mut uws_sys::Request,
    pub(crate) sec_websocket_key: Box<[u8]>,
    pub(crate) sec_websocket_protocol: Box<[u8]>,
    pub(crate) sec_websocket_extensions: Box<[u8]>,
}

// after
pub(crate) enum UpgradeCTX {
    None,
    Live { context: NonNull<WebSocketUpgradeContext>, request: NonNull<uws_sys::Request> },
    Saved { context: NonNull<WebSocketUpgradeContext>, key: Box<[u8]>, protocol: Box<[u8]>, extensions: Box<[u8]> },
}

createForJS builds Live when it is handed an upgrade context and None otherwise. preserve_web_socket_headers_if_needed is now the Live to Saved transition and a no-op in the other two states. The three reset sites (upgrade(), maybe_stop_reading_body, mark_request_as_done) store UpgradeCTX::None, which drops the saved boxes exactly as the old mem::take did, so reset() and the Default impl are deleted. upgrade() matches once at the top for the context (returning false on None, as the old null check did) and reads each header through sec_websocket_key() / sec_websocket_protocol() / sec_websocket_extensions(), which return the live request's header or the saved copy; the protocol and extensions accessors are still only called when the caller did not pass those headers explicitly, so the header lookups performed are the same as before. All users are in this one file; the extern "C" signature of NodeHTTPResponse__createForJS is unchanged. The UpgradeCTX handling contained no unsafe blocks before and contains none now.

Why

The three states the code actually goes through are now named variants, so a reader can see from the type that the uws request is only available in Live, that the header copies only exist in Saved, and that the context is present in both; the "context set but nothing to read from" and "request set but no context" combinations are unrepresentable, and the four hand-written null checks in upgrade() become matches. It is zero-cost: the largest variant is a NonNull plus three Box<[u8]> (56 bytes) and the tag brings the enum to 64 bytes, the same size as the old struct (asserted with a const assertion), and the tag test replaces the old null tests. The only work that changes is that requests without an upgrade context no longer copy three headers nobody can read, and createForJS tests the incoming context pointer for null once when choosing the variant.

Part of a series of small type-system hardening changes; each PR stands alone.

Verification

cargo check and cargo clippy are clean for the touched crates. Debug build succeeds. bun bd test test/js/node/http/node-http.test.ts (143 pass, 1 skip, 1 fail), test/js/first_party/ws/ws.test.ts (47 pass), test/js/node/http/node-http-with-ws.test.ts (2 pass), test/regression/issue/32734.test.ts (2 pass): 194 pass, 1 skip, 1 fail across 4 files.
The single failure, node-http.test.ts "request via http proxy, issue#4295" (connect ECONNREFUSED 127.0.0.1 before any response object exists), fails identically on main (143 pass, 1 skip, 1 fail) and is pre-existing/unrelated to the NodeHTTPResponse.rs UpgradeCTX change.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9e8325e8-ec9e-407a-b438-967015b38c77

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and d077474.

📒 Files selected for processing (1)

• src/runtime/server/NodeHTTPResponse.rs

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical struct-to-enum refactor confined to one file.

What was reviewed:

• Confirmed UpgradeCTX has no consumers outside NodeHTTPResponse.rs; the upgrade_context fields in RequestContext.rs/server_body.rs are a different type.
• Verified JsCell::set(UpgradeCTX::None) drops the old value in place (*slot = value), so the three reset() sites still free the Saved boxes exactly as mem::take did.
• Checked createForJS: request was already unconditionally dereferenced pre-change, so NonNull::from(request_ref) is always valid; the Live variant is only built when upgrade_ctx is non-null, and the now-skipped header copies for non-upgrade requests were provably never read (upgrade() returned early on null context).
• Traced the header-accessor borrows in upgrade(): last use is inside raw_response.upgrade(...), before the set(None) that drops the boxes — same ordering as before.

Extended reasoning...

Overview

Single-file refactor in src/runtime/server/NodeHTTPResponse.rs converting UpgradeCTX from a struct of two nullable raw pointers plus three always-present Box<[u8]> header buffers into a three-state enum (None / Live { context, request } / Saved { context, key, protocol, extensions }). The three call sites that previously invoked reset() now call self.upgrade_context.set(UpgradeCTX::None), preserve_web_socket_headers_if_needed becomes a Live → Saved transition, and upgrade() reads headers through three small accessor methods that match on the variant instead of hand-written null checks. createForJS picks Live vs None based on whether the incoming upgrade_ctx pointer is null. The extern "C" signature is unchanged.

Security risks

None. This touches WebSocket upgrade bookkeeping but does not change what is validated or how the upgrade is performed — the same header values are passed to raw_response.upgrade() on the same paths. No new unsafe blocks are introduced; the existing opaque_deref calls are consolidated into live_header() with an S008 comment. The NonNull invariant for request is upheld because request was already unconditionally dereferenced (opaque_deref(request.cast_const())) at the top of createForJS before this change.

Level of scrutiny

Medium. This is hot-path native code in the node:http server, but the change is a type-system tightening with no intended behavioral difference beyond skipping three header copies on non-upgrade requests (whose results were unreachable — upgrade() short-circuited on null context before ever reading them). I grepped for all UpgradeCTX/upgrade_context references across src/runtime/server/ and confirmed the only consumers of this type are in this file; the identically-named upgrade_context on RequestContext is a Cell<Option<*mut WebSocketUpgradeContext>> and untouched. I read JsCell::set (src/jsc/JSCell.rs:146) to confirm it assigns through *slot = value, dropping the previous enum value and thus freeing any Saved boxes — equivalent to the old drop(mem::take(self)) in reset().

Other factors

The PR description documents that node-http, ws, node-http-with-ws, and issue-32734 test suites pass on the debug build with only a pre-existing unrelated proxy failure also present on main. A const size assertion pins the enum at ≤64 bytes so the per-response footprint doesn't regress. The live_header helper ties the returned &[u8] lifetime to the borrow of the Live variant, which is a conservative bound matching when the uWS request buffer is valid. No CODEOWNERS entry covers this file. This fits the stated pattern of small, standalone type-hardening PRs and is safe to land without a human reviewer.

[robobun]

^{Updated 10:02 AM PT - Aug 11th, 2026}

✅ @robobun, your commit d07747487e13f8c641ecaf0b61b96fcb388f56cb passed in Build #92347! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37543

That installs a local version of the PR into your bun-37543 executable, so you can run:

bun-37543 --bun