http: don't pool a connection when bytes trail a bodyless response

[robobun]

Repro

Raw server that appends a second, unrequested response to a bodyless one:

import net from "net"; import { once } from "events";
let connections = 0;
const injected = "HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\ninjected";
const server = net.createServer(sock => { connections++; let buf = "";
  sock.on("data", d => { buf += d; let i;
    while ((i = buf.indexOf("\r\n\r\n")) !== -1) { const path = buf.slice(0, i).split(" ")[1]; buf = buf.slice(i + 4);
      if (path === "/204") sock.write("HTTP/1.1 204 No Content\r\nConnection: keep-alive\r\n\r\n" + injected);
      else if (path === "/cl0") sock.write("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: keep-alive\r\n\r\n" + injected);
      else if (path === "/head") sock.write("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nConnection: keep-alive\r\n\r\nhello");
      else sock.write("HTTP/1.1 200 OK\r\nContent-Length: 6\r\nConnection: keep-alive\r\n\r\nlegit!"); } }); });
await once(server.listen(0, "127.0.0.1"), "listening");
const port = server.address().port, out = [];
for (const [path, init] of [["/204"], ["/legit"], ["/cl0"], ["/legit"], ["/head", { method: "HEAD" }], ["/legit"]]) {
  const r = await fetch(`http://127.0.0.1:${port}${path}`, init); await r.text(); out.push([path, r.status, connections]); }
console.log(JSON.stringify(out));

Connections column before (bun 1.4.0): 1,1,1,1,1,1, every /legit is sent on the connection that just misbehaved. After: 1,2,2,3,3,4. Same thing for a followed redirect: a streamed POST answered with 303 + Content-Length: 0 + trailing bytes had its GET hop sent on the same connection.

Cause

handle_on_data_headers (src/http/lib.rs): when handle_response_metadata returns ShouldContinue::Finished (HEAD response, 204/304, Content-Length: 0, or a redirect being followed) the remaining bytes of the packet in to_read were dropped on the floor and progress_update / do_redirect released the socket to the keep-alive pool as usual.

The rest of this class: a body longer than its Content-Length already clears allow_keepalive in handle_response_body, data arriving later on an idle pooled socket already gets the socket evicted in HTTPContext::on_data, and bytes after the terminating chunk of a chunked body are fixed in #37436. This PR covers the header-block-terminated responses. #37436 introduces an on_bytes_past_response_end helper for the Content-Length and chunked sites; whichever of the two PRs lands second should route this check through it too. The two PRs apply independently in either order (checked with git apply --check of #37436 on top of this branch).

Fix

In the Finished branch, clear state.flags.allow_keepalive when to_read is not empty, before the redirect check so do_redirect (which goes through is_keep_alive_possible as well) makes the same decision. The response is still delivered exactly as before; the only change is that the connection is closed instead of pooled.

Why closing is the right call: RFC 9112 section 6.3 lets a client discard data left over after a complete response but says it must not be processed as a separate response. We only ever see the part of that data that happened to share a packet with the header block; whatever else the server is going to send would be parsed as the reply to the next request that picks this socket out of the pool. The connection has no trustworthy message boundary any more, which is the same reasoning as the existing Content-Length overshoot rule. Node destroys the socket in this situation too (the leftover bytes fail to parse as a response or are rejected as a double response). The cost is only paid by servers that are mis-framing, and for them it is one extra connection.

HTTPContext::on_data tolerates a bare 0\r\n\r\n arriving later on an idle pooled socket (a 2022 drive-by for servers that send a chunk terminator after a bodyless chunked response). This change does not special-case that sequence when it arrives in the same packet: such a server still gets its response delivered, it just does not get connection reuse. The idle-socket tolerance itself is left as is.

Verification

