diff --git a/src/http.zig b/src/http.zig index fb2631dc56..fa42aba1a3 100644 --- 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) { diff --git a/test/js/bun/http/proxy.test.js b/test/js/bun/http/proxy.test.js index fc1e220df3..aecb0ee3ae 100644 --- 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); + }); });