Repository navigation
Conversation
bsd_create_listen_socket, bsd_create_bound_socket and bsd_create_udp_socket walked every AF_INET6 entry before any AF_INET entry, so listen(port, "localhost") bound ::1 even when the resolver put 127.0.0.1 first. Node binds the first dns.lookup() result. The three walks now run over the getaddrinfo chain in order and bind the first entry. The wildcard (no host) keeps IPv6 first so the default listener stays on the dual-stack ::. A bind() failure moves to the next entry only when it says the family is unusable on this host (EADDRNOTAVAIL, EAFNOSUPPORT). EADDRINUSE and EACCES are reported instead of binding the other loopback family. UDP gains the same fallback and no longer leaks the fd when IPV6_V6ONLY fails.
… resolver contract in tests The wildcard (no host) falls through on any bind error as before, so a `::` bind that fails still gets a 0.0.0.0 listener. Only a named host stops on EADDRINUSE and EACCES. bsd_create_socket now writes its error for the TCP walks too, so a run where every socket() fails reports EAFNOSUPPORT. Tests: every listener binds the first dns.lookup() result on every platform, the wildcard fallback is pinned, the gai.conf test asserts only the order it controls and skips when the namespace cannot change it, and the server.spec hostname test is no longer skipped.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit details
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 |
|
Updated 5:49 PM PT - Oct 28th, 2026
❌ @robobun, your commit 1126b33 has 7 failures in
🧪 To try this PR locally: bunx bun-pr 47224That installs a local version of the PR into your bun-47224 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked two things in the bsd.c rewrite and ruled them out: relinking ai_next in bsd_addrinfo_inet6_first is safe for freeaddrinfo() on musl as well as glibc/Windows (musl counts the nodes in the chain and locates the allocation base from the last node's slot, so a reordered chain that still contains every node frees cleanly), and passing error into bsd_create_socket does not clobber a later bind errno — it zeroes *error on success before the bind result is written, so the reported code is still the last failing call's.
Extended reasoning...
The change rewrites the getaddrinfo walk shared by bsd_create_bound_socket, bsd_create_listen_socket and bsd_create_udp_socket in packages/bun-usockets/src/bsd.c, plus three test files; it touches the native listen/bind path used by Bun.serve, Bun.listen, node:net and Bun.udpSocket but no auth, crypto or input-parsing surface. Inline findings were posted, so a human look is already signalled; this note records only the chain-relink/freeaddrinfo and out-parameter ordering concerns that were examined and ruled out.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| last_error = LIBUS_ERR; | ||
| bsd_close_socket(listenFd); | ||
| freeaddrinfo(result); | ||
| return LIBUS_SOCKET_ERROR; | ||
| if (!bsd_bind_should_try_next(host, last_error)) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
🔴 Operators running Bun.serve with http3 and no hostname can now get a QUIC listener on 0.0.0.0 while TCP sits on ::, where the base failed or retried. The new UDP walk at bsd.c:1829 falls through on any bind() error when host == NULL, so a UDP [::]:N conflict silently yields an IPv4-only QUIC socket. The retry loop in src/runtime/server/mod.rs:3129-3145 only fires when h3_listener is None, so neither the fresh-port retry nor the "Failed to listen on UDP port {h3_port} for HTTP/3" error runs. Fix: when the wildcard UDP walk cannot bind the same family the TCP listener bound, report the error so mod.rs can retry or throw, e.g. limit the wildcard fallthrough in bsd_create_udp_socket to EAFNOSUPPORT/EADDRNOTAVAIL, or have the H3 bind pass the TCP listener's bound family instead of NULL. [also at: packages/bun-usockets/src/bsd.c:1829 - HTTP/3 users whose wildcard UDP port is already held now get a server whose QUIC half silently lands on 0.0.0.0 while TCP stays on ::, instead of the error the base throws.]
Why this was flagged
Trigger: Bun.serve({ port: 0, http3: true }) with no hostname on Linux while another process holds a UDP socket on an IPv6-specific address at the port the TCP listener just got. The TCP walk binds [::]:N (bsd.c:1432-1441). The H3 app then calls uws_h3_app_listen_with_config with the same null host (src/runtime/server/mod.rs:3118-3128), which reaches bsd_create_udp_socket(NULL, N, ...). bind([::]:N) fails with EADDRINUSE; bsd_bind_should_try_next(NULL, EADDRINUSE) at bsd.c:1829 returns true because host == NULL, so the loop binds 0.0.0.0:N and returns success. On the base branch the old UDP code bound the first AF_INET6 entry once and returned LIBUS_SOCKET_ERROR on that failure, so h3_listener stayed None and mod.rs:3130-3137 closed TCP:N and retried with a fresh port (port 0) or threw "Failed to listen on UDP port {h3_port} for HTTP/3" (mod.rs:3140). After the change h3_listener is Some, the retry and the error are skipped, and the server advertises Alt-Svc on port N while QUIC is reachable only over IPv4.
Verification: When Bun.serve({ http3: true }) has no hostname and another process holds a UDP socket on an IPv6-only address, the new UDP walk silently binds 0.0.0.0:N where the base reported the failure. bsd_bind_should_try_next (bsd.c:1176) is return host == NULL || err == LIBUS_EAFNOSUPPORT || err == LIBUS_EADDRNOTAVAIL;, so with host == NULL every bind error moves on to the next entry.
| using occupant = Bun.listen({ hostname: "::1", port: 0, socket: { data() {} } }); | ||
| using server = serve({ port: occupant.port, fetch: () => new Response() }); | ||
| expect(server.address).toMatchObject({ address: "0.0.0.0", port: occupant.port }); |
There was a problem hiding this comment.
🟡 (optional) Linux CI runs get an intermittent failure of the new 0.0.0.0 fallback test whenever the kernel hands the ::1 occupant a port that a concurrent test already uses on 127.0.0.1. Bun.listen at serve-listen.test.ts:301 asks for port 0 on ::1; Linux only rejects a candidate port when the bound address matches, and ::1 never matches 127.0.0.1, so P can collide with another file's 127.0.0.1:P server. serve({ port: P }) at :302 then fails on :: (held by ::1) and again on 0.0.0.0 (held by 127.0.0.1:P) and throws EADDRINUSE. Fix: choose P so both loopback families are known free (probe-bind 0.0.0.0:0, release it, then occupy ::1:P), or retry the occupant/serve pair a bounded number of times on EADDRINUSE.
Why this was flagged
Trigger: the test runs on Linux (skipIf(!isLinux || !hasIPv6) at :298) while any other process on the runner holds 127.0.0.1:P or 0.0.0.0:P, where P is the ephemeral port the kernel picks for Bun.listen({ hostname: "::1", port: 0 }) at serve-listen.test.ts:301. Linux's inet_csk_find_open_port only skips a port when inet_bind_conflict finds a socket whose receive address equals the new one; ipv6_rcv_saddr_equal(::1, 127.0.0.1) is false, so a port already bound by an IPv4 listener of a concurrently running test file is a valid pick for ::1. serve({ port: occupant.port }) at :302 passes host NULL; bsd.c:1432-1447 tries :: (conflicts with ::1:P, EADDRINUSE), then 0.0.0.0:P (conflicts with the foreign 127.0.0.1:P, EADDRINUSE), and the test throws before the expect at :303. Bun's CI runs test files in parallel and each file opens several 127.0.0.1:0 servers, so the per-run probability is roughly (concurrent IPv4 listeners)/(ephemeral range), a steady low-rate flake on every Linux lane. The base branch had no such test.
Verification: packages/bun-usockets/src/bsd.c:1432-1447 iterates :: first and then 0.0.0.0:P, which conflicts with the foreign 127.0.0.1:P owner. The loop exhausts and returns LIBUS_SOCKET_ERROR (line 1450), so serve() throws and the assertion at line 303 is never reached. There is no retry, no reservation of 127.0.0.1:P, and no catch around serve(); any EADDRINUSE from the wildcard walk throws and fails the test.
| // A private network namespace with IPv6 turned off on lo: bind(::1) fails | ||
| // with EADDRNOTAVAIL and every listener falls back to 127.0.0.1. | ||
| const canDisableIPv6 = | ||
| isLinux && | ||
| Bun.which("unshare") !== null && | ||
| Bun.spawnSync({ | ||
| cmd: ["unshare", "-Urn", "sh", "-c", "ip link set lo up && sysctl -qw net.ipv6.conf.lo.disable_ipv6=1"], | ||
| stdout: "ignore", | ||
| stderr: "ignore", | ||
| }).exitCode === 0; | ||
|
|
||
| test.skipIf(!canDisableIPv6)("every listener falls back to 127.0.0.1 when IPv6 is disabled on lo", async () => { | ||
| const out = await runProbe([ | ||
| "unshare", | ||
| "-Urn", | ||
| "sh", | ||
| "-c", | ||
| 'ip link set lo up && sysctl -qw net.ipv6.conf.lo.disable_ipv6=1 && exec "$@"', | ||
| "sh", | ||
| ]); | ||
| expectBound(out, "127.0.0.1"); | ||
| }); |
There was a problem hiding this comment.
🟡 (optional) Maintainers get no test for the EADDRNOTAVAIL/EAFNOSUPPORT fallthrough: deleting those two clauses from bsd_bind_should_try_next breaks no test. In the IPv6-disabled namespace glibc and musl apply RFC 6724 rule 1 (unusable destinations last), so getaddrinfo("localhost") returns 127.0.0.1 first and bind(::1) is never attempted; the test at serve-listen.test.ts:395 passes by ordering alone, not as its comment at line 384 says. Fix: a fixture whose first getaddrinfo entry is unbindable, with the order asserted by getent ahosts in the capability probe and expect(out.lookup[0]).toBe("::1") in the test as line 380 does, so removing the clause at bsd.c:1178 fails a test.
Why this was flagged
The capability probe at test/js/bun/http/serve-listen.test.ts:386-393 only checks that sysctl net.ipv6.conf.lo.disable_ipv6=1 succeeds, and the test at lines 395-405 asserts expectBound(out, "127.0.0.1") without asserting out.lookup[0], unlike the gai.conf test at line 380. With disable_ipv6=1 the kernel removes ::1 and its local route from lo, so the UDP connect() glibc uses in its RFC 6724 sort fails for ::1 and getaddrinfo("localhost") returns 127.0.0.1 before ::1; musl's sort does the same. The walks in packages/bun-usockets/src/bsd.c:1369, 1432 and 1802 therefore bind 127.0.0.1 on the first entry and never reach the bind(::1) EADDRNOTAVAIL path. The only code that consumes EADDRNOTAVAIL/EAFNOSUPPORT is bsd_bind_should_try_next at bsd.c:1177-1179; with || err == LIBUS_EAFNOSUPPORT || err == LIBUS_EADDRNOTAVAIL deleted, every added test still passes. The test still fails on the base binary because the old UDP walk bound the first AF_INET6 entry with no fallback, so it is not vacuous, but the fallthrough clause is unexercised and a later regression of it goes unnoticed.
Verification: nit — triggered whenever the suite runs on Linux glibc/musl. With net.ipv6.conf.lo.disable_ipv6=1 getaddrinfo("localhost") returns 127.0.0.1 first, so the new code never calls bind(::1). The test at test/js/bun/http/serve-listen.test.ts:395-405 asserts only expectBound(out, "127.0.0.1"), so deleting the EADDRNOTAVAIL/EAFNOSUPPORT clauses from bsd_bind_should_try_next breaks no test.
| bsd_close_socket(listenFd); | ||
| if (!bsd_bind_should_try_next(host, *error)) { |
There was a problem hiding this comment.
🟡 (optional) Users whose first loopback entry fails bind() for any reason other than the two allowed errnos now get a listen error where the base silently bound the next family. bsd_bind_should_try_next at bsd.c:1180 lets a named host move on only for EAFNOSUPPORT or EADDRNOTAVAIL; every other code stops the walk at bsd.c:1444 and bsd.c:1391. The new *error writes at bsd.c:1251 and bsd.c:1268 feed that check too, so a failing IPV6_V6ONLY setsockopt or bsd_set_reuse on the AF_INET6 entry now ends the walk. Fix: stop the walk only for the port-conflict codes (EADDRINUSE, and WSAEACCES on Windows) and keep trying the remaining entries for everything else, so sandbox and setsockopt failures on one family still reach the other, as on the base.
Why this was flagged
Trigger: listen(port, "localhost") where getaddrinfo lists ::1 first and the ::1 attempt fails with an errno outside {EAFNOSUPPORT, EADDRNOTAVAIL}. Examples reachable from Bun.listen/Bun.serve/node:net: a macOS sandbox-exec profile that allows network-bind only for ip4 (bind on the AF_INET6 socket returns EPERM), or a container whose IPV6_V6ONLY setsockopt fails at bsd.c:1267. On the base, bsd_create_listen_socket closed the fd and went on to the AF_INET loop on any bsd_bind_listen_fd failure, and the IPV6_V6ONLY path returned without writing *error, so 127.0.0.1 was bound silently. The new code sets *error at bsd.c:1251/1268/1094, then bsd_bind_should_try_next(host, *error) at bsd.c:1444 returns 0 for EPERM/EACCES/ENOPROTOOPT and the loop breaks; the same break exists at bsd.c:1391 for the cluster path. The caller gets a thrown listen error (Listener.rs:538) or Bun.serve's errno-based error instead of a working 127.0.0.1 listener. Remedy: stop only on EADDRINUSE (and WSAEACCES on Windows), continue on all other errors.
Verification: bsd_bind_should_try_next at bsd.c:1177-1179 is host == NULL || err == LIBUS_EAFNOSUPPORT || err == LIBUS_EADDRNOTAVAIL. In bsd_create_listen_socket the walk does if (!bsd_bind_should_try_next(host, *error)) { break; } at bsd.c:1443-1446. On the base, the AF_INET6 loop fell through to the AF_INET loop unconditionally, so any ::1 failure previously ended with a working 127.0.0.1 listener; now the listen call reports the error.
| test("every listener binds the first address dns.lookup reports", async () => { | ||
| const out = await runProbe([]); | ||
| expectBound(out, out.lookup[0]); |
There was a problem hiding this comment.
🟡 (optional) macOS CI gets a failing ungated test on any Mac whose libc getaddrinfo does not put ::1 first for localhost, because the test's reference order comes from a different resolver than the listen path. dns.lookup on macOS goes to dns_sd (dns.rs:5141-5144) and take_results force-sorts AF_INET6 first at dns_sd.rs:306-309; the listeners bind libc getaddrinfo order via bsd.c:1150. The assertion at serve-listen.test.ts:353 compares the two and has no platform gate. Fix: derive the expected address from the same resolver the listen path uses (a libc getaddrinfo probe, or skip the test where dns.lookup and getaddrinfo are known to differ), so the contract test only asserts what it controls on every platform.
Why this was flagged
Trigger: the test at serve-listen.test.ts:351 runs on macOS (no skipIf) and the Mac's libinfo getaddrinfo("localhost", AI_PASSIVE) returns 127.0.0.1 before ::1. out.lookup at :312 is produced by dns_sd::lookup (dns.rs:5144), whose take_results sorts every AF_INET6 answer ahead of AF_INET at dns_sd.rs:307-309 regardless of system order, so out.lookup[0] is ::1 whenever an AAAA arrives. The four listeners in the probe bind bsd_listen_getaddrinfo's first entry (bsd.c:1150, resolver order for a named host), which is 127.0.0.1 on such a Mac. expectBound(out, out.lookup[0]) at :353 then fails on serve/listen/net/udp. The base branch had no such test. The test is ungated and the two resolvers are independent code paths, so any divergence on a CI Mac fails the suite. Remedy: compute the expected address with the same resolver as the listen path or gate the test on macOS.
Verification: Triggered on any macOS host where libinfo getaddrinfo("localhost", AI_PASSIVE) does not return ::1 first. The test at serve-listen.test.ts:351-353 runs ungated. On macOS the lookup is served by dns_sd::lookup (dns.rs:5141-5144) and take_results force-sorts AF_INET6 ahead of AF_INET (dns_sd.rs:306-309). The listeners bind the first entry of libc getaddrinfo() in bsd_listen_getaddrinfo (bsd.c:1158).
| const gaiConf = join(tmpdirSync(), "gai.conf"); | ||
| writeFileSync(gaiConf, "precedence ::ffff:0:0/96 100\n"); |
There was a problem hiding this comment.
🟡 nit (optional): the new gai.conf fixture is created with tmpdirSync plus a module-scope writeFileSync, which the root CLAUDE.md (line 101) says not to use; tests should use tempDir from "harness". Fix: build the fixture with tempDir("gai", { "gai.conf": "precedence ::ffff:0:0/96 100\n" }) and pass join(String(dir), "gai.conf") to the unshare probe and the test, so the directory is created and disposed through the harness like the other tests in the suite. [also at: test/js/bun/http/serve-listen.test.ts:358 - nit: the new gai.conf fixture is created with tmpdirSync plus writeFileSync at describe scope, which the root CLAUDE.md asks tests not to do, and it runs on every platform even though only the Linux unshare test reads it.]
Why this was flagged
test/js/bun/http/serve-listen.test.ts:359 calls tmpdirSync() and test/js/bun/http/serve-listen.test.ts:360 writes gai.conf into it at describe-body scope. CLAUDE.md line 101 states: use tempDir from harness, do not use tmpdirSync or fs.mkdtempSync to create temporary directories. The file already had one pre-existing tmpdirSync use at line 13, but the diff adds a second, so the convention slip is widened rather than fixed. Consequence is maintainability only: the directory is never cleaned up and the test deviates from the documented harness pattern; no runtime behaviour differs from the base branch.
Verification: nit. Triggering condition: whenever the file is loaded (the fixture is built unconditionally at describe-body scope). The diff adds const gaiConf = join(tmpdirSync(), "gai.conf"); at /home/claude/bun/test/js/bun/http/serve-listen.test.ts:359. /home/claude/bun/CLAUDE.md:101 states: "Do not use tmpdirSync or fs.mkdtempSync to create temporary directories." Nothing functionally breaks.
Fixes #47172, fixes #15560
Problem
listen(port, "localhost")binds::1even when the resolver puts127.0.0.1first, as the reporter's Windows machine does. Node binds127.0.0.1, socurl http://127.0.0.1:<port>/reaches Node and notbun ./index.html.bsd_create_listen_socket(packages/bun-usockets/src/bsd.c:1360),bsd_create_bound_socketandbsd_create_udp_socketwalk everyAF_INET6result before anyAF_INETresult.::1:Ptaken,listen(P, "localhost")binds127.0.0.1:Pin silence.Fix
AF_INET6first, so the default listener stays on::and still falls back to0.0.0.0on any error, as before.bind()failure moves to the next entry only forEADDRNOTAVAILorEAFNOSUPPORT(IPv6 disabled onlo, fix(serve): When IPv6 is not enabled, attempt to bind to IPv4 address under the same hostname. #6533). Every other error is reported.IPV6_V6ONLYfails.socket()errors now reach*errorin the TCP walks.test/js/bun/http/serve-listen.test.ts,test/js/node/net/node-net-server.test.tsandtest/js/node/net/server.spec.ts. 5 fail on the released binary.Background
getaddrinfo(3)returns a chain the system has sorted (gai.conf on glibc, the prefix policy on Windows). Node'snet.Server.listen(port, host)runsdns.lookup(host)and binds its first entry only.bsd.cis the one native sink forBun.serve,Bun.listen,node:net,bun ./index.html, cluster andBun.udpSocket. Adns.lookuphop innode:netwas considered: it leavesBun.serveon::1.bsd.chunk of usockets: stop the listen addrinfo loop on EADDRINUSE so listen(port, "localhost") reports the conflict #36710. node:net: fall back to 0.0.0.0 when listen() gets no host and the kernel has no IPv6 #43201 (node:net::with no IPv6 stack) is a separate path and stays. net, tls: consolidate the open node:net, node:tls, Bun.connect, Bun.listen and usockets pull requests #46522 rewrites the same loop IPv6-first for named hosts and would need this walk.Downsides
listen(P, "localhost")now fails withEADDRINUSEwhere it used to bind the other loopback family, as in Node.localhostlistener moves from::1to127.0.0.1. A client that connects to[::1]by literal is refused, as with Node.socket/bind/listenunchanged at 1/1/1 in both resolver orders (LD_PRELOAD counter, main vs PR).bsd.c-O2text 11748 -> 11692 bytes.Notes
Repro on Linux with the reporter's resolver order:
Stock Windows and glibc both put
::1first forlocalhost. The reporter's machine prefers IPv4 (a prefix policy orDisabledComponentssetting). Node and Bun then disagree, and this change makes them agree. On hosts with the default order nothing moves.Tests. "every listener binds the first address dns.lookup reports" runs on every platform and pins the contract:
Bun.serve,Bun.listen,net.createServer().listenandBun.udpSocketonlocalhostbinddns.lookup("localhost", { order: "verbatim" })[0]. Two namespace tests make the order bug deterministic on Linux: one bind-mounts an IPv4-firstgai.confinsideunshare -Urm, one disables IPv6 onloinsideunshare -Urn. They skip where user namespaces are off. The EADDRINUSE tests need no namespace and fail on the released binary everywhere both loopback families exist. "falls back to 0.0.0.0 when only [::1]:port is taken" pins the unchanged wildcard fallback (Linux refusesbind(::)there, other platforms allow it). Theserver.spec.tstest was skipped with "FIXME: hostname is not resolved" and now runs with port 0 and the resolved address.Before this change
Bun.udpSocket({ hostname: "localhost" })failed withbind EADDRNOTAVAILwhen IPv6 was disabled onlo: the UDP walk picked the firstAF_INET6entry and bound once, with no fallback.The
EADDRNOTAVAILfallthrough is per address, not per family, and is broader than Node: for a host whose first record is not local (a stale AAAA record) Node reportsEADDRNOTAVAILand Bun binds the next record. The TCP walks did that before this change too. UDP now does the same, so the TCP and QUIC halves of oneBun.servepick the same address.Wildcard:
Bun.serve({ port })with[::1]:portheld still binds0.0.0.0, as in 1.4.x and as #46522 documents ("without a hostname nothing changes"). Node reportsEADDRINUSEthere. OnlyBun.servepasses a null host tobsd.c;node:netpasses the literal"::"(#43201 handles that path) andBun.listenrequires a hostname.Syscall counts: an
LD_PRELOADshim that countssocket/bind/listen(AF_INET/AF_INET6 only), run on-escripts forBun.serve,Bun.listenandBun.udpSocketwithhostname: "localhost"and for the wildcard, on the main and PR debug builds, with the default and the IPv4-firstgai.conf. Every cell is 1 socket, 1 bind, 1 listen (0 listen for UDP).straceandbloatyare not installed in this environment. Size issizeonbsd.ccompiled withclang -O2from main and from this branch.dns.setDefaultResultOrder("ipv4first")and--dns-result-orderstill do not move anode:netlistener: the listen path does not go throughdns.lookup. That is a separate node-compat gap insrc/js/node/net.tsand is not changed here.Suites run on the debug build:
serve-listen.test.ts,server.spec.ts,node-net-server.test.ts,serve.test.ts,udp_socket.test.ts,test/js/node/cluster,node-net.test.ts,node-http.test.ts,localhost-loopback-contract.test.ts,test/js/node/dgram. Pre-existing failures in this container, the same on main:serve.test.ts"root range port" (runs as root) and "abort a sendfile response" (timeout),node-http.test.ts"a dispatch that throws while an earlier response is pending" (timeouts), a fewnode-net.test.tssubprocess tests (no network, timeouts).Self-reviewed: 10 concerns raised, 8 addressed (wildcard policy kept and pinned,
socket()error reported, comment no longer claims a Windows order, ungated contract test, gai.conf test asserts only what it controls and skips instead of returning,server.spec.tsun-skipped, #36710/#43201/#46522 named). Not taken: narrow theEADDRNOTAVAILfallthrough to the IPv6 loopback entry (kept the existing per-address behaviour and documented it), and land this as a commit on #46522 instead of a PR (a sequencing call for the maintainer).