New test in test/js/web/fetch/fetch.test.ts, placed just ahead of the Content-Length overshoot one (#37436 adds its test right after it). It drives 204, 304, Content-Length: 0, HEAD and a followed 303 (via a streamed POST, the request shape whose redirect hop currently reuses the pooled socket) through the server above, each once with trailing bytes and once clean, and checks that the trailing-bytes variants cost exactly one new connection for the follow-up request while the clean variants still reuse theirs. Without the fix the five trailing-bytes rows report 0 new connections; with it the whole table matches.

Also ran fetch-keepalive, fetch-redirect, fetch-connection-header, content-length, fetch-url-after-redirect and the proxy tests against the debug build; the only failures are the pre-existing ones caused by this container resolving localhost to ::1 / setting NO_PROXY, identical with the release binary.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/fetch.test.ts

[robobun]

^{Updated 3:26 AM PT - Aug 11th, 2026}

✅ @robobun, your commit 3de4401981eae448f76834b41cc6d65b7879deb0 passed in Build #92148! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37438

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

bun-37438 --bun

[robobun]

Status: reproduced on bun 1.4.0 with the raw net server in the description (204, Content-Length: 0 and HEAD responses with trailing bytes all kept the follow-up request on the same connection; a streamed POST answered with a 303 plus trailing bytes kept the redirect hop on it too). Fix plus test are in this PR; the new test fails on the unfixed debug build (the five trailing-bytes rows report 0 new connections) and passes with the fix.

[coderabbitai]

Walkthrough

Header-terminated responses with unread trailing bytes now disable connection reuse. Fetch regression tests cover bodyless responses, clean keep-alive reuse, connection replacement, and redirects.

Changes

HTTP response reuse

Layer / File(s)	Summary
Header-terminated response reuse src/http/lib.rs, test/js/web/fetch/fetch.test.ts	handle_on_data_headers disables persistence when unread bytes follow the header block while still delivering the response. Tests cover 204, 304, zero-length, HEAD, and 303 responses, including clean reuse and redirect behavior after connection replacement.

Possibly related PRs

• oven-sh/bun#36145: Both changes modify handle_on_data_headers and fetch regression tests.
• oven-sh/bun#36370: Both changes update HTTP keep-alive eligibility in src/http/lib.rs.
• oven-sh/bun#36777: Both changes update HTTP connection reuse handling.

Suggested reviewers: jarred-sumner, cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely describes the primary change: preventing connection pooling when bytes follow a bodyless response.
Description check	✅ Passed	The description explains the problem, cause, fix, affected cases, testing, and known environment limitations in sufficient detail.

---

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

[robobun]

Heads up: #37436 handles the chunked sibling of this (bytes after the terminating chunk) and moves the existing Content-Length overshoot check into a small on_bytes_past_response_end(count) helper in HTTPClient. Whichever of the two lands second can route its check through that helper; the Finished branch here would become self.on_bytes_past_response_end(to_read.len()). Both tests insert after the Content-Length overshoot test, so expect a trivial conflict in fetch.test.ts when rebasing.

[robobun]

One more input for the Finished-branch check, from review on #37436: pretend_304 (force_last_modified, set by bun install under BUN_FEATURE_FLAG_LAST_MODIFIED_PRETEND_304) rewrites a real 200 into a 304 and forces content_length to 0, so handle_response_metadata returns Finished while the server's genuine body bytes are sitting in to_read. The new check then closes that connection. That is the safe outcome (before, the socket was pooled with the rest of the body still undrained), so no behaviour change seems needed, but the comment and any debug log on that branch should not describe those bytes as a server-side framing violation.

[claude]

LGTM. The three-line guard mirrors the existing Content-Length overshoot rule at lib.rs:4539, and both earlier review points (chunked sibling → now scoped to #37436 in the description; long code comment → removed in 3de4401) are resolved.

What was reviewed:

• Confirmed to_read at the Finished branch is the post-header-block remainder (re-sliced at lib.rs:3722), so the emptiness check is the right predicate.
• Checked the pretend_304 / force_last_modified interaction — a rewritten 200→304 with real body bytes now closes instead of pooling an undrained socket, which is strictly safer.
• Test is hermetic (local net server on 127.0.0.1, port 0, sockets destroyed in finally) and asserts the full 10-row matrix including that clean variants still reuse (newConnections: 0).

Extended reasoning...

Overview

Three-line addition to handle_on_data_headers in src/http/lib.rs: when handle_response_metadata returns ShouldContinue::Finished (HEAD, 204/304, Content-Length: 0, or a followed redirect) and to_read — the slice past the parsed header block — is non-empty, clear allow_keepalive so the socket is closed rather than returned to the keep-alive pool. Placed before the is_redirect_pending check so do_redirect sees the same flag. Accompanied by a ~130-line test in fetch.test.ts driving 204/304/CL:0/HEAD/303 through a raw net server, each with and without trailing bytes, asserting exact new-connection counts.

Security risks

None introduced; this is a security-positive change. It closes a response-smuggling / framing-desync vector where a misbehaving server's trailing bytes on a bodyless response would be delivered as the reply to the next request that draws the pooled socket. The only cost is one extra connection to servers that mis-frame, which matches Node's behavior and RFC 9112 §6.3 guidance.

Level of scrutiny

Low-to-medium. The production change is three lines that copy the exact pattern already used four other places in the same file (self.state.flags.allow_keepalive = false), most directly the Content-Length overshoot check at lib.rs:4539-4543. to_read is provably the post-header remainder (re-sliced at lib.rs:3722 after each picohttp::Response::parse_parts). The test is the bulk of the diff and is well-constructed: local server, port 0, explicit 127.0.0.1 binding, cleanup in finally, and a full-table toEqual assertion that verifies both the fix (trailing-bytes rows → 1 new connection) and non-regression (clean rows → 0 new connections).

Other factors

All prior review threads are resolved: my earlier nit about the chunked-terminator sibling was answered (handled in #37436; description now cites it and drops the completeness claim), and the comment-cop flag on the explanatory paragraph was addressed by removing it in 3de4401. The robobun note about pretend_304 is informational — that path now closes instead of pooling a socket with undrained body bytes, which is the safe direction, and with the comment removed there is no misleading wording to adjust. No outstanding reviewer comments.