Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 2 commits into from
Jan 8, 2026
+175 −129
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45554e2
Ensure we don't keepalive the connection when proxy authentication fails
Jarred-Sumner Jan 8, 2026
37be8db
Update http.zig
Jarred-Sumner Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/http.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +525 to +526
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Minor wording improvement suggestion.

The comment dropped "it" ("is not possible" vs "it is not possible"). While grammatically acceptable, restoring "it" would improve readability.

♻️ Suggested refinement 
-        // is not possible to reuse Proxy with TLS, so disable keepalive if url is tunneling HTTPS
+        // it is not possible to reuse Proxy with TLS, so disable keepalive if url is tunneling HTTPS
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// is not possible to reuse Proxy with TLS, so disable keepalive if url is tunneling HTTPS
// it is not possible to reuse Proxy with TLS, so disable keepalive if url is tunneling HTTPS
🤖 Prompt for AI Agents 
In @src/http.zig around lines 525 - 526, Update the inline comment that
currently reads "is not possible to reuse Proxy with TLS, so disable keepalive
if url is tunneling HTTPS" to restore the missing pronoun so it reads "it is not
possible to reuse Proxy with TLS, so disable keepalive if url is tunneling
HTTPS" to improve readability; locate that comment string in src/http.zig and
replace accordingly.

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;
Expand Down Expand Up @@ -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) {
Expand Down
291 changes: 165 additions & 126 deletions test/js/bun/http/proxy.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());

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);
});
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);
});

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);
});
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}`;
});

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");
}
});
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");
});
}
Comment on lines +143 to +152
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add descriptive test names for better debugging.

The tests in this loop lack descriptive names, making it difficult to identify which request configuration fails when debugging.

♻️ Suggested improvement

Consider adding descriptive test names based on the request configuration:

-    for (let callback of requests) {
-      test(async () => {
+    for (let i = 0; i < requests.length; i++) {
+      test(`proxy request configuration ${i + 1}/${requests.length}`, async () => {
-        const [request, proxy] = callback();
+        const [request, proxy] = requests[i]();
         gc();
         const response = await fetch(request, { verbose: true, proxy });
         gc();

Or better yet, use it.each with descriptive labels for each configuration.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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");
});
}
for (let i = 0; i < requests.length; i++) {
test(`proxy request configuration ${i + 1}/${requests.length}`, async () => {
const [request, proxy] = requests[i]();
gc();
const response = await fetch(request, { verbose: true, proxy });
gc();
const text = await response.text();
gc();
expect(text).toBe("Hello, World");
});
}
🤖 Prompt for AI Agents 
In @test/js/bun/http/proxy.test.js around lines 143 - 152, The anonymous tests
in the loop use test(async () => { ... }) making failures hard to trace; update
the test declarations to include descriptive names (e.g., test(name, async () =>
{ ... })) that reflect the request/proxy configuration returned by callback() or
switch to a parameterized pattern like it.each with descriptive labels; use
values from callback() (the request or proxy details) to build the test title so
each case (the fetch(request, { verbose: true, proxy })) is uniquely
identifiable during failures.

});

it("proxy non-TLS auth can fail", async () => {
const url = `http://localhost:${server.port}`;
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://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();
{
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.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",
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(() => {});
});

try {
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);
});
});