diff --git a/src/js/node/dns.ts b/src/js/node/dns.ts index 203bd87a9b5d..f680bf320c02 100644 --- a/src/js/node/dns.ts +++ b/src/js/node/dns.ts @@ -410,26 +410,35 @@ var InternalResolver = class Resolver { throw $ERR_INVALID_ARG_TYPE("rrtype", "string", rrtype); } - callback = validateResolve(hostname, callback); - - Resolver.#getResolver(this) - .resolve(hostname, rrtype) - .then( - results => { - switch (rrtype?.toLowerCase()) { - case "a": - case "aaaa": - callback(null, results.map(mapResolveX)); - break; - default: - callback(null, results); - break; - } - }, - error => { - callback(withTranslatedError(error)); - }, - ); + // $call: the module-level function runs with `this` undefined when destructured. + switch (rrtype) { + case "A": + return Resolver.prototype.resolve4.$call(this, hostname, callback); + case "AAAA": + return Resolver.prototype.resolve6.$call(this, hostname, callback); + case "ANY": + return Resolver.prototype.resolveAny.$call(this, hostname, callback); + case "CAA": + return Resolver.prototype.resolveCaa.$call(this, hostname, callback); + case "CNAME": + return Resolver.prototype.resolveCname.$call(this, hostname, callback); + case "MX": + return Resolver.prototype.resolveMx.$call(this, hostname, callback); + case "NAPTR": + return Resolver.prototype.resolveNaptr.$call(this, hostname, callback); + case "NS": + return Resolver.prototype.resolveNs.$call(this, hostname, callback); + case "PTR": + return Resolver.prototype.resolvePtr.$call(this, hostname, callback); + case "SOA": + return Resolver.prototype.resolveSoa.$call(this, hostname, callback); + case "SRV": + return Resolver.prototype.resolveSrv.$call(this, hostname, callback); + case "TXT": + return Resolver.prototype.resolveTxt.$call(this, hostname, callback); + default: + throw $ERR_INVALID_ARG_VALUE("rrtype", rrtype, "is invalid"); + } } resolve4(hostname, options, callback) { @@ -602,10 +611,7 @@ var InternalResolver = class Resolver { if (arguments.length > 2) { callback = arguments[2]; } - if (typeof callback !== "function") { - throw $ERR_INVALID_ARG_TYPE("callback", "function", callback); - } - callback = guardCallback(callback); + callback = validateResolve(hostname, callback); Resolver.#getResolver(this) .resolveCaa(hostname) @@ -623,10 +629,7 @@ var InternalResolver = class Resolver { if (arguments.length > 2) { callback = arguments[2]; } - if (typeof callback !== "function") { - throw $ERR_INVALID_ARG_TYPE("callback", "function", callback); - } - callback = guardCallback(callback); + callback = validateResolve(hostname, callback); Resolver.#getResolver(this) .resolveTxt(hostname) @@ -643,10 +646,7 @@ var InternalResolver = class Resolver { if (arguments.length > 2) { callback = arguments[2]; } - if (typeof callback !== "function") { - throw $ERR_INVALID_ARG_TYPE("callback", "function", callback); - } - callback = guardCallback(callback); + callback = validateResolve(hostname, callback); Resolver.#getResolver(this) .resolveSoa(hostname) @@ -829,61 +829,90 @@ const promises = { }, resolve(hostname, rrtype) { - if (typeof hostname !== "string") { - throw $ERR_INVALID_ARG_TYPE("hostname", "string", hostname); - } - if (typeof rrtype === "undefined") { rrtype = "A"; } else if (typeof rrtype !== "string") { throw $ERR_INVALID_ARG_TYPE("rrtype", "string", rrtype); } - switch (rrtype?.toLowerCase()) { - case "a": - case "aaaa": - return translateErrorCode(dns.resolve(hostname, rrtype).then(promisifyResolveX(false))); + switch (rrtype) { + case "A": + return promises.resolve4(hostname); + case "AAAA": + return promises.resolve6(hostname); + case "ANY": + return promises.resolveAny(hostname); + case "CAA": + return promises.resolveCaa(hostname); + case "CNAME": + return promises.resolveCname(hostname); + case "MX": + return promises.resolveMx(hostname); + case "NAPTR": + return promises.resolveNaptr(hostname); + case "NS": + return promises.resolveNs(hostname); + case "PTR": + return promises.resolvePtr(hostname); + case "SOA": + return promises.resolveSoa(hostname); + case "SRV": + return promises.resolveSrv(hostname); + case "TXT": + return promises.resolveTxt(hostname); default: - return translateErrorCode(dns.resolve(hostname, rrtype)); + throw $ERR_INVALID_ARG_VALUE("rrtype", rrtype, "is invalid"); } }, resolve4(hostname, options) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolve(hostname, "A").then(promisifyResolveX(options?.ttl))); }, resolve6(hostname, options) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolve(hostname, "AAAA").then(promisifyResolveX(options?.ttl))); }, resolveAny(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveAny(hostname)); }, resolveSrv(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveSrv(hostname)); }, resolveTxt(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveTxt(hostname)); }, resolveSoa(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveSoa(hostname)); }, resolveNaptr(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveNaptr(hostname)); }, resolveMx(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveMx(hostname)); }, resolveCaa(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveCaa(hostname)); }, resolveNs(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveNs(hostname)); }, resolvePtr(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolvePtr(hostname)); }, resolveCname(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(dns.resolveCname(hostname)); }, reverse(ip) { @@ -913,68 +942,100 @@ const promises = { if (typeof rrtype === "undefined") { rrtype = "A"; } else if (typeof rrtype !== "string") { - rrtype = null; + throw $ERR_INVALID_ARG_TYPE("rrtype", "string", rrtype); } - switch (rrtype?.toLowerCase()) { - case "a": - case "aaaa": - return translateErrorCode( - Resolver.#getResolver(this).resolve(hostname, rrtype).then(promisifyResolveX(false)), - ); + + switch (rrtype) { + case "A": + return Resolver.prototype.resolve4.$call(this, hostname); + case "AAAA": + return Resolver.prototype.resolve6.$call(this, hostname); + case "ANY": + return Resolver.prototype.resolveAny.$call(this, hostname); + case "CAA": + return Resolver.prototype.resolveCaa.$call(this, hostname); + case "CNAME": + return Resolver.prototype.resolveCname.$call(this, hostname); + case "MX": + return Resolver.prototype.resolveMx.$call(this, hostname); + case "NAPTR": + return Resolver.prototype.resolveNaptr.$call(this, hostname); + case "NS": + return Resolver.prototype.resolveNs.$call(this, hostname); + case "PTR": + return Resolver.prototype.resolvePtr.$call(this, hostname); + case "SOA": + return Resolver.prototype.resolveSoa.$call(this, hostname); + case "SRV": + return Resolver.prototype.resolveSrv.$call(this, hostname); + case "TXT": + return Resolver.prototype.resolveTxt.$call(this, hostname); default: - return translateErrorCode(Resolver.#getResolver(this).resolve(hostname, rrtype)); + throw $ERR_INVALID_ARG_VALUE("rrtype", rrtype, "is invalid"); } } resolve4(hostname, options) { + validateString(hostname, "hostname"); return translateErrorCode( Resolver.#getResolver(this).resolve(hostname, "A").then(promisifyResolveX(options?.ttl)), ); } resolve6(hostname, options) { + validateString(hostname, "hostname"); return translateErrorCode( Resolver.#getResolver(this).resolve(hostname, "AAAA").then(promisifyResolveX(options?.ttl)), ); } resolveAny(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveAny(hostname)); } resolveCname(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveCname(hostname)); } resolveMx(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveMx(hostname)); } resolveNaptr(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveNaptr(hostname)); } resolveNs(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveNs(hostname)); } resolvePtr(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolvePtr(hostname)); } resolveSoa(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveSoa(hostname)); } resolveSrv(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveSrv(hostname)); } resolveCaa(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveCaa(hostname)); } resolveTxt(hostname) { + validateString(hostname, "hostname"); return translateErrorCode(Resolver.#getResolver(this).resolveTxt(hostname)); } diff --git a/test/js/node/dns/node-dns-rrtype.test.js b/test/js/node/dns/node-dns-rrtype.test.js new file mode 100644 index 000000000000..25313b798b56 --- /dev/null +++ b/test/js/node/dns/node-dns-rrtype.test.js @@ -0,0 +1,127 @@ +import { describe, expect, it } from "bun:test"; +import * as dgram from "node:dgram"; +import * as dns from "node:dns"; +import * as dns_promises from "node:dns/promises"; +import { once } from "node:events"; + +// https://github.com/oven-sh/bun/issues/39553 +// Node validates rrtype case-sensitively: 'a' is invalid, only 'A' works. +// These tests are in their own file because they do not depend on a working +// resolver: an invalid rrtype throws before any query is issued, the dispatch +// tests at the end talk to a socket on 127.0.0.1, and the module-level +// positive checks discard their query's result. +describe.each([ + ["dns.resolve", rrtype => dns.resolve("localhost", rrtype, () => {})], + ["dns.Resolver#resolve", rrtype => new dns.Resolver().resolve("localhost", rrtype, () => {})], + ["dns.promises.resolve", rrtype => dns_promises.resolve("localhost", rrtype)], + ["dns.promises.Resolver#resolve", rrtype => new dns_promises.Resolver().resolve("localhost", rrtype)], +])("%s", (_, fn) => { + // "constructor" and "toString" would match an inherited property if the + // dispatch ever became a plain-object lookup. + it.each(["a", "aaaa", "txt", "Mx", "", "BOGUS", "constructor", "toString"])( + "with rrtype %p throws ERR_INVALID_ARG_VALUE", + rrtype => { + expect(() => fn(rrtype)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_VALUE", + message: `The argument 'rrtype' is invalid. Received '${rrtype}'`, + }), + ); + }, + ); +}); + +// Only the module-level entry points, which share the default resolver and so +// cannot be pointed at a local socket. The Resolver surfaces are covered for +// every rrtype by the QTYPE dispatch tests below, with a cancel. +describe.each([ + ["dns.resolve", rrtype => dns.resolve("localhost", rrtype, () => {})], + ["dns.promises.resolve", rrtype => dns_promises.resolve("localhost", rrtype).catch(() => {})], +])("%s", (_, fn) => { + it.each(["A", "AAAA", "ANY", "CAA", "CNAME", "MX", "NAPTR", "NS", "PTR", "SOA", "SRV", "TXT"])( + "with rrtype %p does not throw synchronously", + rrtype => { + // The query itself may fail depending on the environment's resolver. + // Only the synchronous validation is under test here. + fn(rrtype); + }, + ); +}); + +it("dns.promises.Resolver#resolve with non-string rrtype throws ERR_INVALID_ARG_TYPE", () => { + expect(() => new dns_promises.Resolver().resolve("localhost", 1)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: expect.stringContaining('The "rrtype" argument must be of type string'), + }), + ); +}); + +it.each([ + ["dns.resolve", () => dns.resolve(1, "a", () => {})], + ["dns.Resolver#resolve", () => new dns.Resolver().resolve(1, "a", () => {})], + ["dns.promises.resolve", () => dns_promises.resolve(1, "a")], + ["dns.promises.Resolver#resolve", () => new dns_promises.Resolver().resolve(1, "a")], +])("%s checks rrtype before hostname, like Node", (_, fn) => { + expect(fn).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" })); +}); + +// QTYPE of each record type, from the IANA DNS parameters registry. +const qtypes = { + A: 1, + AAAA: 28, + ANY: 255, + CAA: 257, + CNAME: 5, + MX: 15, + NAPTR: 35, + NS: 2, + PTR: 12, + SOA: 6, + SRV: 33, + TXT: 16, +}; + +// The fake server never answers. The QTYPE of the query that reaches it shows +// which query resolve(hostname, rrtype) issues. The trailing dot in the name +// keeps c-ares from also trying the host's search domains. +async function querySentBy(startQuery) { + const socket = dgram.createSocket("udp4"); + try { + socket.bind(0, "127.0.0.1"); + await once(socket, "listening"); + const received = once(socket, "message"); + const { resolver, settled } = startQuery("127.0.0.1:" + socket.address().port, "rrtype.example.test."); + const [query] = await received; + resolver.cancel(); + const { code, syscall } = await settled; + // QNAME ends at the first zero byte after the 12-byte header. QTYPE follows it. + return { qtype: query.readUInt16BE(query.indexOf(0, 12) + 1), code, syscall }; + } finally { + socket.close(); + } +} + +describe.each(Object.entries(qtypes))("resolve(hostname, %p)", (rrtype, qtype) => { + const expected = { qtype, code: "ECANCELLED", syscall: "query" + rrtype[0] + rrtype.slice(1).toLowerCase() }; + + it.concurrent("dns.Resolver#resolve issues that query", async () => { + const sent = await querySentBy((server, hostname) => { + const resolver = new dns.Resolver({ timeout: 1000, tries: 1 }); + resolver.setServers([server]); + const { promise, resolve } = Promise.withResolvers(); + resolver.resolve(hostname, rrtype, resolve); + return { resolver, settled: promise }; + }); + expect(sent).toEqual(expected); + }); + + it.concurrent("dns.promises.Resolver#resolve issues that query", async () => { + const sent = await querySentBy((server, hostname) => { + const resolver = new dns_promises.Resolver({ timeout: 1000, tries: 1 }); + resolver.setServers([server]); + return { resolver, settled: resolver.resolve(hostname, rrtype).catch(err => err) }; + }); + expect(sent).toEqual(expected); + }); +}); diff --git a/test/js/node/dns/node-dns.test.js b/test/js/node/dns/node-dns.test.js index faa2f7fe288b..538e45373173 100644 --- a/test/js/node/dns/node-dns.test.js +++ b/test/js/node/dns/node-dns.test.js @@ -1033,7 +1033,8 @@ describe("pending cache", () => { // The socket never answers. The QTYPE that reaches it and the syscall the // cancelled query reports pin resolve()'s rrtype dispatch to the query that // resolveNaptr() issues; decoding is covered by the resolveNaptr() tests. -test.concurrent.each(["NAPTR", "naptr"])("resolve(hostname, %p) issues a NAPTR query", async rrtype => { +// Only the uppercase name is valid: "naptr" throws, like in Node. +test.concurrent('resolve(hostname, "NAPTR") issues a NAPTR query', async () => { const socket = dgram.createSocket("udp4"); try { socket.bind(0, "127.0.0.1"); @@ -1041,7 +1042,7 @@ test.concurrent.each(["NAPTR", "naptr"])("resolve(hostname, %p) issues a NAPTR q const resolver = new dns_promises.Resolver(); resolver.setServers(["127.0.0.1:" + socket.address().port]); const received = once(socket, "message"); - const promise = resolver.resolve("naptr.example.test", rrtype); + const promise = resolver.resolve("naptr.example.test", "NAPTR"); const [query] = await received; // QNAME ends at the first zero byte after the 12-byte header; QTYPE follows it. expect(query.readUInt16BE(query.indexOf(0, 12) + 1)).toBe(35); diff --git a/test/js/node/test/parallel/test-c-ares.js b/test/js/node/test/parallel/test-c-ares.js index 0d32d871dc60..0ebe090f5de4 100644 --- a/test/js/node/test/parallel/test-c-ares.js +++ b/test/js/node/test/parallel/test-c-ares.js @@ -67,7 +67,7 @@ dns.lookup('::1', common.mustSucceed((result, addressType) => { const err = { code: 'ERR_INVALID_ARG_VALUE', name: 'TypeError', - // message: `The argument 'rrtype' is invalid. Received '${val}'`, + message: `The argument 'rrtype' is invalid. Received '${val}'`, }; assert.throws(