From 78f56ca4e6b821473fa71668ae40fc2f062e38ec Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 04:09:41 +0000 Subject: [PATCH 1/5] fetch: honour Connection: close on non-2xx responses and HTTP/1.0 defaults The HTTP client only applied the response's Connection header when the status code was 2xx, so a 3xx/4xx/5xx response carrying Connection: close was returned to the keep-alive pool and the next request to that origin was written onto a socket the server declared closed. Origins commonly send close on error responses and then actually close, so the reused connection races the FIN. Separately, HTTP/1.0 responses were treated as persistent by default. RFC 9112 9.3: an HTTP/1.0 response is non-persistent unless it carries an explicit Connection: keep-alive. Drop the status-code guard on the Connection header, and set allow_keepalive = false for HTTP/1.0 responses before the header loop so an explicit keep-alive can turn it back on. --- src/http/lib.rs | 35 +++++++---- test/js/web/fetch/fetch-keepalive.test.ts | 75 +++++++++++++++++++++++ 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/http/lib.rs b/src/http/lib.rs index eca6a662c26a..4360b059a4b5 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -4767,6 +4767,15 @@ impl<'a> HTTPClient<'a> { let mut location: &[u8] = b""; let mut pretend_304 = false; let mut is_server_sent_events = false; + + // RFC 9112 §9.3: HTTP/1.0 connections are non-persistent unless the + // response carries an explicit `Connection: keep-alive`. The header loop + // below can flip this back on. h2/h3 reach here with a synthetic + // minor_version of 0 but overwrite allow_keepalive afterwards. + if response.minor_version == 0 { + self.state.flags.allow_keepalive = false; + } + for (header_i, header) in response.headers.list.iter().enumerate() { match hash_header_name(header.name()) { h if h == hash_header_const(b"Content-Length") => { @@ -4876,19 +4885,19 @@ impl<'a> HTTPClient<'a> { location = header.value(); } h if h == hash_header_const(b"Connection") => { - if response.status_code >= 200 && response.status_code <= 299 { - // HTTP headers are case-insensitive (RFC 7230) - if bun_core::strings::eql_case_insensitive_ascii_check_length( - header.value(), - b"close", - ) { - self.state.flags.allow_keepalive = false; - } else if bun_core::strings::eql_case_insensitive_ascii_check_length( - header.value(), - b"keep-alive", - ) { - self.state.flags.allow_keepalive = true; - } + // RFC 9112 §9.6: `close` on a response means the server will + // close after this message regardless of status code; the + // connection MUST NOT be reused. + if bun_core::strings::eql_case_insensitive_ascii_check_length( + header.value(), + b"close", + ) { + self.state.flags.allow_keepalive = false; + } else if bun_core::strings::eql_case_insensitive_ascii_check_length( + header.value(), + b"keep-alive", + ) { + self.state.flags.allow_keepalive = true; } } h if h == hash_header_const(b"Last-Modified") => { diff --git a/test/js/web/fetch/fetch-keepalive.test.ts b/test/js/web/fetch/fetch-keepalive.test.ts index d949b21df8de..21d6426d53c3 100644 --- a/test/js/web/fetch/fetch-keepalive.test.ts +++ b/test/js/web/fetch/fetch-keepalive.test.ts @@ -251,6 +251,81 @@ test("an early response to a streaming POST closes the socket instead of pooling }); }); +// RFC 9112 §9.3/§9.6: a response carrying `Connection: close` must not be +// pooled whatever its status code, and an HTTP/1.0 response is non-persistent +// unless it carries an explicit `Connection: keep-alive`. The server below says +// `close` but keeps the socket open so reuse is directly observable as the +// same connection serving more than one request. Subprocess so the pool is +// empty at the start and doesn't leak between rows. +test("Connection: close on a non-2xx response and HTTP/1.0 defaults are not pooled", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import net from "node:net"; + const rows = [ + ["1.1 200 close", "HTTP/1.1 200 OK", "Connection: close\\r\\n", 3], + ["1.1 404 close", "HTTP/1.1 404 Not Found", "Connection: close\\r\\n", 3], + ["1.1 503 close", "HTTP/1.1 503 Unavailable", "Connection: close\\r\\n", 3], + ["1.1 302 close", "HTTP/1.1 302 Found", "Connection: close\\r\\nLocation: /x\\r\\n", 3], + ["1.0 200 (no keep-alive)", "HTTP/1.0 200 OK", "", 3], + ["1.0 404 (no keep-alive)", "HTTP/1.0 404 Not Found", "", 3], + ["1.0 200 keep-alive", "HTTP/1.0 200 OK", "Connection: keep-alive\\r\\n", 1], + ["1.1 404 (implicit keep-alive)", "HTTP/1.1 404 Not Found", "", 1], + ]; + const result = []; + for (const [name, line, hdr, expected] of rows) { + let conns = 0; + const per = []; + const server = net.createServer(s => { + const id = ++conns; + let buf = ""; + s.on("error", () => {}); + s.on("data", d => { + buf += d; + while (buf.includes("\\r\\n\\r\\n")) { + buf = buf.slice(buf.indexOf("\\r\\n\\r\\n") + 4); + per.push(id); + s.write(line + "\\r\\n" + hdr + "content-length: 2\\r\\n\\r\\nhi"); + } + }); + }); + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const url = "http://127.0.0.1:" + server.address().port + "/"; + for (let i = 0; i < 3; i++) { + const res = await fetch(url, { redirect: "manual" }); + await res.arrayBuffer(); + } + server.close(); + result.push({ row: name, connections: conns, per: per.join(","), expected }); + } + console.log(JSON.stringify(result)); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const result = stdout.startsWith("[") ? JSON.parse(stdout.trim()) : { stdout, stderr }; + expect({ result, exitCode }).toEqual({ + result: [ + { row: "1.1 200 close", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.1 404 close", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.1 503 close", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.1 302 close", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.0 200 (no keep-alive)", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.0 404 (no keep-alive)", connections: 3, per: "1,2,3", expected: 3 }, + { row: "1.0 200 keep-alive", connections: 1, per: "1,1,1", expected: 1 }, + { row: "1.1 404 (implicit keep-alive)", connections: 1, per: "1,1,1", expected: 1 }, + ], + exitCode: 0, + }); +}); + // Negative contract for the gate above: a streamed POST whose chunked body // completed (terminator written) before the response arrived must still hand // its connection back to the keep-alive pool. From e8e0e8b9a35fbc19931458cceffb0a772e34cd22 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 04:35:31 +0000 Subject: [PATCH 2/5] skip the HTTP/1.0 persistence default for the CONNECT reply handle_response_metadata runs once for the proxy's CONNECT reply and again for the origin response inside the tunnel, with no state reset in between. An HTTP/1.0 200 Connection Established from the proxy (Squid, tinyproxy, Apache mod_proxy_connect) would latch allow_keepalive=false and the origin's implicit HTTP/1.1 keep-alive could not clear it, so the tunnel was closed instead of pooled after every request. Gate the default on !(proxy_tunneling && proxy_tunnel.is_none()), mirroring the CONNECT exemption on the Content-Length and Transfer-Encoding arms. Add a regression test that counts CONNECTs through a raw HTTP/1.0 proxy. --- src/http/lib.rs | 11 +++- test/js/web/fetch/fetch-keepalive.test.ts | 77 +++++++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/src/http/lib.rs b/src/http/lib.rs index 4360b059a4b5..311797a64950 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -4770,9 +4770,14 @@ impl<'a> HTTPClient<'a> { // RFC 9112 §9.3: HTTP/1.0 connections are non-persistent unless the // response carries an explicit `Connection: keep-alive`. The header loop - // below can flip this back on. h2/h3 reach here with a synthetic - // minor_version of 0 but overwrite allow_keepalive afterwards. - if response.minor_version == 0 { + // below can flip this back on. Skip the CONNECT reply itself: that + // status line is about the client↔proxy hop, and allow_keepalive is not + // reset between here and the origin response inside the tunnel. h2/h3 + // reach here with a synthetic minor_version of 0 but overwrite + // allow_keepalive afterwards. + if response.minor_version == 0 + && !(self.flags.proxy_tunneling && self.proxy_tunnel.is_none()) + { self.state.flags.allow_keepalive = false; } diff --git a/test/js/web/fetch/fetch-keepalive.test.ts b/test/js/web/fetch/fetch-keepalive.test.ts index 21d6426d53c3..07d9fb8f19ce 100644 --- a/test/js/web/fetch/fetch-keepalive.test.ts +++ b/test/js/web/fetch/fetch-keepalive.test.ts @@ -326,6 +326,83 @@ test("Connection: close on a non-2xx response and HTTP/1.0 defaults are not pool }); }); +// Guard for the CONNECT exemption on the HTTP/1.0 default above: older +// proxies (Squid, tinyproxy, Apache mod_proxy_connect) answer CONNECT with +// `HTTP/1.0 200 Connection Established`. That status line is about the +// client↔proxy hop; once the tunnel is up the origin's HTTP/1.1 response +// governs persistence, so the tunnel must still be pooled. +test("a proxy that answers CONNECT with HTTP/1.0 200 still allows the tunnel to be pooled", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import net from "node:net"; + const tlsCert = ${JSON.stringify({ cert: tls.cert, key: tls.key })}; + await using origin = Bun.serve({ + port: 0, + tls: tlsCert, + fetch: () => new Response("ok"), + }); + + let connects = 0; + const proxy = net.createServer(client => { + let head = Buffer.alloc(0); + let upstream; + client.on("error", () => {}); + client.on("close", () => upstream?.destroy()); + client.on("data", chunk => { + if (upstream) return upstream.write(chunk); + head = Buffer.concat([head, chunk]); + const end = head.indexOf("\\r\\n\\r\\n"); + if (end === -1) return; + connects++; + const leftover = head.subarray(end + 4); + upstream = net.connect(origin.port, "127.0.0.1", () => { + client.write("HTTP/1.0 200 Connection Established\\r\\n\\r\\n"); + if (leftover.length) upstream.write(leftover); + }); + upstream.on("error", () => {}); + upstream.on("data", d => client.write(d)); + upstream.on("close", () => client.destroy()); + }); + }); + await new Promise(r => proxy.listen(0, "127.0.0.1", r)); + + const bodies = []; + for (let i = 0; i < 3; i++) { + const res = await fetch("https://localhost:" + origin.port + "/", { + proxy: "http://127.0.0.1:" + proxy.address().port, + tls: { rejectUnauthorized: false }, + }); + bodies.push(await res.text()); + } + proxy.close(); + console.log(JSON.stringify({ connects, bodies })); + process.exit(0); + `, + ], + env: { + ...bunEnv, + NO_PROXY: undefined, + no_proxy: undefined, + HTTP_PROXY: undefined, + http_proxy: undefined, + HTTPS_PROXY: undefined, + https_proxy: undefined, + }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const result = stdout.startsWith("{") ? JSON.parse(stdout.trim()) : { stdout, stderr }; + expect({ result, exitCode }).toEqual({ + result: { connects: 1, bodies: ["ok", "ok", "ok"] }, + exitCode: 0, + }); +}); + // Negative contract for the gate above: a streamed POST whose chunked body // completed (terminator written) before the response arrived must still hand // its connection back to the keep-alive pool. From ff67ffe6f88c6e8b3c990f3264e6e45ff1ad073f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 04:38:47 +0000 Subject: [PATCH 3/5] drop added src/ comment blocks --- src/http/lib.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/http/lib.rs b/src/http/lib.rs index 311797a64950..94807d4c3c41 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -4768,13 +4768,6 @@ impl<'a> HTTPClient<'a> { let mut pretend_304 = false; let mut is_server_sent_events = false; - // RFC 9112 §9.3: HTTP/1.0 connections are non-persistent unless the - // response carries an explicit `Connection: keep-alive`. The header loop - // below can flip this back on. Skip the CONNECT reply itself: that - // status line is about the client↔proxy hop, and allow_keepalive is not - // reset between here and the origin response inside the tunnel. h2/h3 - // reach here with a synthetic minor_version of 0 but overwrite - // allow_keepalive afterwards. if response.minor_version == 0 && !(self.flags.proxy_tunneling && self.proxy_tunnel.is_none()) { @@ -4890,9 +4883,6 @@ impl<'a> HTTPClient<'a> { location = header.value(); } h if h == hash_header_const(b"Connection") => { - // RFC 9112 §9.6: `close` on a response means the server will - // close after this message regardless of status code; the - // connection MUST NOT be reused. if bun_core::strings::eql_case_insensitive_ascii_check_length( header.value(), b"close", From 7069cdb4daf91fa728e985276caa14d4d9700645 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 07:01:01 +0000 Subject: [PATCH 4/5] test: move new keepalive tests after the streaming-POST pair Keeps the 'Negative contract for the gate above' comment adjacent to the test it refers to. --- test/js/web/fetch/fetch-keepalive.test.ts | 112 +++++++++++----------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/test/js/web/fetch/fetch-keepalive.test.ts b/test/js/web/fetch/fetch-keepalive.test.ts index 07d9fb8f19ce..0fa3969093bb 100644 --- a/test/js/web/fetch/fetch-keepalive.test.ts +++ b/test/js/web/fetch/fetch-keepalive.test.ts @@ -251,6 +251,62 @@ test("an early response to a streaming POST closes the socket instead of pooling }); }); +// Negative contract for the gate above: a streamed POST whose chunked body +// completed (terminator written) before the response arrived must still hand +// its connection back to the keep-alive pool. +test("a completed streaming POST keeps its connection in the keep-alive pool", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import net from "node:net"; + let connections = 0; + const server = net.createServer(sock => { + connections++; + sock.on("error", () => {}); + let buf = ""; + sock.on("data", d => { + buf += d.toString("latin1"); + // One response per fully-received chunked message (terminator seen). + while (buf.includes("0\\r\\n\\r\\n")) { + buf = buf.slice(buf.indexOf("0\\r\\n\\r\\n") + 5); + sock.write("HTTP/1.1 200 OK\\r\\nContent-Length: 2\\r\\n\\r\\nok"); + } + }); + }); + server.listen(0, "127.0.0.1"); + await new Promise(r => server.on("listening", r)); + const url = "http://127.0.0.1:" + server.address().port + "/"; + + const results = []; + for (let i = 0; i < 8; i++) { + const body = new ReadableStream({ + start(c) { + c.enqueue(new TextEncoder().encode("hello")); + c.close(); + }, + }); + const res = await fetch(url, { method: "POST", duplex: "half", body }); + results.push(res.status, await res.text()); + } + console.log(JSON.stringify({ results, connections })); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const result = stdout.startsWith("{") ? JSON.parse(stdout.trim()) : { stdout, stderr }; + expect({ result, exitCode }).toEqual({ + result: { results: Array(8).fill([200, "ok"]).flat(), connections: 1 }, + exitCode: 0, + }); +}); + // RFC 9112 §9.3/§9.6: a response carrying `Connection: close` must not be // pooled whatever its status code, and an HTTP/1.0 response is non-persistent // unless it carries an explicit `Connection: keep-alive`. The server below says @@ -402,59 +458,3 @@ test("a proxy that answers CONNECT with HTTP/1.0 200 still allows the tunnel to exitCode: 0, }); }); - -// Negative contract for the gate above: a streamed POST whose chunked body -// completed (terminator written) before the response arrived must still hand -// its connection back to the keep-alive pool. -test("a completed streaming POST keeps its connection in the keep-alive pool", async () => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` - import net from "node:net"; - let connections = 0; - const server = net.createServer(sock => { - connections++; - sock.on("error", () => {}); - let buf = ""; - sock.on("data", d => { - buf += d.toString("latin1"); - // One response per fully-received chunked message (terminator seen). - while (buf.includes("0\\r\\n\\r\\n")) { - buf = buf.slice(buf.indexOf("0\\r\\n\\r\\n") + 5); - sock.write("HTTP/1.1 200 OK\\r\\nContent-Length: 2\\r\\n\\r\\nok"); - } - }); - }); - server.listen(0, "127.0.0.1"); - await new Promise(r => server.on("listening", r)); - const url = "http://127.0.0.1:" + server.address().port + "/"; - - const results = []; - for (let i = 0; i < 8; i++) { - const body = new ReadableStream({ - start(c) { - c.enqueue(new TextEncoder().encode("hello")); - c.close(); - }, - }); - const res = await fetch(url, { method: "POST", duplex: "half", body }); - results.push(res.status, await res.text()); - } - console.log(JSON.stringify({ results, connections })); - process.exit(0); - `, - ], - env: bunEnv, - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const result = stdout.startsWith("{") ? JSON.parse(stdout.trim()) : { stdout, stderr }; - expect({ result, exitCode }).toEqual({ - result: { results: Array(8).fill([200, "ok"]).flat(), connections: 1 }, - exitCode: 0, - }); -}); From 000f11286211fe537b8ffc597fe339ddbac35731 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:35:17 +0000 Subject: [PATCH 5/5] test: cover ReadableStream POST + 303 Connection: close redirect follow A completed chunked ReadableStream body sets request_stage=Done, so the do_redirect pool/close decision applies. With Connection: close ignored on 3xx (the bug this PR fixes) the closing socket was pooled and immediately reused for the follow-up GET, which either hangs (origin lingers per RFC 9112 9.6) or gets delivered twice (origin closes, GET hits FIN, idempotent retry fires). --- test/js/web/fetch/fetch-keepalive.test.ts | 103 ++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/test/js/web/fetch/fetch-keepalive.test.ts b/test/js/web/fetch/fetch-keepalive.test.ts index 0fa3969093bb..48717a98088f 100644 --- a/test/js/web/fetch/fetch-keepalive.test.ts +++ b/test/js/web/fetch/fetch-keepalive.test.ts @@ -307,6 +307,109 @@ test("a completed streaming POST keeps its connection in the keep-alive pool", a }); }); +// The do_redirect pooling gate only fires when request_stage == Done, which a +// ReadableStream body reaches after the terminating chunk is written (a bytes +// body parks at .body so the socket was already closed on redirect regardless +// of Connection). With Connection: close ignored on 3xx, the closing socket was +// pooled and immediately reused for the follow-up GET: if the origin lingers +// (RFC 9112 §9.6: it stops reading further requests) fetch hangs forever, and +// if it closes the GET hits FIN and is silently retried, so the origin sees it +// twice. Subprocess so the pool is empty and a hang doesn't wedge the runner. +test("a 303 with Connection: close after a streaming POST sends the follow-up GET on a new connection", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import net from "node:net"; + const conns = []; + const server = net.createServer(sock => { + const rec = { id: conns.length, data: "", requests: [] }; + conns.push(rec); + sock.on("error", () => {}); + sock.on("data", chunk => { + rec.data += chunk.toString("latin1"); + for (const m of rec.data.matchAll(/(GET|POST) (\\S+) HTTP\\/1\\.1\\r\\n/g)) { + const line = m[1] + " " + m[2]; + if (!rec.requests.includes(line)) rec.requests.push(line); + } + if (rec.id === 0) { + // Origin: reply 303+close once the chunked body terminator arrives, + // then linger (keep the socket open, ignore anything further). Any + // pipelined follow-up written here never gets a response. + if (!rec.responded && rec.data.includes("0\\r\\n\\r\\n")) { + rec.responded = true; + sock.write( + "HTTP/1.1 303 See Other\\r\\n" + + "Location: /second\\r\\n" + + "Connection: close\\r\\n" + + "Content-Length: 0\\r\\n\\r\\n", + ); + } + } else { + // Fresh connection: answer the redirected GET. + sock.write("HTTP/1.1 200 OK\\r\\nContent-Length: 2\\r\\nConnection: close\\r\\n\\r\\nok"); + sock.end(); + } + }); + }); + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const url = "http://127.0.0.1:" + server.address().port + "/first"; + + const body = new ReadableStream({ + start(c) { + c.enqueue(new TextEncoder().encode("streamed-upload")); + c.close(); + }, + }); + let outcome; + try { + const res = await fetch(url, { + method: "POST", + body, + duplex: "half", + signal: AbortSignal.timeout(2000), + }); + outcome = { status: res.status, path: new URL(res.url).pathname, redirected: res.redirected, text: await res.text() }; + } catch (e) { + outcome = { error: String(e) }; + } + // outcome being populated means the follow-up GET already completed on + // its connection; nothing else writes to the recorded sockets after this. + console.log( + JSON.stringify({ + outcome, + connections: conns.length, + conn0Requests: conns[0]?.requests, + conn0SawGET: (conns[0]?.data ?? "").includes("GET /second"), + conn1Requests: conns[1]?.requests, + }), + ); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const result = stdout.startsWith("{") ? JSON.parse(stdout.trim()) : { stdout, stderr }; + expect({ result, exitCode }).toEqual({ + // Without the fix the GET is pipelined onto conn0 (conn0SawGET: true, + // conn0Requests includes "GET /second") and the lingering origin never + // answers it, so outcome is {error: TimeoutError ...} and connections: 1. + result: { + outcome: { status: 200, path: "/second", redirected: true, text: "ok" }, + connections: 2, + conn0Requests: ["POST /first"], + conn0SawGET: false, + conn1Requests: ["GET /second"], + }, + exitCode: 0, + }); +}); + // RFC 9112 §9.3/§9.6: a response carrying `Connection: close` must not be // pooled whatever its status code, and an HTTP/1.0 response is non-persistent // unless it carries an explicit `Connection: keep-alive`. The server below says