Bun.serve: reject a Request that a different server received in timeout(), requestIP() and upgrade()

[robobun]

Problem

• server.timeout(req, s), server.requestIP(req) and server.upgrade(req) act on a Request that a different Bun.serve() instance received. A.timeout(reqB, 0) clears B's idleTimeout on that connection, and for a WebSocket handshake sent to B, A.upgrade(reqB) returns true and runs A's handlers on B's connection.
• Cause: timeout() (server_body.rs:1568), request_ip() (:1533) and on_upgrade() (:1796) use request.request_context without comparing its server backref with self. on_upgrade() only matched the ssl/debug type tag.

Fix

• AnyRequestContext::server_ptr() returns the owning server. check_request_owner() compares it with self and throws TypeError (ERR_INVALID_ARG_VALUE) for a different live server. The NodeHTTPResponse arms compare response.server the same way.
• A request that already detached from its server keeps the current no-op / null / false results. The foreign case throws, because it is always a caller bug and the message names the fix: use the server that fetch(req, server) receives.
• Self-reviewed: ctx.server is the receiver's own address on every dispatch path, and no internal caller, doc example or existing test pairs a request with another server.
• Verified: test/js/bun/http/serve.test.ts (new test, fails on 1.4.3), plus the websocket-server and first_party/ws suites.

Background

• Each in-flight request has a pooled RequestContext with a server backref to the NewServer that owns the pool. A JS Request reaches it through the tagged pointer AnyRequestContext.
• server.upgrade() builds a ServerWebSocket from the receiver's websocket handlers, then moves the socket into the WebSocket context of the request's own server. With a foreign receiver, the handlers and the socket belong to different servers.

Notes

• Found by a multi-instance isolation sweep. requestIP() on a foreign request was read-only and returned a correct value before this change. It now throws too, so the three per-request methods share one contract. Code that shares one handler between two servers and closes over the wrong server variable gets the error message instead of working by accident for requestIP/timeout and failing silently (or cross-wiring) for upgrade. If that tradeoff is not wanted for requestIP, dropping its check is a one-line change. A search of the issue tracker found no report that depends on cross-server calls.
• Unix-socket servers return null from timeout()/requestIP() before they look at the request, as before.
• --hot: a reload that keeps the same server id reuses the same NewServer, so in-flight requests still match. A reload that changes hostname/port creates a new server, and the old server's in-flight requests are foreign to it.
• node:http: ws's handleUpgrade() path calls server.upgrade(nodeHttpResponse) with socket.server[kBunInternals], which is the server that created the response, so it is unaffected (first_party/ws 78/78 pass).
• The seconds > 255 clamp in timeout() is a separate item and is covered by Bun.serve: validate server.timeout() seconds argument #34958.
• Repro used for the report (bun 1.4.3, both directions, wall-clock controls): B with idleTimeout: 8 answered at 13 s after A.timeout(reqB, 0); A with idleTimeout: 30 was reset at 3 s after B.timeout(reqA, 2). A.requestIP(reqB) answered. A WebSocket client to B got A's handlers and A.pendingWebSockets became 1 while B's stayed 0.
• Suites run locally with the debug build: the requestIP|timeout|upgrade tests in serve.test.ts, websocket-server.test.ts + websocket-server-upgrade-reentrant.test.ts + websocket-upgrade-signal-gc.test.ts, first_party/ws/ws.test.ts + ws-upgrade-events.test.ts, serve-http2-lifecycle.test.ts, bun-serve-routes.test.ts, bun-server.test.ts, bun-types.test.ts.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 13 days. After that, they cost $0.25 per reviewed file.}

Or wait 17 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 196307ad-d7b5-4aaf-b3ed-04447f431a30

📥 Commits

Reviewing files that changed from the base of the PR and between b52d3e5 and b0fb823.

📒 Files selected for processing (5)

• docs/runtime/http/server.mdx
• packages/bun-types/serve.d.ts
• src/runtime/server/AnyRequestContext.rs
• src/runtime/server/server_body.rs
• test/js/bun/http/serve.test.ts

---

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

[robobun]

^{Updated 6:23 PM PT - Sep 7th, 2026}

@robobun, your commit b0fb823 is building: #112412

[robobun]

Closing: this is covered by #41811, which makes the same change (an ownership check in timeout(), requestIP() and upgrade() through the request context's server backref). I left a note there with the docs/types additions from this branch.

[claude]

I reviewed this PR and didn't find any bugs. It changes user-facing API behavior — requestIP()/timeout()/upgrade() now throw where they previously no-op'd or (for requestIP) returned a correct value — and touches a CODEOWNER-gated .d.ts, so a maintainer should sign off on the tradeoff the description already flags.

What was reviewed:

• check_request_owner pointer comparison: core::ptr::from_ref(self).cast() vs the type-erased server_ptr()/AnyServer.ptr — same address space, null keeps the detached no-op.
• All five call sites (Request + NodeHTTPResponse arms of timeout/upgrade, plus request_ip); unix-socket early returns still precede the check.
• Error routed through jsc::ErrorCode::INVALID_ARG_VALUE; message names the fix.
• New test: port: 0, using, withResolvers with onerror/onclose wired to reject, one .toEqual on the combined result, asserts the upgrade landed on the right server.

Extended reasoning...

Overview

The PR adds an ownership check to Bun.serve's per-request methods so that calling server.timeout(), server.requestIP(), or server.upgrade() with a Request/NodeHTTPResponse that a different Bun.serve() instance dispatched throws a TypeError (ERR_INVALID_ARG_VALUE). It touches AnyRequestContext.rs (new server_ptr()), server_body.rs (new check_request_owner() wired into five call sites), serve.d.ts (@ throws JSDoc), the server docs page, and adds one concurrent test to serve.test.ts.

Security risks

None identified. The change tightens validation rather than relaxing it. The pointer comparison is identity-only (no dereference of the foreign pointer), and the null case falls through to the pre-existing no-op path, so a detached/pooled context can't be misidentified. There is no new user-controlled data reaching a native boundary.

Level of scrutiny

Medium-high, warranting maintainer review rather than auto-approval. This is a deliberate behavior change on a public API surface: previously A.requestIP(reqB) returned a correct value and A.timeout(reqB, n) silently acted on B's connection; now both throw. REVIEW.md's "Never silently break existing users" applies — the PR argues (reasonably) that any such caller is already a bug, but the description itself offers to drop the requestIP check if the tradeoff isn't wanted, which is a design call for a maintainer. Additionally, packages/bun-types/serve.d.ts is CODEOWNER-gated.

Other factors

The implementation follows the repo's error conventions (centralized jsc::ErrorCode, remedy in the message), the pointer-provenance shape matches src/CLAUDE.md guidance (core::ptr::eq / from_ref), and the new test follows test/CLAUDE.md conventions closely (no sleeps, failure events reject the awaited promise, single combined .toEqual, it.concurrent). The test exercises the Request arm; the NodeHTTPResponse arm's throw path isn't directly asserted, though the PR reports the first_party/ws suite (which goes through that arm on the happy path) passes. No prior reviews or objections exist on the timeline.