http: fail with ConnectionClosed instead of asserting when outer proxy socket is dead at ProxyHeaders write

[robobun]

Crash signature

Panic: Internal assertion failure inside on_writable after the proxy-tunnel inner TLS handshake completes. Sentry BUN-2V7Z (~1080 lifetime, ~28/24h) and BUN-36JT (sibling). 100% Windows; affected 1.3.x back to firstSeen 2026-04-11.

Stack (bun 1.3.14):

onData                    src/http/HTTPContext.zig
onData                    src/http/http.zig
receive                   src/http/ProxyTunnel.zig
receiveData / handleTraffic / updateHandshakeState / triggerHandshakeCallback
onHandshake               src/http/ProxyTunnel.zig
onWritable                src/http/http.zig   <- assert(!socket.isShutdown()); assert(!socket.isClosed());

Cause

on_writable has two sites that asserted the outer socket was live immediately before writing the initial request:

• send_initial_request_payload (Pending/Headers/Opened arm)
• the ProxyHeaders arm (proxy tunnel: first HTTP write after the inner TLS handshake)

When fetch() goes through a CONNECT proxy to an https:// target, the inner TLS handshake runs inside an in-process SSLWrapper. ProxyTunnel.on_handshake fires from handleTraffic while draining buffered bytes delivered by a single onData call, then synchronously calls HTTPClient.on_writable. Between that call and the ProxyHeaders write, the outer socket can already be closed or shut down:

• proxy.on_writable (run first, line 2849) writes buffered encrypted bytes with socket.write() and then wrapper.flush() re-enters handleTraffic; on an HTTPS proxy, a failing outer SSL_write sets ssl_fatal_error so socket.is_shutdown() flips true, and a buffered inner close_notify fires on_close -> close_and_fail -> terminate_socket so socket.is_closed() flips true.

In release builds without debug assertions, both sites fall through to write_to_socket / ProxyTunnel::write which return 0 on a dead socket, leaving the request stuck at Headers / ProxyHeaders until the idle timeout. In builds with debug assertions enabled the assert panicked.

The Sentry cluster is 100% Windows because Windows Zig release builds used ReleaseSafe (see scripts/build/zig.ts at the affected revisions: "since Bun 1.1, Windows builds use ReleaseSafe"), which enables Environment.allow_assert. Other platforms shipped ReleaseFast where the assert compiled out. Since the Rust rewrite the assert is debug_assert!, so release builds on all platforms now stall instead of panicking; bun bd still panics.

Fix

Convert both assertion sites to real runtime checks that surface error.ConnectionClosed through the existing close_and_fail path. send_initial_request_payload already returns Result<_, Error> and its caller already close_and_fails on error; the ProxyHeaders arm calls close_and_fail directly (matching the adjacent write_request error branch).

Verification

New test in test/js/bun/http/proxy.test.ts (fixture at proxy-handshake-closed-socket-fixture.ts) runs 10 fetches through an HTTP and HTTPS CONNECT proxy that relays the backend's TLS server-handshake flight and immediately RSTs the outer connection. Every iteration rejects with ECONNRESET / ConnectionClosed (not TimeoutError, not a panic). Full proxy.test.ts (47 tests) passes.

Note on fail-before

This race is not deterministically reproducible on Linux loopback. A 100-iteration probe against an unfixed bun bd build (50 HTTP-proxy + 50 HTTPS-proxy) never tripped the assertion: on POSIX the proxy's RST is delivered to the client as a separate event loop iteration after on_writable has already returned, so socket.is_closed()/is_shutdown() is still false at the assertion site. The Windows-specific timing that lands the dead-socket state inside the synchronous on_handshake -> on_writable window (AFD poll semantics plus ReleaseSafe assertions) cannot be reconstructed without instrumenting src/. The new test guards the code path and the observable behaviour (clean rejection, no stall) but does not fail-before on Linux.

Related

• http: fix use-after-free when proxy tunnel closes after response completes mid-read #31959 fixes a UAF on the response side of the same tunnel (handle_reading after the client is freed); this PR is the request side (first write after handshake).
• Bun crash on http.zig sendInitialRequestPayload -> headerStr #23092 is a different crash site in the same function.

[robobun]

^{Updated 10:29 AM PT - Jun 17th, 2026}

✅ @robobun, your commit c0a2e3e3b05af323d5b7e4a0add28411f6d30c08 passed in Build #63149! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32462

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

bun-32462 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 115699d1-c4bd-4eb5-9b86-73645a2f4a53

📥 Commits

Reviewing files that changed from the base of the PR and between f41f875 and c0a2e3e.

📒 Files selected for processing (2)

• test/js/bun/http/proxy-handshake-closed-socket-fixture.ts
• test/js/bun/http/proxy.test.ts

---

Walkthrough

Runtime liveness checks (is_closed / is_shutdown) are added before writing buffered HTTP request bytes in two paths in src/http/lib.rs, returning ConnectionClosed immediately instead of using debug assertions. A new test fixture and regression test reproduce and verify the fix for the case where a proxy outer TCP socket is reset after the inner TLS handshake completes.

