fix(http): disable keep-alive on proxy authentication failure (407) (#25884)

## Summary - Disable HTTP keep-alive when a proxy returns a 407 (Proxy Authentication Required) status code - This prevents subsequent requests from trying to reuse a connection that the proxy server has closed - Refactored proxy tests to use `describe.concurrent` and async `Bun.spawn` patterns ## Test plan - [x] Added test `simultaneous proxy auth failures should not hang` that verifies multiple concurrent requests with invalid proxy credentials complete without hanging - [x] Existing proxy tests pass 🤖 Generated with [Claude Code](https://claude.ai/code)
2026-02-09 10:28:47 +00:00 · 2026-01-07 20:49:30 -08:00
parent b20a70dc40
commit 39e2c22e1a
2 changed files with 179 additions and 133 deletions
--- a/src/http.zig
+++ b/src/http.zig
@@ -522,13 +522,14 @@ pub fn isKeepAlivePossible(this: *HTTPClient) bool {
    if (comptime FeatureFlags.enable_keepalive) {
        // TODO keepalive for unix sockets
        if (this.unix_socket_path.length() > 0) return false;
-        // is not possible to reuse Proxy with TSL, so disable keepalive if url is tunneling HTTPS
+
+        // is not possible to reuse Proxy with TLS, so disable keepalive if url is tunneling HTTPS
        if (this.proxy_tunnel != null or (this.http_proxy != null and this.url.isHTTPS())) {
            log("Keep-Alive release (proxy tunneling https)", .{});
            return false;
        }

-        //check state
+        // check state
        if (this.state.flags.allow_keepalive and !this.flags.disable_keepalive) return true;
    }
    return false;
@@ -2338,12 +2339,18 @@ pub fn handleResponseMetadata(
            return ShouldContinue.continue_streaming;
        }

-        //proxy denied connection so return proxy result (407, 403 etc)
+        // proxy denied connection so return proxy result (407, 403 etc)
        this.flags.proxy_tunneling = false;
+        this.flags.disable_keepalive = true;
    }

    const status_code = response.status_code;

+    if (status_code == 407) {
+        // If the request is being proxied and passes through the 407 status code, then let's also not do HTTP Keep-Alive.
+        this.flags.disable_keepalive = true;
+    }
+
    // if is no redirect or if is redirect == "manual" just proceed
    const is_redirect = status_code >= 300 and status_code <= 399;
    if (is_redirect) {
--- a/test/js/bun/http/proxy.test.js
+++ b/test/js/bun/http/proxy.test.js
@@ -1,7 +1,6 @@
-import { afterAll, beforeAll, expect, it } from "bun:test";
+import { afterAll, beforeAll, describe, expect, it } from "bun:test";
 import fs from "fs";
 import { bunEnv, bunExe, gc } from "harness";
-import { tmpdir } from "os";
 import path from "path";

 let proxy, auth_proxy, server;
@@ -77,145 +76,185 @@ afterAll(() => {
 });

 const test = process.env.PROXY_URL ? it : it.skip;
+describe.concurrent(() => {
+  test("should be able to post on TLS", async () => {
+    const data = JSON.stringify({
+      "name": "bun",
+    });

-test("should be able to post on TLS", async () => {
-  const data = JSON.stringify({
-    "name": "bun",
+    const result = await fetch("https://httpbin.org/post", {
+      method: "POST",
+      proxy: process.env.PROXY_URL,
+      verbose: true,
+      headers: {
+        "Content-Type": "application/json",
+      },
+      body: data,
+    }).then(res => res.json());
+
+    expect(result.data).toBe(data);
  });

-  const result = await fetch("https://httpbin.org/post", {
-    method: "POST",
-    proxy: process.env.PROXY_URL,
-    verbose: true,
-    headers: {
-      "Content-Type": "application/json",
-    },
-    body: data,
-  }).then(res => res.json());
+  test("should be able to post bigger on TLS", async () => {
+    const data = fs.readFileSync(path.join(import.meta.dir, "fetch.json")).toString("utf8");
+    const result = await fetch("https://httpbin.org/post", {
+      method: "POST",
+      proxy: process.env.PROXY_URL,
+      verbose: true,
+      headers: {
+        "Content-Type": "application/json",
+      },
+      body: data,
+    }).then(res => res.json());
+    expect(result.data).toBe(data);
+  });

-  expect(result.data).toBe(data);
-});
+  describe("proxy non-TLS", async () => {
+    let url;
+    let auth_proxy_url;
+    let proxy_url;
+    const requests = [
+      () => [new Request(url), auth_proxy_url],
+      () => [
+        new Request(url, {
+          method: "POST",
+          body: "Hello, World",
+        }),
+        auth_proxy_url,
+      ],
+      () => [url, auth_proxy_url],
+      () => [new Request(url), proxy_url],
+      () => [
+        new Request(url, {
+          method: "POST",
+          body: "Hello, World",
+        }),
+        proxy_url,
+      ],
+      () => [url, proxy_url],
+    ];
+    beforeAll(() => {
+      url = `http://localhost:${server.port}`;
+      auth_proxy_url = `http://squid_user:ASD123%40123asd@localhost:${auth_proxy.port}`;
+      proxy_url = `localhost:${proxy.port}`;
+    });

-test("should be able to post bigger on TLS", async () => {
-  const data = fs.readFileSync(path.join(import.meta.dir, "fetch.json")).toString("utf8");
-  const result = await fetch("https://httpbin.org/post", {
-    method: "POST",
-    proxy: process.env.PROXY_URL,
-    verbose: true,
-    headers: {
-      "Content-Type": "application/json",
-    },
-    body: data,
-  }).then(res => res.json());
-  expect(result.data).toBe(data);
-});
-
-it("proxy non-TLS", async () => {
-  const url = `http://localhost:${server.port}`;
-  const auth_proxy_url = `http://squid_user:ASD123%40123asd@localhost:${auth_proxy.port}`;
-  const proxy_url = `localhost:${proxy.port}`;
-  const requests = [
-    [new Request(url), auth_proxy_url],
-    [
-      new Request(url, {
-        method: "POST",
-        body: "Hello, World",
-      }),
-      auth_proxy_url,
-    ],
-    [url, auth_proxy_url],
-    [new Request(url), proxy_url],
-    [
-      new Request(url, {
-        method: "POST",
-        body: "Hello, World",
-      }),
-      proxy_url,
-    ],
-    [url, proxy_url],
-  ];
-  for (let [request, proxy] of requests) {
-    gc();
-    const response = await fetch(request, { verbose: true, proxy });
-    gc();
-    const text = await response.text();
-    gc();
-    expect(text).toBe("Hello, World");
-  }
-});
-
-it("proxy non-TLS auth can fail", async () => {
-  const url = `http://localhost:${server.port}`;
-
-  {
-    try {
-      const response = await fetch(url, { verbose: true, proxy: `http://localhost:${auth_proxy.port}` });
-      expect(response.status).toBe(407);
-    } catch (err) {
-      expect(true).toBeFalsy();
-    }
-  }
-
-  {
-    try {
-      const response = await fetch(url, {
-        verbose: true,
-        proxy: `http://squid_user:asdf123@localhost:${auth_proxy.port}`,
+    for (let callback of requests) {
+      test(async () => {
+        const [request, proxy] = callback();
+        gc();
+        const response = await fetch(request, { verbose: true, proxy });
+        gc();
+        const text = await response.text();
+        gc();
+        expect(text).toBe("Hello, World");
      });
-      expect(response.status).toBe(403);
-    } catch (err) {
-      expect(true).toBeFalsy();
    }
-  }
-});
-
-it.each([
-  [undefined, undefined],
-  ["", ""],
-  ["''", "''"],
-  ['""', '""'],
-])("test proxy env, http_proxy=%s https_proxy=%s", async (http_proxy, https_proxy) => {
-  const path = `${tmpdir()}/bun-test-http-proxy-env-${Date.now()}.ts`;
-  fs.writeFileSync(path, 'await fetch("https://example.com");');
-
-  const { stderr, exitCode } = Bun.spawnSync({
-    cmd: [bunExe(), "run", path],
-    env: {
-      ...bunEnv,
-      http_proxy: http_proxy,
-      https_proxy: https_proxy,
-    },
-    stdout: "inherit",
-    stderr: "pipe",
  });

-  try {
+  it("proxy non-TLS auth can fail", async () => {
+    const url = `http://localhost:${server.port}`;
+
+    {
+      try {
+        const response = await fetch(url, {
+          verbose: true,
+          proxy: `http://localhost:${auth_proxy.port}`,
+        });
+        expect(response.status).toBe(407);
+      } catch (err) {
+        expect(true).toBeFalsy();
+      }
+    }
+
+    {
+      try {
+        const response = await fetch(url, {
+          verbose: true,
+          proxy: `http://squid_user:asdf123@localhost:${auth_proxy.port}`,
+        });
+        expect(response.status).toBe(403);
+      } catch (err) {
+        expect(true).toBeFalsy();
+      }
+    }
+  });
+
+  it("simultaneous proxy auth failures should not hang", async () => {
+    const url = `http://localhost:${server.port}`;
+    const invalidProxy = `http://localhost:${auth_proxy.port}`;
+
+    // First batch: 5 simultaneous fetches with invalid credentials
+    const firstBatch = await Promise.all([
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+    ]);
+    expect(firstBatch.map(r => r.status)).toEqual([407, 407, 407, 407, 407]);
+    await Promise.all(firstBatch.map(r => r.text())).catch(() => {});
+
+    // Second batch: immediately send another 5
+    // Before the fix, these would hang due to keep-alive on failed proxy connections
+    const secondBatch = await Promise.all([
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+      fetch(url, { proxy: invalidProxy }),
+    ]);
+    expect(secondBatch.map(r => r.status)).toEqual([407, 407, 407, 407, 407]);
+    await Promise.all(secondBatch.map(r => r.text())).catch(() => {});
+  });
+
+  it.each([
+    [undefined, undefined],
+    ["", ""],
+    ["''", "''"],
+    ['""', '""'],
+  ])("test proxy env, http_proxy=%s https_proxy=%s", async (http_proxy, https_proxy) => {
+    const { exited, stderr: stream } = Bun.spawn({
+      cmd: [bunExe(), "-e", 'await fetch("https://example.com")'],
+      env: {
+        ...bunEnv,
+        http_proxy: http_proxy,
+        https_proxy: https_proxy,
+      },
+      stdout: "inherit",
+      stderr: "pipe",
+    });
+
+    const [exitCode, stderr] = await Promise.all([exited, stream.text()]);
+
    expect(stderr.includes("FailedToOpenSocket: Was there a typo in the url or port?")).toBe(false);
    expect(exitCode).toBe(0);
-  } finally {
-    fs.unlinkSync(path);
-  }
-});
-
-it.each([
-  // Empty entries in NO_PROXY should not cause out-of-bounds access
-  ["localhost, , example.com"],
-  [",localhost,example.com"],
-  ["localhost,example.com,"],
-  ["  ,  ,  "],
-  [",,,"],
-  [". , .. , ..."],
-])("NO_PROXY with empty entries does not crash: %s", async no_proxy => {
-  // We just need to verify parsing NO_PROXY doesn't crash.
-  // The fetch target doesn't matter - NO_PROXY parsing happens before the connection.
-  const { exitCode } = Bun.spawnSync({
-    cmd: [bunExe(), "-e", `fetch("http://localhost:1").catch(() => {})`],
-    env: {
-      ...bunEnv,
-      http_proxy: "http://127.0.0.1:1",
-      NO_PROXY: no_proxy,
-    },
  });

-  expect(exitCode).toBe(0);
+  it.each([
+    // Empty entries in NO_PROXY should not cause out-of-bounds access
+    ["localhost, , example.com"],
+    [",localhost,example.com"],
+    ["localhost,example.com,"],
+    ["  ,  ,  "],
+    [",,,"],
+    [". , .. , ..."],
+  ])("NO_PROXY with empty entries does not crash: %s", async no_proxy => {
+    // We just need to verify parsing NO_PROXY doesn't crash.
+    // The fetch target doesn't matter - NO_PROXY parsing happens before the connection.
+    const { exited, stderr: stream } = Bun.spawn({
+      cmd: [bunExe(), "-e", `fetch("http://localhost:1").catch(() => {})`],
+      env: {
+        ...bunEnv,
+        http_proxy: "http://127.0.0.1:1",
+        NO_PROXY: no_proxy,
+      },
+      stderr: "pipe",
+    });
+    const [exitCode, stderr] = await Promise.all([exited, stream.text()]);
+    if (exitCode !== 0) {
+      console.error("stderr:", stderr);
+    }
+    expect(exitCode).toBe(0);
+  });
 });