fetch: honour Connection: close on non-2xx responses and HTTP/1.0 defaults in the keep-alive pool

src/http/lib.rs

@@ -4767,6 +4767,13 @@ impl<'a> HTTPClient<'a> {
         let mut location: &[u8] = b"";
         let mut pretend_304 = false;
         let mut is_server_sent_events = false;
+
+        if response.minor_version == 0
+            && !(self.flags.proxy_tunneling && self.proxy_tunnel.is_none())
+        {
+            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 +4883,16 @@ 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;
-                        }
+                    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") => {

test/js/web/fetch/fetch-keepalive.test.ts

@@ -306,3 +306,155 @@ test("a completed streaming POST keeps its connection in the keep-alive pool", a
     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
+// `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,
+  });
+});
+
+// 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,
+  });
+});