Changes

Proxy closed-socket write fix and regression test

Layer / File(s)	Summary
Runtime liveness guards in HTTP write paths src/http/lib.rs	Replaces debug-only assertions with is_closed() / is_shutdown() runtime checks in send_initial_request_payload and the RequestStage::ProxyHeaders branch of on_writable, returning ConnectionClosed immediately when the socket is dead.
Proxy-handshake closed-socket test fixture test/js/bun/http/proxy-handshake-closed-socket-fixture.ts	New fixture that spins up a non-responsive HTTPS backend and a TCP/TLS proxy that relays the inner TLS handshake bytes then forcibly destroys the outer connection; runs 20 proxied fetch iterations with an abort timeout and reports resolved/rejected/hung counts.
Regression test asserting no hang on proxy socket reset test/js/bun/http/proxy.test.ts	New test spawning the fixture for both http and https proxy schemes; asserts each fetch rejects with a connection-flavored error (ECONNRESET, ConnectionClosed, ECONNREFUSED, ConnectionRefused), that stderr contains no "hung" output, and that the subprocess exits cleanly.

Possibly related PRs

• oven-sh/bun#31325: Modifies the same HTTP request bytes-writing path in src/http/lib.rs (gating writes behind checkServerIdentity via parking/resume), directly intersecting with the write-stage liveness guards added in this PR.

Suggested reviewers

• cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: converting assertions to runtime checks that return ConnectionClosed when the outer proxy socket is dead during ProxyHeaders writes.
Description check	✅ Passed	The description includes both required sections: 'Crash signature/Cause/Fix' explains what the PR does, and 'Verification' documents how the code was tested.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/http/proxy.test.ts`:
- Line 943: The spawn configuration at line 943 specifies stderr: "pipe" but
lacks stdout: "pipe". Add stdout: "pipe" to the spawn config options alongside
stderr: "pipe" to ensure consistency with other subprocess tests in the file and
maintain reliable behavior across different Bun versions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 548a7b34-4b08-4472-a852-7327832d30b2

📥 Commits

Reviewing files that changed from the base of the PR and between 0c537fe and 9292195.

📒 Files selected for processing (3)

• src/http/lib.rs
• test/js/bun/http/proxy-handshake-closed-socket-fixture.ts
• test/js/bun/http/proxy.test.ts

[claude]

Nits from the previous pass are addressed and I found nothing further, but given the fail-before caveat (the Linux test can't deterministically reproduce the Windows race) and that this touches the HTTP client's proxy-tunnel write state machine, I'd like a maintainer to confirm the reasoning.

Extended reasoning...

Overview

This PR converts two debug_assert!(!socket.is_shutdown()/!socket.is_closed()) sites in src/http/lib.rs into runtime checks that fail the request with ConnectionClosed via the existing close_and_fail path. One site is in send_initial_request_payload (returns Err, caller already close_and_fails on error), the other is in the RequestStage::ProxyHeaders arm of on_writable (calls close_and_fail directly, matching the adjacent write_request OOM branch). A new fixture (proxy-handshake-closed-socket-fixture.ts) and two test.concurrent cases in proxy.test.ts exercise the path by RST'ing the outer proxy socket immediately after relaying the inner TLS server-handshake flight.

Security risks

None identified. The change is purely defensive — it converts a debug-build panic / release-build silent stall into a clean ConnectionClosed rejection. No new attack surface, no auth/crypto/permission logic touched. The test fixture binds to 127.0.0.1:0 and uses the harness's self-signed cert.

Level of scrutiny

Moderate-to-high. The Rust diff itself is small and mechanical (assert → guard using the established error pattern), and both hunks mirror adjacent error-handling code exactly. However:

• It sits inside the HTTP client's proxy-tunnel write state machine, which has known subtle reentrancy (the PR description walks through on_handshake → on_writable → proxy.on_writable → wrapper.flush → handleTraffic synchronously mutating socket state). A maintainer familiar with this state machine should confirm close_and_fail is safe to call from inside that synchronous chain at this point.
• The PR's own Note on fail-before is candid that the race is not reproducible on Linux loopback, so the new test guards the code path's observable behaviour (clean rejection, no stall) but does not actually fail before the fix on CI platforms. The fix's correctness therefore rests on the prose argument about Windows AFD timing + ReleaseSafe assertions, which deserves human confirmation.
• The author explicitly noted the PR was "parked for maintainer review" when declining a cosmetic suggestion.

Other factors

• All three nits I raised on the previous revision (incorrect comment about env-var caching, test.concurrent, dead postConnectClientFlights counter) were addressed in c0a2e3e and the threads are resolved.
• The bug-hunting system found no issues.
• CodeRabbit's only suggestion was cosmetic (withdrawn).
• CI shows musl build failures at f41f875; unclear whether infrastructure or related to this change, but worth a glance before merge.
• Suggested reviewer per CodeRabbit: cirospaciari (who has reviewed adjacent proxy-tunnel PRs like #31959).