fix(fetch): custom Host header no longer causes ConnectionRefused #26596
Conversation
When making a fetch() request with a custom Host header different from the URL's host, Bun incorrectly used the Host header value for the connection target, causing ConnectionRefused errors. Changes: - Remove incorrect Host header extraction in fetch.zig that was overriding the connection hostname - Update HTTPThread.zig to prefer explicit hostname override, falling back to URL hostname (no functional change for normal requests) - Add regression test for custom Host header with fetch The fix ensures: 1. Connection is made to the URL's hostname (correct target) 2. Custom Host header is still sent in the HTTP request 3. Explicit hostname overrides for proxies still work correctly Fixes oven-sh#26579 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
WalkthroughThis change addresses an issue where custom Host headers in fetch requests were incorrectly overriding the connection hostname. The fix removes Host header extraction from fetch setup and updates HTTP connection logic to use the proper hostname while preserving Host header functionality. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/http/HTTPThread.zig`:
- Around line 285-286: The requires_custom_request_ctx branch currently uses
client.url.hostname directly and skips the client.hostname override; modify that
branch to compute and use the same target_hostname logic (const target_hostname
= client.hostname orelse client.url.hostname) and pass target_hostname into
this.context(is_ssl).connect (the same way the default path does), ensuring both
custom TLS context paths and the default use client.hostname when provided.
In `@test/js/web/fetch/fetch.test.ts`:
- Around line 2132-2163: Replace the manual try/finally stop of the test server
with a using-based disposal so cleanup is automatic: instead of creating
testServer via serve(...) and calling testServer.stop(true) in finally, declare
the server with using (e.g., using const testServer = serve({...})) and remove
the try/finally block and explicit testServer.stop call; keep the same serve
options and references to testServer.port and ensure the rest of the test (fetch
and assertions) remains unchanged.
| const target_hostname = client.hostname orelse client.url.hostname; | ||
| return try this.context(is_ssl).connect(client, target_hostname, client.url.getPortAuto()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the structure of HTTPThread.zig around lines 285-286
head -n 300 src/http/HTTPThread.zig | tail -n 30Repository: oven-sh/bun
Length of output: 1367
🏁 Script executed:
# Get the total line count
wc -l src/http/HTTPThread.zigRepository: oven-sh/bun
Length of output: 82
🏁 Script executed:
# Search for requires_custom_request_ctx to understand the code structure
rg -n 'requires_custom_request_ctx' src/http/HTTPThread.zig -A 10 -B 2Repository: oven-sh/bun
Length of output: 801
🏁 Script executed:
# Search for where client.hostname is assigned
rg -n '\.hostname\s*=' src/http/HTTPThread.zigRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for all connect() calls in HTTPThread.zig to understand the patterns
rg -n 'connect\(client' src/http/HTTPThread.zig -B 2 -A 1Repository: oven-sh/bun
Length of output: 1555
🏁 Script executed:
# Search for client.hostname assignments across http directory
rg -n 'client\.hostname\s*=' src/http/ --type=zig -B 2 -A 2Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for where client struct is defined and see if hostname field exists
rg -n 'hostname' src/http/HTTPThread.zig -B 1 -A 1Repository: oven-sh/bun
Length of output: 1312
🏁 Script executed:
# Search broader for HTTPClient struct definition
rg -n 'struct.*Client' src/http/ --type=zig -A 10 | head -100Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for HTTPClient struct definition
rg -n 'pub.*struct.*HTTPClient' src/ --type=zig -A 20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for where hostname field is defined on client
rg -n 'hostname\s*:' src/ --type=zig -B 2 -A 1Repository: oven-sh/bun
Length of output: 13019
🏁 Script executed:
# Look at the function signature of the problematic function to understand client type
rg -n 'fn requestAsync\|fn requestAsyncInternal\|fn connect' src/http/HTTPThread.zig -B 5 | head -80Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Get more context around the function containing lines 285-286
sed -n '200,290p' src/http/HTTPThread.zigRepository: oven-sh/bun
Length of output: 4701
🏁 Script executed:
# Check the complete requires_custom_request_ctx handling block
sed -n '225,280p' src/http/HTTPThread.zigRepository: oven-sh/bun
Length of output: 3164
Apply hostname override to custom TLS context paths for consistency.
In the requires_custom_request_ctx branch, direct connections ignore client.hostname and use client.url.hostname on lines 241 and 273, while the default path correctly applies the override. If client.hostname can be set alongside custom TLS contexts, these paths should use the same target_hostname logic:
🛠️ Example adjustment
} else {
+ const target_hostname = client.hostname orelse client.url.hostname;
- return try custom_ssl_context_map.get(other_config).?.connect(client, client.url.hostname, client.url.getPortAuto());
+ return try custom_ssl_context_map.get(other_config).?.connect(client, target_hostname, client.url.getPortAuto());
}
...
+ const target_hostname = client.hostname orelse client.url.hostname;
- return try custom_context.connect(client, client.url.hostname, client.url.getPortAuto());
+ return try custom_context.connect(client, target_hostname, client.url.getPortAuto());🤖 Prompt for AI Agents
In `@src/http/HTTPThread.zig` around lines 285 - 286, The
requires_custom_request_ctx branch currently uses client.url.hostname directly
and skips the client.hostname override; modify that branch to compute and use
the same target_hostname logic (const target_hostname = client.hostname orelse
client.url.hostname) and pass target_hostname into this.context(is_ssl).connect
(the same way the default path does), ensuring both custom TLS context paths and
the default use client.hostname when provided.
| it("should allow custom Host header different from URL (issue #26579)", async () => { | ||
| // Start a test server that echoes back the Host header | ||
| const testServer = serve({ | ||
| idleTimeout: 0, | ||
| port: 0, | ||
| fetch(req) { | ||
| const hostHeader = req.headers.get("host") || "not-set"; | ||
| return new Response(JSON.stringify({ host: hostHeader }), { | ||
| status: 200, | ||
| headers: { "content-type": "application/json" }, | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| try { | ||
| const testServerUrl = `http://127.0.0.1:${testServer.port}`; | ||
|
|
||
| // Fetch to the actual server with a custom Host header | ||
| // The request should still connect to the actual server, but send the custom header | ||
| const response = await fetch(testServerUrl, { | ||
| headers: { "host": "custom-host.example.com" }, | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| const body = await response.json<{ host: string }>(); | ||
|
|
||
| // The server should receive the custom Host header | ||
| expect(body.host).toBe("custom-host.example.com"); | ||
| } finally { | ||
| testServer.stop(true); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Check how serve() is used in other tests and what cleanup pattern is standard
rg -A 5 "serve\(" test/js/web/fetch/ --max-count 20 | head -100Repository: oven-sh/bun
Length of output: 5411
🏁 Script executed:
# Check if there are any existing uses of 'using' with serve() in the test files
rg "using.*serve\(" test/ --max-count 20Repository: oven-sh/bun
Length of output: 25079
🏁 Script executed:
# Look at the serve() function definition to understand its type
fd -e ts -e tsx test/js/web/fetch/ | xargs rg -l "serve" | head -5Repository: oven-sh/bun
Length of output: 610
🏁 Script executed:
# Search for Bun.serve implementation to check if it's a DisposableResource
rg "Symbol.dispose|Symbol.asyncDispose" src/ --max-count 10Repository: oven-sh/bun
Length of output: 2835
🏁 Script executed:
head -50 test/js/web/fetch/fetch.test.ts | grep -E "import|from"Repository: oven-sh/bun
Length of output: 697
🏁 Script executed:
sed -n '2120,2165p' test/js/web/fetch/fetch.test.tsRepository: oven-sh/bun
Length of output: 1688
🏁 Script executed:
# Check if serve() function has Symbol.dispose or Symbol.asyncDispose support
rg -A 10 "serve\s*\(" src/js/bun/ -t ts -t js | grep -A 10 "Symbol\.(async)?Dispose" | head -30Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for serve() definition to see if it's a DisposableResource
rg "export.*serve|class.*Server" src/js/bun/ -A 5 -t ts -t js | head -50Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check fetch.test.ts more broadly to see what the test structure looks like and if there are other patterns
rg -B 5 "using server = serve\|using server = Bun.serve" test/js/web/fetch/fetch.test.ts | head -40Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search more broadly for serve function definition
rg "function serve\|const serve" src/ -A 5 -t ts -t js | head -60Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for Server class definition with dispose methods
rg "class Server|Symbol.dispose|Symbol.asyncDispose" src/js/bun/http.ts -A 3 -B 3Repository: oven-sh/bun
Length of output: 113
🏁 Script executed:
# Check the actual bun type definitions for serve
fd "\.d\.ts" src/ | xargs rg "serve\(" | grep -i "return\|serve"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look at fetch.test.ts to see multiple patterns of using with serve
rg -C 3 "using.*serve\|await using.*serve" test/js/web/fetch/fetch.test.ts | head -100Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Find where serve is exported from in bun
rg "export.*serve" src/js/bun/ -t ts -t js | head -20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for Server implementation files
fd "server\|serve" src/js/bun/ -t f | head -20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for the actual file containing serve implementation
find src -name "*.ts" -o -name "*.js" | xargs grep -l "export.*function serve" | head -5Repository: oven-sh/bun
Length of output: 87
🏁 Script executed:
# Check what the current test does - look at the actual behavior of try/finally with testServer.stop
sed -n '2132,2175p' test/js/web/fetch/fetch.test.tsRepository: oven-sh/bun
Length of output: 1673
🏁 Script executed:
# Let me verify the pattern is consistent by checking how many times using vs await using is used with serve
rg "using server = serve\|using server = Bun.serve" test/js/web/fetch/fetch.test.ts | wc -lRepository: oven-sh/bun
Length of output: 56
🏁 Script executed:
# Check if there are any other tests in fetch.test.ts using try/finally with serve
rg -B 3 "testServer.stop\|server.stop" test/js/web/fetch/fetch.test.ts | head -30Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check the specific pattern in the test to understand if using or await using is needed
rg -A 20 "should allow custom Host header" test/js/web/fetch/fetch.test.tsRepository: oven-sh/bun
Length of output: 873
🏁 Script executed:
# Verify serve() from bun supports dispose by checking if Bun.serve is exported with dispose
rg "Bun.serve\|serve.*Symbol" test/js/web/fetch/fetch.test.ts | head -5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check if there are many using patterns in fetch.test.ts with Bun.serve
rg "using.*Bun.serve" test/js/web/fetch/fetch.test.ts | head -10Repository: oven-sh/bun
Length of output: 375
Use using for Bun.serve cleanup in this test.
This test manually stops the server via try/finally. Prefer using so cleanup is automatic and consistent with other tests in this file using Bun.serve.
♻️ Proposed refactor
- const testServer = serve({
+ using testServer = serve({
idleTimeout: 0,
port: 0,
fetch(req) {
const hostHeader = req.headers.get("host") || "not-set";
return new Response(JSON.stringify({ host: hostHeader }), {
status: 200,
headers: { "content-type": "application/json" },
});
},
});
-
- try {
- const testServerUrl = `http://127.0.0.1:${testServer.port}`;
-
- // Fetch to the actual server with a custom Host header
- // The request should still connect to the actual server, but send the custom header
- const response = await fetch(testServerUrl, {
- headers: { "host": "custom-host.example.com" },
- });
-
- expect(response.status).toBe(200);
- const body = await response.json<{ host: string }>();
-
- // The server should receive the custom Host header
- expect(body.host).toBe("custom-host.example.com");
- } finally {
- testServer.stop(true);
- }
+ const testServerUrl = `http://127.0.0.1:${testServer.port}`;
+
+ // Fetch to the actual server with a custom Host header
+ // The request should still connect to the actual server, but send the custom header
+ const response = await fetch(testServerUrl, {
+ headers: { "host": "custom-host.example.com" },
+ });
+
+ expect(response.status).toBe(200);
+ const body = await response.json<{ host: string }>();
+
+ // The server should receive the custom Host header
+ expect(body.host).toBe("custom-host.example.com");🤖 Prompt for AI Agents
In `@test/js/web/fetch/fetch.test.ts` around lines 2132 - 2163, Replace the manual
try/finally stop of the test server with a using-based disposal so cleanup is
automatic: instead of creating testServer via serve(...) and calling
testServer.stop(true) in finally, declare the server with using (e.g., using
const testServer = serve({...})) and remove the try/finally block and explicit
testServer.stop call; keep the same serve options and references to
testServer.port and ensure the rest of the test (fetch and assertions) remains
unchanged.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| expect(response.status).toBe(200); | ||
| expect(response.arrayBuffer()).resolves.toHaveLength(0); | ||
| }); | ||
|
|
There was a problem hiding this comment.
CLAUDE.md violation: Tests for specific numbered GitHub issues should be placed in the regression test directory.
Per CLAUDE.md:
"If a test is for a specific numbered GitHub Issue, it should be placed in
test/regression/issue/${issueNumber}.test.ts."
Since this test is specifically for issue #26579 (as noted in the test name), it should be moved to:
test/regression/issue/26579.test.ts
Note: The test code itself looks correct - it just needs to be relocated to the appropriate file.
1 participant
Summary
fetch()with custom Host header different from URL causing ConnectionRefusedfetch("https://example.com", { headers: { host: "whatever" } })now works correctlyRoot Cause
In
fetch.zig, the Host header was being extracted and stored in thehostnamefield, which was then used for the connection target. This caused the HTTP client to try connecting to the wrong host.Changes
Test plan
Fixes #26579
🤖 Generated with Claude Code
Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com