-
Notifications
You must be signed in to change notification settings - Fork 5.1k
node:dns: validate rrtype in resolve() like Node #39556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b4cce68
ac546f4
e37b6ed
4a43c26
034d199
f017aba
4ffa6e7
a003d11
e296c5b
6243f24
9b35cf7
893c22d
b9eb935
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @robobun why you need to use $call here? just do this.resolve4 etc
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the receiver. If you prefer |
||
| 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)); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robobun how we are compensating for this 3 output differences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each case lands on the same output the removed block produced:
resolve4(hostname, callback)andresolve6(hostname, callback)call the nativeresolve(hostname, "A" | "AAAA")and, with no options, returnaddresses.map(mapResolveX). That is the oldresults.map(mapResolveX)branch.resolve(hostname, rrtype)arm and the nativeresolveXmethod both end indo_resolve_cares::<T>with the sameT(src/runtime/dns_jsc/dns.rs:5003-5030and theresolve_record_fn!list at 5305-5360, for examplestruct_ares_mx_replyfor MX,NsHostentfor NS). Same c-ares query, same record objects. That is the old pass-through branch.resolveXrejects throughwithTranslatedError, the same handler the old block used, and the callback goes throughvalidateResolve(guardCallback) in each method as before.The QTYPE dispatch tests in
node-dns-rrtype.test.jspin the query each rrtype sends. The one intentional difference:resolve(hostname, "NS" | "SOA")now accepts an empty hostname, becauseresolveNs("")andresolveSoa("")allow it (allow_empty = true) and Node does too. That is listed in the PR notes.