fix(dns): fallback to public DNS servers when c-ares detects only 127.0.0.1 on Windows #26468
+96
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….0.0.1 on Windows On Windows, c-ares uses IPHLPAPI (GetNetworkParams/GetAdaptersInfo) to detect system DNS servers. This can fail on some Windows configurations, causing c-ares to fall back to 127.0.0.1, which doesn't respond to DNS queries. This results in ECONNREFUSED errors for dns.resolveSrv() and other DNS record queries that must use c-ares. This fix detects when c-ares has only 127.0.0.1 configured as a DNS server after initialization and replaces it with public DNS servers (Cloudflare 1.1.1.1, Google 8.8.8.8/8.8.4.4) as a fallback. Fixes #26467 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
Author
Contributor
WalkthroughAdds Windows-specific DNS fallback logic that detects when system-reported DNS servers default to localhost (127.0.0.1) and automatically replaces them with public DNS servers. A regression test validates DNS resolution and the fallback behavior on affected systems. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/deps/c_ares.zig`:
- Around line 622-659: The applyWindowsDnsFallback function currently
unconditionally replaces a single 127.0.0.1 DNS server; change it to respect
user intent by (1) checking an opt-out environment variable or config flag
(e.g., "CARES_DISABLE_LOCALHOST_FALLBACK") at the start of
applyWindowsDnsFallback and returning early if set, and (2) before calling
ares_set_servers_csv, attempt to verify the localhost resolver is unreachable by
performing a quick query against the channel (or a direct socket probe) and only
apply the fallback if that probe fails; also emit a clear processLogger or debug
log when skipping or applying the fallback so users can discover the behavior.
Ensure references to ares_get_servers_ports, ares_set_servers_csv, and
applyWindowsDnsFallback are used to locate and modify the code.
In `@test/regression/issue/26467.test.ts`:
- Around line 33-49: The test's non-localhost assertion should only run on
Windows to avoid false failures on systems using loopback DNS (e.g.,
127.0.0.53); modify the test "dns.getServers should not return only 127.0.0.1
after initialization" to check process.platform === "win32" before asserting
hasNonLocalhost, and on Windows assert that the servers array is not composed
solely of "127.0.0.1" and "::1" (i.e., fail only when every entry is exactly
"127.0.0.1" or "::1"); keep the existing Array.isArray and length checks for all
platforms and only gate the non-localhost check (hasNonLocalhost / servers.some)
behind the Windows platform check.
Comment on lines
+622
to
+659
| /// On Windows, c-ares uses IPHLPAPI (GetNetworkParams/GetAdaptersInfo) to detect | ||
| /// system DNS servers. This can fail on some Windows configurations, causing | ||
| /// c-ares to fall back to 127.0.0.1, which doesn't respond to DNS queries. | ||
| /// This function detects that case and sets public DNS servers as a fallback. | ||
| fn applyWindowsDnsFallback(channel: *Channel) void { | ||
| var servers: ?*struct_ares_addr_port_node = null; | ||
| if (Error.get(ares_get_servers_ports(channel, &servers))) |_| { | ||
| // Failed to get servers, can't apply fallback | ||
| return; | ||
| } | ||
| defer ares_free_data(servers); | ||
|
|
||
| // Check if the only configured server is localhost (127.0.0.1) | ||
| const first_server = servers orelse return; | ||
| if (first_server.next != null) { | ||
| // Multiple servers configured, don't override | ||
| return; | ||
| } | ||
|
|
||
| // Check if it's an IPv4 address | ||
| if (first_server.family != AF.INET) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if it's 127.0.0.1 (localhost) | ||
| // struct_in_addr is std.posix.sockaddr.in which contains .sa (sockaddr struct) | ||
| // The s_addr is the first 4 bytes in network byte order | ||
| const addr_bytes: *const [4]u8 = @ptrCast(&first_server.addr.addr4); | ||
| const is_localhost = addr_bytes[0] == 127 and addr_bytes[1] == 0 and addr_bytes[2] == 0 and addr_bytes[3] == 1; | ||
|
|
||
| if (!is_localhost) { | ||
| return; | ||
| } | ||
|
|
||
| // The only DNS server is 127.0.0.1, which likely won't work. | ||
| // Set fallback public DNS servers: Cloudflare (1.1.1.1) and Google (8.8.8.8, 8.8.4.4) | ||
| _ = ares_set_servers_csv(channel, "1.1.1.1,8.8.8.8,8.8.4.4"); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the entire function and surrounding context
cat -n src/deps/c_ares.zig | head -700 | tail -100Repository: oven-sh/bun
Length of output: 5382
🏁 Script executed:
# Search for calls to applyWindowsDnsFallback
rg "applyWindowsDnsFallback" src/Repository: oven-sh/bun
Length of output: 198
🏁 Script executed:
# Look for any configuration or opt-in mechanisms related to DNS fallback
rg -i "fallback|dns.*opt|opt.*dns" src/deps/c_ares.zig -A 2 -B 2Repository: oven-sh/bun
Length of output: 4017
🏁 Script executed:
# Check if there's any documentation or comments about Windows DNS issues
rg -i "windows.*dns|iphlpapi|getnetworkparams" src/deps/c_ares.zig -A 3 -B 3Repository: oven-sh/bun
Length of output: 2553
Public DNS fallback can override intentional localhost resolvers.
The function unconditionally replaces a 127.0.0.1-only DNS configuration with public resolvers on Windows. While defensive checks prevent override when multiple servers are configured, a user with an intentional single localhost resolver (Pi-hole, corporate DNS stub, VPN tunnel) would have it silently replaced. This addresses a real Windows c-ares bug (#26467), but there's no mechanism for users to disable this behavior. Consider either (a) adding an environment variable or configuration flag to opt out, (b) attempting to verify the localhost resolver is actually unreachable before replacing it, or (c) making this behavior more discoverable in logs/documentation.
🤖 Prompt for AI Agents
In `@src/deps/c_ares.zig` around lines 622 - 659, The applyWindowsDnsFallback
function currently unconditionally replaces a single 127.0.0.1 DNS server;
change it to respect user intent by (1) checking an opt-out environment variable
or config flag (e.g., "CARES_DISABLE_LOCALHOST_FALLBACK") at the start of
applyWindowsDnsFallback and returning early if set, and (2) before calling
ares_set_servers_csv, attempt to verify the localhost resolver is unreachable by
performing a quick query against the channel (or a direct socket probe) and only
apply the fallback if that probe fails; also emit a clear processLogger or debug
log when skipping or applying the fallback so users can discover the behavior.
Ensure references to ares_get_servers_ports, ares_set_servers_csv, and
applyWindowsDnsFallback are used to locate and modify the code.
Comment on lines
+33
to
+49
| test("dns.getServers should not return only 127.0.0.1 after initialization", () => { | ||
| // After the fix, if c-ares detects only 127.0.0.1 on Windows, | ||
| // it should have been replaced with public DNS servers. | ||
| const servers = dns.getServers(); | ||
|
|
||
| expect(Array.isArray(servers)).toBe(true); | ||
| expect(servers.length).toBeGreaterThan(0); | ||
|
|
||
| // The servers should contain at least one non-localhost server | ||
| // If the fix worked, we shouldn't see only "127.0.0.1" as the sole server | ||
| const hasNonLocalhost = servers.some(server => !server.startsWith("127.") && server !== "::1"); | ||
|
|
||
| // Note: This test may pass even without the fix on systems with proper DNS configuration. | ||
| // The fix specifically targets Windows systems where IPHLPAPI fails to detect DNS servers. | ||
| // On Linux/macOS, this should always pass since /etc/resolv.conf is reliable. | ||
| expect(hasNonLocalhost).toBe(true); | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard the non-localhost assertion to Windows to avoid false failures.
Line 43 treats any 127.* or ::1 entry as a failure, but Linux (systemd-resolved 127.0.0.53) and container DNS (127.0.0.11) legitimately use loopback. This can make the test flaky off Windows. Gate the assertion to Windows and only reject a sole 127.0.0.1/::1 entry.
💡 Proposed fix
- // The servers should contain at least one non-localhost server
- // If the fix worked, we shouldn't see only "127.0.0.1" as the sole server
- const hasNonLocalhost = servers.some(server => !server.startsWith("127.") && server !== "::1");
-
- // Note: This test may pass even without the fix on systems with proper DNS configuration.
- // The fix specifically targets Windows systems where IPHLPAPI fails to detect DNS servers.
- // On Linux/macOS, this should always pass since /etc/resolv.conf is reliable.
- expect(hasNonLocalhost).toBe(true);
+ // Only assert the localhost-only fallback case on Windows.
+ if (process.platform === "win32") {
+ const onlyLocalhost =
+ servers.length === 1 && (servers[0] === "127.0.0.1" || servers[0] === "::1");
+ expect(onlyLocalhost).toBe(false);
+ }📝 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("dns.getServers should not return only 127.0.0.1 after initialization", () => { | |
| // After the fix, if c-ares detects only 127.0.0.1 on Windows, | |
| // it should have been replaced with public DNS servers. | |
| const servers = dns.getServers(); | |
| expect(Array.isArray(servers)).toBe(true); | |
| expect(servers.length).toBeGreaterThan(0); | |
| // The servers should contain at least one non-localhost server | |
| // If the fix worked, we shouldn't see only "127.0.0.1" as the sole server | |
| const hasNonLocalhost = servers.some(server => !server.startsWith("127.") && server !== "::1"); | |
| // Note: This test may pass even without the fix on systems with proper DNS configuration. | |
| // The fix specifically targets Windows systems where IPHLPAPI fails to detect DNS servers. | |
| // On Linux/macOS, this should always pass since /etc/resolv.conf is reliable. | |
| expect(hasNonLocalhost).toBe(true); | |
| }); | |
| test("dns.getServers should not return only 127.0.0.1 after initialization", () => { | |
| // After the fix, if c-ares detects only 127.0.0.1 on Windows, | |
| // it should have been replaced with public DNS servers. | |
| const servers = dns.getServers(); | |
| expect(Array.isArray(servers)).toBe(true); | |
| expect(servers.length).toBeGreaterThan(0); | |
| // Only assert the localhost-only fallback case on Windows. | |
| if (process.platform === "win32") { | |
| const onlyLocalhost = | |
| servers.length === 1 && (servers[0] === "127.0.0.1" || servers[0] === "::1"); | |
| expect(onlyLocalhost).toBe(false); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In `@test/regression/issue/26467.test.ts` around lines 33 - 49, The test's
non-localhost assertion should only run on Windows to avoid false failures on
systems using loopback DNS (e.g., 127.0.0.53); modify the test "dns.getServers
should not return only 127.0.0.1 after initialization" to check process.platform
=== "win32" before asserting hasNonLocalhost, and on Windows assert that the
servers array is not composed solely of "127.0.0.1" and "::1" (i.e., fail only
when every entry is exactly "127.0.0.1" or "::1"); keep the existing
Array.isArray and length checks for all platforms and only gate the
non-localhost check (hasNonLocalhost / servers.some) behind the Windows platform
check.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
dns.resolveSrv()failing withECONNREFUSEDon Windows when c-ares falls back to using127.0.0.1as the DNS serverRoot Cause
On Windows, c-ares uses IPHLPAPI (
GetNetworkParams/GetAdaptersInfo) to detect system DNS servers. This can fail on certain Windows configurations (e.g., fresh installs, certain network setups), causing c-ares to fall back to127.0.0.1as the DNS server.Since
127.0.0.1typically doesn't run a DNS server, all DNS queries that require c-ares (like SRV records, since Windows'getaddrinfo()doesn't support them) fail withECONNREFUSED.On Linux, this issue doesn't occur because c-ares reads DNS servers from
/etc/resolv.conf, which is reliably maintained by the system.Solution
After initializing the c-ares channel on Windows, we check if the only configured DNS server is
127.0.0.1. If so, we replace it with well-known public DNS servers:This allows DNS queries to succeed even when Windows' DNS server detection fails.
Test plan
test/regression/issue/26467.test.tstest/js/node/dns/node-dns.test.js)Fixes #26467
🤖 Generated with Claude Code