http: fail a tunnelled request with the error its callback found
Problem
- Through a CONNECT tunnel,
fetch()resolves with a damaged compressed body. A gzip stream cut inside a chunked body gives 50,080 of 108,000 bytes. Without the proxy it rejects withZlibError.bun installbehind a proxy uses a manifest whose gzip checksum failed. on_data(src/http/ProxyTunnel.rs) passed the error toclose_raw, which shut the TLS wrapper down. That runson_close, which reports success when a close completes the body. #23719 (first in 1.3.2) put that check ahead of the stored error.
Fix
- The six error sites in the tunnel's callbacks call
HTTPClient::close_and_failthroughfail_request, as a direct connection does. fail()detaches the tunnel before it shuts the wrapper down. Soon_closeonly reports that the inner connection ended, andshutdown_erris removed.- Verified:
test/js/bun/http/proxy-stress-errors.test.ts(30 cases, 18 fail without the fix) andtest/cli/install/bun-install.test.ts(1 case).
Background
- A CONNECT tunnel carries a TLS connection to the origin inside the proxy socket. An
SSLWrapperdecrypts it and callson_dataandon_close. - A body with neither
Content-Lengthnor chunked encoding ends when the connection closes, soon_closecompletes it. - Considered:
on_closereads the stored error first. That keeps one callback with two meanings.
Downsides
- Behind a proxy, a fetch or install that resolved with a damaged body now rejects. #28792 lists the related change in #34922 (truncated close-delimited bodies now reject) as breaking for 1.4.
- None found for requests that succeed: their paths through
on_dataandon_closeare unchanged.
Notes
Origin. #23719 (commit 24d9d642de, first released in bun-v1.3.2, for the ECONNRESET report #23717) added the branch that completes a body when the tunnel's connection closes, and put it ahead of the read of the stored error. Before it, onClose always failed the request with the stored error. I checked this in that diff and with git describe --contains. I did not run 1.3.1. I found no GitHub issue that reports this bug (20 searches, 111 distinct results, titles read).
How a related change was classified. #34922 (merged, no linked issue) made truncated compressed bodies on close-delimited responses reject, and reworked this same block of on_close. #28792 lists it as a breaking change for 1.4. This PR makes the same kind of change for an error found inside the tunnel, after 1.4.0. Whether it needs a release note is a maintainer decision.
Where it showed. The origin sends the response head, and the body in a later read. Through an HTTP proxy to an https origin, before this change (a release build without the fix):
chunked close-delimited
gzip cut 50,080 bytes rejects
gzip wrong CRC 108,000 bytes 108,000 bytes
gzip no stream 0 bytes 0 bytes
deflate cut 50,338 bytes rejects
deflate wrong Adler 108,000 bytes 108,000 bytes
deflate no stream 0 bytes 0 bytes
br cut 0 bytes rejects
br no stream 0 bytes 0 bytes
zstd cut 0 bytes rejects
zstd no stream 0 bytes 0 bytes
Each cell now rejects with ZlibError, BrotliDecompressionError or ZstdDecompressionError, which is what the same fetch gives without the proxy. Content-Length bodies and connections that drop in the middle already rejected.
Why the old path resolved. Debug log of gzip, chunked, cut: ProxyTunnel onData body_chunk, Decompression error: ZlibError, ProxyTunnel onClose tunnel exists, progressUpdate true. on_close asked is_body_complete_on_close() (true for a chunked decoder in its trailers, and for a body with no framing in stage Body), then finalize_body_on_eof(), which returned Ok because the three streaming decoders accept input once they are in their error state. When the head and the body share one read, the error is found in handle_on_data_headers, which already called close_and_fail. That is why a small response in one packet never showed the bug.
Not only decoders. Any Err from handle_response_body or handle_response_body_chunked_encoding took the same route, for example a failed allocation on a close-delimited body.
bun install. A registry manifest with Content-Encoding: gzip, chunked, with one bit of its CRC-32 flipped: through the tunnel the manifest was used and the tarball was requested. It now stops at error: ZlibError downloading package manifest bar, as it does without the proxy. The manifest in the test is 300 KB of incompressible data so that its last chunk arrives in a later read than the head.
The Socket::None arm. The old failure tail of on_close failed a tunnel with no socket only when finalize_body_on_eof had failed (#34922 added that condition). fail_request always fails it. A tunnel that is attached to a client always has its socket (start and adopt set it, and it is cleared only after the client gave the tunnel up), so I found no way to reach this arm by reading. I also counted it: a local build with a counter ran fail_request 46 times in the proxy suites listed below, and each time the tunnel had its socket. That does not prove that the arm cannot be reached.
Other designs.
- A failure latch in
InternalState, or decoders that return their error again: both stopfinalize_body_on_eoffrom succeeding after a failure. Once the error site fails the request,stageisFailandon_closeis not reached with the tunnel attached, so nothing calls it after a failure. - One response path for sockets and tunnels (the tunnel's
on_datarepeats the stage dispatch ofHTTPClient::on_data): a larger change in code that #40385 rewrites.
Related pull requests. #43169 changes one call in on_close (finalize_body_on_eof to finish_body_on_close). This diff leaves that line and the lines around it as they are. #40385 keeps shutdown_err and the same decision in on_close. The description of #42261 names the path ProxyTunnel::on_data -> close_from_callback -> fail. That path is now on_data -> fail_request -> close_and_fail -> fail, and it still ends in dispatch_result_and_reset. #34342 adds a call to close_from_callback and edits the parked-data site that this diff also edits. The one that lands second must call fail_request there.
Handshake reports during a response (#44618). The TLS wrapper can now call on_handshake while a response is in progress: when a renegotiation ends, and when a fatal TLS error has a queued reason. On main, a failure that is reported there goes through close_raw to on_close, so it can complete a close-delimited body in the same way as the bug above. With this change it fails the request, as HTTPContext::on_handshake does for a direct connection. I read this in the code and did not run it. A fatal TLS error that the wrapper reports only through on_close still completes a close-delimited body. #44618 also merged the two failure sites of on_handshake into one, so there are six error sites now.
Suites run on a debug build with ASAN, on this branch merged with main 6bb5dc1956. proxy-stress-errors (83 pass), proxy.test.ts (103), proxy-stress-lifecycle (93), proxy-stress-adversarial (151), proxy-stress-matrix (335), proxy-stress-headers (76), proxy-stress-protocol (102), proxy-stress-concurrent (43), proxy.test.js (16), fetch-proxy-connect-tunnel-split-envelope (5), fetch-proxy-tls-intern-race (1), bun-install-proxy (14), the proxy cases of bun-install.test.ts (4). On the previous merge (main ed680823cb) with main's ProxyTunnel.rs, 18 of the 30 new cases and the install case fail, and no other case of proxy-stress-errors fails. Main did not change src/http between the two merges. bun check reports no error in the lines that this PR adds. It reports 7 errors in other lines of bun-install.test.ts, which are main's.
One stall that I could not reproduce. In the first run of proxy-stress-matrix on an earlier merge of this branch with main (d7562f260d), with the change, four adjacent cases (chunked br and zstd bodies through the tunnel) took about 5 s, and two of them hit the 5 s limit. In 30 more runs, 15 with the change and 15 with main's ProxyTunnel.rs, every case passed and none of the response-matrix cases took more than 3 s. The one run on each later merge passed. I did not find the cause.
The ECONNRESET in proxy-stress-concurrent comes from the test's origin server. 32× parallel http-proxy → https-origin (both keepalive values) sometimes rejects with ECONNRESET on a slow host, with and without this change. An earlier version of these notes said that I did not find why. The cause:
- The block runs 8 cases at once. That is 256 requests, and up to 256 server-side TLS handshakes on the one JS-thread loop.
- uSockets reads at most 5 handshaking sockets per loop iteration and parks the others in a LIFO queue (
MAX_LOW_PRIO_SOCKETS_PER_LOOP_ITERATIONinpackages/bun-usockets/src/loop.c). The first handshakes to arrive are served last. - The HTTP server closes a connection that has no request 8 to 12 s after accept (
idleTimeout = 10inpackages/bun-uws/src/HttpResponseData.h, swept every 4 s). On a slow debug build the oldest handshakes stay parked that long. The origin resets them, the proxy ends the client connection, andfetchrejects.
A probe with the same workload recorded the proxy's sockets to the origin. In a failing run, 9 of them got ECONNRESET from the origin 9.6 s after they connected, 4 of them before the origin had sent one byte. Runs on the previous merge (main ed680823cb), with two builds: this branch, and this branch with main's ProxyTunnel.rs.
- Slow host (the last cases took 10 to 14 s): the probe had a failure in 4 of 13 runs with the change and in 4 of 6 without it.
- Fast host (4 to 5 s): the test block passed in 6 of 6 alternating runs on each build.
- One core, shared with two busy loops: the block failed on both builds (1 run each).
- The 8 cases one after another: 0 of 3 runs failed.
These counts do not show a difference between the two builds. The one full-file run on that tree, with the change, failed this case: 42 of 43. The one full-file run on the current tree passed: 43 of 43.
Two other cases that failed with and without this change, on an earlier base (main 71d0d43997). On a later base (116dd64fd5) both passed on both builds, in one alternating run each. Both passed on the two later merges with the change.
proxy-stress-concurrent, memory probemode=redirect ×300: it exceeds its 120 s limit. Without the change: 3 of 3. With it: 2 of 3. The tunnel's debug log is the same on both builds, and the changed code does not run.proxy.test.ts, the bad-record 101 case: it exceeds its 5 s limit. Without the change: 2 of 2. With it: 1 of 2. Its fixture alone takes 4.9 to 5.7 s on both builds.
Over the 22 cases of proxy-stress-concurrent that take more than 1.5 s, the ratio of mean durations (with the change over without it) has a median of 0.99, from 0.81 to 1.13. Three runs per side do not show that the two rates are equal.
Binary size. CI's size check reports between -64.0 KB and +0.0 KB on the 12 targets. The comparison is against a canary build of main, not the merge base. The diff removes two functions and one field of ProxyTunnel and adds one function.
no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-install.test.ts