From 18ff1624dcfab55ebc6deed27b24f847c11f2a30 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:31:36 +0000 Subject: [PATCH 1/2] dns: refuse setServers() ports above 65535 node:dns setServers() took a port above 65535 modulo 65536, and modulo 2**32 before that: "127.0.0.1:99999" set port 34463. Node v26.12.1 throws ERR_SOCKET_BAD_PORT (nodejs/node#65021). setServersOn() now calls validatePort() in both parse arms, as node does, so the order of the checks is node's. --- src/js/node/dns.ts | 4 +- test/js/node/dns/dns-resolver-class.test.ts | 154 ++++++++++++++++++++ test/js/node/test/parallel/test-dns.js | 20 +++ 3 files changed, 177 insertions(+), 1 deletion(-) diff --git a/src/js/node/dns.ts b/src/js/node/dns.ts index 2c136ed2128b..e9df1465e395 100644 --- a/src/js/node/dns.ts +++ b/src/js/node/dns.ts @@ -158,6 +158,7 @@ function setServersOn(servers: readonly string[], object: { setServers(servers: ipVersion = isIP(match[1]); if (ipVersion !== 0) { const port = parseInt(addrSplitRE[Symbol.replace](server, "$2")) || IANA_DNS_PORT; + validatePort(port); triples.push([ipVersion, stripZoneId(match[1]), port]); return; } @@ -168,11 +169,12 @@ function setServersOn(servers: readonly string[], object: { setServers(servers: if (addrSplitMatch) { const hostIP = addrSplitMatch[1]; - const port = addrSplitMatch[2] ? parseInt(addrSplitMatch[2]) : IANA_DNS_PORT; + const port = parseInt(addrSplitMatch[2]) || IANA_DNS_PORT; ipVersion = isIP(hostIP); if (ipVersion !== 0) { + validatePort(port); triples.push([ipVersion, hostIP, port]); return; } diff --git a/test/js/node/dns/dns-resolver-class.test.ts b/test/js/node/dns/dns-resolver-class.test.ts index 5124663172ee..4b0a656728d8 100644 --- a/test/js/node/dns/dns-resolver-class.test.ts +++ b/test/js/node/dns/dns-resolver-class.test.ts @@ -1,7 +1,9 @@ // node-dns.test.js resolves public hostnames. These tests need no network. import { describe, expect, test } from "bun:test"; import { runWithError } from "harness"; +import dgram from "node:dgram"; import dns from "node:dns"; +import { once } from "node:events"; describe.each([ ["dns.Resolver", dns.Resolver], @@ -46,3 +48,155 @@ describe.each([ expect(missing).toEqual(runWithError(() => handle[method](undefined))); }); }); + +/** A UDP socket that never answers: a query sent to it stays pending until `cancel()`. */ +async function listenWithoutAnswering() { + const socket = dgram.createSocket("udp4"); + socket.bind(0, "127.0.0.1"); + await once(socket, "listening"); + return { socket, servers: ["127.0.0.1:" + socket.address().port] }; +} + +/** Starts a query of `resolver` that settles with the code of its error. */ +const pendingQueries: [string, () => { resolver: any; query: () => Promise }][] = [ + [ + "dns.Resolver", + () => { + const resolver = new dns.Resolver(); + const query = () => + new Promise(resolve => + resolver.resolve4("pending.example.test", err => resolve(err?.code)), + ); + return { resolver, query }; + }, + ], + [ + "dns.promises.Resolver", + () => { + const resolver = new dns.promises.Resolver(); + const query = () => + resolver.resolve4("pending.example.test").then( + () => undefined, + err => err.code, + ); + return { resolver, query }; + }, + ], +]; + +// https://github.com/nodejs/node/pull/65021 +describe("setServers port range", () => { + const badPort = { + name: "RangeError", + code: "ERR_SOCKET_BAD_PORT", + message: expect.stringMatching(/^Port should be >= 0 and < 65536\. Received /), + }; + + type Target = { setServers(servers: readonly unknown[]): void; getServers(): string[] }; + describe.each<[string, () => Target]>([ + ["dns", () => dns as Target], + ["dns.promises", () => dns.promises as Target], + ["dns.Resolver", () => new dns.Resolver() as Target], + ["dns.promises.Resolver", () => new dns.promises.Resolver() as Target], + // With a receiver that is no Resolver, the method acts on the default resolver. + [ + "dns.Resolver.prototype.setServers.call(undefined)", + () => ({ + setServers: servers => dns.Resolver.prototype.setServers.call(undefined, servers as string[]), + getServers: () => dns.getServers(), + }), + ], + ])("%s", (_name, make) => { + // `dns` and `dns.promises` set the servers of the default resolver, which the whole process shares: put them back. + function withResolver(fn: (resolver: Target, original: string[]) => void) { + const resolver = make(); + const original = resolver.getServers(); + try { + fn(resolver, original); + } finally { + resolver.setServers(original); + } + } + + // Before the check, 65536 and 2**32 + 53 were the default port, and 99999 was 34463. + test.each([ + ["127.0.0.1:65536", "127.0.0.1:65536"], + ["127.0.0.1:99999", "127.0.0.1:99999"], + ["127.0.0.1:2**32+53", `127.0.0.1:${2 ** 32 + 53}`], + ["127.0.0.1:2**64", "127.0.0.1:18446744073709551616"], + ["127.0.0.1:<400 digits>", `127.0.0.1:${Buffer.alloc(400, "9")}`], + ["[::1]:65536", "[::1]:65536"], + ["[::1]:99999", "[::1]:99999"], + ["[::1]:2**32+53", `[::1]:${2 ** 32 + 53}`], + ["[::1]:<400 digits>", `[::1]:${Buffer.alloc(400, "9")}`], + ["[fe80::1%lo]:99999", "[fe80::1%lo]:99999"], + ])("%s throws ERR_SOCKET_BAD_PORT and leaves the servers as they were", (_label, server) => { + withResolver((resolver, original) => { + expect(runWithError(() => resolver.setServers([server]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["192.0.2.2", server]))).toMatchObject(badPort); + expect(resolver.getServers()).toEqual(original); + }); + }); + + test("accepts the ports 0 to 65535", () => { + withResolver(resolver => { + resolver.setServers([ + "127.0.0.1:65535", + "[::1]:65535", + "127.0.0.2:0", + "[::2]:0", + "127.0.0.3:00053", + "127.0.0.4:000000000000099", + ]); + expect(resolver.getServers()).toEqual([ + "127.0.0.1:65535", + "[::1]:65535", + "127.0.0.2", + "::2", + "127.0.0.3", + "127.0.0.4:99", + ]); + }); + }); + + test("checks the servers in order, and the address of a server before its port", () => { + withResolver((resolver, original) => { + const invalidAddress = (server: string) => ({ + name: "TypeError", + code: "ERR_INVALID_IP_ADDRESS", + message: `Invalid IP address: ${server}`, + }); + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", "nope"]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", 5]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["nope", "1.2.3.4:65536"]))).toMatchObject( + invalidAddress("nope"), + ); + expect(runWithError(() => resolver.setServers(["nope:65536"]))).toMatchObject(invalidAddress("nope:65536")); + expect(runWithError(() => resolver.setServers(["[nope]:65536"]))).toMatchObject(invalidAddress("[nope]:65536")); + expect(resolver.getServers()).toEqual(original); + }); + }); + }); + + test.concurrent.each(pendingQueries)("%s with a pending query reports the port first", async (_name, make) => { + const { socket, servers } = await listenWithoutAnswering(); + const { resolver, query } = make(); + try { + resolver.setServers(servers); + const received = once(socket, "message"); + const settled = query(); + await received; + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536"]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["1.2.3.4"]))).toMatchObject({ + code: "ERR_DNS_SET_SERVERS_FAILED", + }); + expect(resolver.getServers()).toEqual(servers); + resolver.cancel(); + expect(await settled).toBe("ECANCELLED"); + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536"]))).toMatchObject(badPort); + } finally { + resolver.cancel(); + socket.close(); + } + }); +}); diff --git a/test/js/node/test/parallel/test-dns.js b/test/js/node/test/parallel/test-dns.js index 8c2b0f8e480e..d4fde4ba5b55 100644 --- a/test/js/node/test/parallel/test-dns.js +++ b/test/js/node/test/parallel/test-dns.js @@ -90,6 +90,22 @@ assert(existing.length > 0); }); } +{ + // Out-of-range ports, which should throw a clean error. + const invalidPorts = [2 ** 16, 2 ** 32, 2 ** 64]; + invalidPorts.forEach((port) => { + assert.throws( + () => { + dns.setServers([`1.2.3.4:${port}`]); + }, + { + name: 'RangeError', + code: 'ERR_SOCKET_BAD_PORT' + } + ); + }); +} + const goog = [ '8.8.8.8', '8.8.4.4', @@ -136,6 +152,10 @@ const portsExpected = [ dns.setServers(ports); assert.deepStrictEqual(dns.getServers(), portsExpected); +// Port 0 means "use the default port" for c-ares. +dns.setServers(['4.4.4.4:0', '[2001:4860:4860::8888]:0']); +assert.deepStrictEqual(dns.getServers(), ['4.4.4.4', '2001:4860:4860::8888']); + dns.setServers([]); assert.deepStrictEqual(dns.getServers(), []); From b932ed4cd4fcb4bca8e42aa6732d7cd61096b24b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 29 Oct 2026 05:52:54 +0000 Subject: [PATCH 2/2] test(dns): check the default resolver's setServers() in a child process The tests set the servers of the default resolver and put them back from getServers(). That list drops the zone id of a link-local server, so on such a host the rest of the process ran with other servers. The three forms that act on the default resolver now run in a child process, and the full list of ports runs on Resolver instances. --- test/js/node/dns/dns-resolver-class.test.ts | 154 ++++++++++++-------- 1 file changed, 90 insertions(+), 64 deletions(-) diff --git a/test/js/node/dns/dns-resolver-class.test.ts b/test/js/node/dns/dns-resolver-class.test.ts index 4b0a656728d8..fb90fec94355 100644 --- a/test/js/node/dns/dns-resolver-class.test.ts +++ b/test/js/node/dns/dns-resolver-class.test.ts @@ -1,6 +1,6 @@ // node-dns.test.js resolves public hostnames. These tests need no network. import { describe, expect, test } from "bun:test"; -import { runWithError } from "harness"; +import { bunEnv, bunExe, runWithError } from "harness"; import dgram from "node:dgram"; import dns from "node:dns"; import { once } from "node:events"; @@ -92,32 +92,10 @@ describe("setServers port range", () => { message: expect.stringMatching(/^Port should be >= 0 and < 65536\. Received /), }; - type Target = { setServers(servers: readonly unknown[]): void; getServers(): string[] }; - describe.each<[string, () => Target]>([ - ["dns", () => dns as Target], - ["dns.promises", () => dns.promises as Target], - ["dns.Resolver", () => new dns.Resolver() as Target], - ["dns.promises.Resolver", () => new dns.promises.Resolver() as Target], - // With a receiver that is no Resolver, the method acts on the default resolver. - [ - "dns.Resolver.prototype.setServers.call(undefined)", - () => ({ - setServers: servers => dns.Resolver.prototype.setServers.call(undefined, servers as string[]), - getServers: () => dns.getServers(), - }), - ], - ])("%s", (_name, make) => { - // `dns` and `dns.promises` set the servers of the default resolver, which the whole process shares: put them back. - function withResolver(fn: (resolver: Target, original: string[]) => void) { - const resolver = make(); - const original = resolver.getServers(); - try { - fn(resolver, original); - } finally { - resolver.setServers(original); - } - } - + describe.each([ + ["dns.Resolver", dns.Resolver], + ["dns.promises.Resolver", dns.promises.Resolver], + ])("%s", (_name, Resolver: any) => { // Before the check, 65536 and 2**32 + 53 were the default port, and 99999 was 34463. test.each([ ["127.0.0.1:65536", "127.0.0.1:65536"], @@ -131,50 +109,47 @@ describe("setServers port range", () => { ["[::1]:<400 digits>", `[::1]:${Buffer.alloc(400, "9")}`], ["[fe80::1%lo]:99999", "[fe80::1%lo]:99999"], ])("%s throws ERR_SOCKET_BAD_PORT and leaves the servers as they were", (_label, server) => { - withResolver((resolver, original) => { - expect(runWithError(() => resolver.setServers([server]))).toMatchObject(badPort); - expect(runWithError(() => resolver.setServers(["192.0.2.2", server]))).toMatchObject(badPort); - expect(resolver.getServers()).toEqual(original); - }); + const resolver = new Resolver(); + resolver.setServers(["192.0.2.1"]); + expect(runWithError(() => resolver.setServers([server]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["192.0.2.2", server]))).toMatchObject(badPort); + expect(resolver.getServers()).toEqual(["192.0.2.1"]); }); test("accepts the ports 0 to 65535", () => { - withResolver(resolver => { - resolver.setServers([ - "127.0.0.1:65535", - "[::1]:65535", - "127.0.0.2:0", - "[::2]:0", - "127.0.0.3:00053", - "127.0.0.4:000000000000099", - ]); - expect(resolver.getServers()).toEqual([ - "127.0.0.1:65535", - "[::1]:65535", - "127.0.0.2", - "::2", - "127.0.0.3", - "127.0.0.4:99", - ]); - }); + const resolver = new Resolver(); + resolver.setServers([ + "127.0.0.1:65535", + "[::1]:65535", + "127.0.0.2:0", + "[::2]:0", + "127.0.0.3:00053", + "127.0.0.4:000000000000099", + ]); + expect(resolver.getServers()).toEqual([ + "127.0.0.1:65535", + "[::1]:65535", + "127.0.0.2", + "::2", + "127.0.0.3", + "127.0.0.4:99", + ]); }); test("checks the servers in order, and the address of a server before its port", () => { - withResolver((resolver, original) => { - const invalidAddress = (server: string) => ({ - name: "TypeError", - code: "ERR_INVALID_IP_ADDRESS", - message: `Invalid IP address: ${server}`, - }); - expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", "nope"]))).toMatchObject(badPort); - expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", 5]))).toMatchObject(badPort); - expect(runWithError(() => resolver.setServers(["nope", "1.2.3.4:65536"]))).toMatchObject( - invalidAddress("nope"), - ); - expect(runWithError(() => resolver.setServers(["nope:65536"]))).toMatchObject(invalidAddress("nope:65536")); - expect(runWithError(() => resolver.setServers(["[nope]:65536"]))).toMatchObject(invalidAddress("[nope]:65536")); - expect(resolver.getServers()).toEqual(original); + const resolver = new Resolver(); + resolver.setServers(["192.0.2.1"]); + const invalidAddress = (server: string) => ({ + name: "TypeError", + code: "ERR_INVALID_IP_ADDRESS", + message: `Invalid IP address: ${server}`, }); + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", "nope"]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", 5]))).toMatchObject(badPort); + expect(runWithError(() => resolver.setServers(["nope", "1.2.3.4:65536"]))).toMatchObject(invalidAddress("nope")); + expect(runWithError(() => resolver.setServers(["nope:65536"]))).toMatchObject(invalidAddress("nope:65536")); + expect(runWithError(() => resolver.setServers(["[nope]:65536"]))).toMatchObject(invalidAddress("[nope]:65536")); + expect(resolver.getServers()).toEqual(["192.0.2.1"]); }); }); @@ -199,4 +174,55 @@ describe("setServers port range", () => { socket.close(); } }); + + // These set the default resolver of the whole process. getServers() drops zone ids, so nothing can put it back. + test.concurrent("dns.setServers, dns.promises.setServers and a receiver that is no Resolver", async () => { + const fixture = ` + const dns = require("node:dns"); + const forms = { + "dns.setServers": servers => dns.setServers(servers), + "dns.promises.setServers": servers => dns.promises.setServers(servers), + "Resolver.prototype.setServers.call(undefined)": servers => + dns.Resolver.prototype.setServers.call(undefined, servers), + }; + const errorOf = fn => { + try { + fn(); + } catch (error) { + return { name: error.name, code: error.code, message: error.message }; + } + }; + const results = {}; + for (const [form, setServers] of Object.entries(forms)) { + dns.setServers(["192.0.2.1"]); + const errors = { + ipv4: errorOf(() => setServers(["127.0.0.1:65536"])), + ipv6: errorOf(() => setServers(["[::1]:99999"])), + afterAnAcceptedServer: errorOf(() => setServers(["192.0.2.2", "127.0.0.1:65536"])), + beforeABadAddress: errorOf(() => setServers(["127.0.0.1:65536", "nope"])), + }; + const serversAfterTheErrors = dns.getServers(); + setServers(["127.0.0.1:65535", "[::1]:0"]); + results[form] = { ...errors, serversAfterTheErrors, accepted: dns.getServers() }; + } + console.log(JSON.stringify(results)); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const ofEachForm = { + ipv4: badPort, + ipv6: badPort, + afterAnAcceptedServer: badPort, + beforeABadAddress: badPort, + serversAfterTheErrors: ["192.0.2.1"], + accepted: ["127.0.0.1:65535", "::1"], + }; + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + "dns.setServers": ofEachForm, + "dns.promises.setServers": ofEachForm, + "Resolver.prototype.setServers.call(undefined)": ofEachForm, + }); + expect(exitCode).toBe(0); + }); });