Skip to content

fix(ext/node): dns resolveAny with real ANY query, retry/maxTimeout support #33684

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.

Sign up for GitHub

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

Closed
bartlomieju wants to merge 1 commit into from
Closed

fix(ext/node): dns resolveAny with real ANY query, retry/maxTimeout support #33684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion ext/net/ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,10 @@ pub enum DnsRecordData {
pub struct DnsRecordWithTtl {
#[to_v8(serde)]
pub data: DnsRecordData,
/// Record type name, populated for ANY queries to distinguish
/// untagged string variants (A vs AAAA vs NS vs PTR vs CNAME).
#[to_v8(serde)]
pub record_type: Option<String>,
pub ttl: u32,
}

Expand Down Expand Up @@ -1066,10 +1070,22 @@ pub async fn op_dns_resolve(
.records()
.iter()
.filter_map(|rec| {
let r = format_rdata(record_type)(rec.data()).transpose();
let is_any = record_type == RecordType::ANY;
// For ANY queries, use each record's actual type for formatting
let effective_type = if is_any {
rec.record_type()
} else {
record_type
};
let r = format_rdata(effective_type)(rec.data()).transpose();
r.map(|maybe_data| {
maybe_data.map(|data| DnsRecordWithTtl {
data,
record_type: if is_any {
Some(effective_type.to_string())
} else {
None
},
ttl: rec.ttl(),
})
})
Expand Down
12 changes: 11 additions & 1 deletion ext/node/polyfills/internal/dns/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,21 @@ export function validateTries(options?: { tries?: number }) {
return tries;
}

export function validateMaxTimeout(
options?: { maxTimeout?: number },
): number {
if (options?.maxTimeout === undefined) return -1; // no cap
validateInt32(options.maxTimeout, "options.maxTimeout", 0, 2 ** 31 - 1);
return options.maxTimeout;
}

export interface ResolverOptions {
timeout?: number | undefined;
/**
* @default 4
*/
tries?: number;
maxTimeout?: number | undefined;
}

/**
Expand Down Expand Up @@ -288,7 +297,8 @@ export class Resolver {
constructor(options?: ResolverOptions) {
const timeout = validateTimeout(options);
const tries = validateTries(options);
this._handle = new ChannelWrap(timeout, tries);
const maxTimeout = validateMaxTimeout(options);
this._handle = new ChannelWrap(timeout, tries, maxTimeout);
}

cancel() {
Expand Down
282 changes: 141 additions & 141 deletions ext/node/polyfills/internal_binding/cares_wrap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,16 @@ export class ChannelWrap extends AsyncWrap implements ChannelWrapQuery {
#servers: [string, number][] | null = null;
#timeout: number;
#tries: number;
#maxTimeout: number;
#pendingQueries: Set<QueryReqWrap> = new Set();
#cancelRids: Set<number> = new Set();

constructor(timeout: number, tries: number) {
constructor(timeout: number, tries: number, maxTimeout: number) {
super(providerType.DNSCHANNEL);

this.#timeout = timeout;
this.#tries = tries;
this.#maxTimeout = maxTimeout;
}

async #query(query: string, recordType: Deno.RecordType, ttl?: boolean) {
Expand Down Expand Up @@ -297,161 +299,159 @@ export class ChannelWrap extends AsyncWrap implements ChannelWrapQuery {
// deno-lint-ignore no-explicit-any
ret: any[];
}> {
let ret = [];
// deno-lint-ignore no-explicit-any
let code: any = 0;

// Always create a cancel handle so cancel() can abort in-flight ops.
const cancelRid = core.createCancelHandle();
this.#cancelRids.add(cancelRid);
let timer: ReturnType<typeof setTimeout> | undefined;

try {
if (this.#timeout >= 0) {
timer = setTimeout(() => {
this.#cancelRids.delete(cancelRid);
core.tryClose(cancelRid);
}, this.#timeout);
}
const tries = this.#tries > 0 ? this.#tries : 1;

for (let attempt = 0; attempt < tries; attempt++) {
let ret = [];
// deno-lint-ignore no-explicit-any
let code: any = 0;

// Always create a cancel handle so cancel() can abort in-flight ops.
const cancelRid = core.createCancelHandle();
this.#cancelRids.add(cancelRid);
let timer: ReturnType<typeof setTimeout> | undefined;

try {
if (this.#timeout >= 0) {
// c-ares doubles timeout on each retry, capped by maxTimeout
let currentTimeout = this.#timeout * Math.pow(2, attempt);
if (this.#maxTimeout >= 0) {
currentTimeout = Math.min(currentTimeout, this.#maxTimeout);
}
timer = setTimeout(() => {
this.#cancelRids.delete(cancelRid);
core.tryClose(cancelRid);
}, currentTimeout);
}

const res = await op_dns_resolve({
query,
recordType,
options: resolveOptions,
cancelRid,
}, /* useEdns0 */ false);
if (ttl) {
ret = res;
} else {
ret = res.map((recordWithTtl) => recordWithTtl.data);
}
} catch (e) {
if (e instanceof Deno.errors.Interrupted) {
code = "ETIMEOUT";
} else if (e instanceof Deno.errors.NotFound) {
code = codeMap.get("EAI_NODATA")!;
} else {
// TODO(cmorten): map errors to appropriate error codes.
code = codeMap.get("UNKNOWN")!;
const res = await op_dns_resolve({
query,
recordType,
options: resolveOptions,
cancelRid,
}, /* useEdns0 */ false);
if (ttl) {
ret = res;
} else {
ret = res.map((recordWithTtl) => recordWithTtl.data);
}
return { code, ret };
} catch (e) {
if (e instanceof Deno.errors.Interrupted) {
// Interrupted means explicit cancel - don't retry
code = "ETIMEOUT";
return { code, ret };
} else if (e instanceof Deno.errors.TimedOut) {
// TimedOut from hickory - retry if attempts remain
if (attempt < tries - 1) continue;
code = "ETIMEOUT";
} else if (e instanceof Deno.errors.NotFound) {
code = codeMap.get("EAI_NODATA")!;
} else {
// TODO(cmorten): map errors to appropriate error codes.
code = codeMap.get("UNKNOWN")!;
}
return { code, ret };
} finally {
if (timer !== undefined) clearTimeout(timer);
this.#cancelRids.delete(cancelRid);
core.tryClose(cancelRid);
}
} finally {
if (timer !== undefined) clearTimeout(timer);
this.#cancelRids.delete(cancelRid);
core.tryClose(cancelRid);
}

return { code, ret };
return { code: codeMap.get("UNKNOWN")!, ret: [] };
}

queryAny(req: QueryReqWrap, name: string): number {
// TODO(@bartlomieju): implemented temporary measure to allow limited usage of
// `resolveAny` like APIs.
//
// Ideally we move to using the "ANY" / "*" DNS query in future
// REF: https://github.com/denoland/deno/issues/14492
this.#pendingQueries.add(req);
(async () => {
const records: { type: Deno.RecordType; [key: string]: unknown }[] = [];

await Promise.allSettled([
this.#query(name, "A").then(({ ret }) => {
ret.forEach((record) =>
records.push({ type: "A", address: record, ttl: 0 })
);
}),
this.#query(name, "AAAA").then(({ ret }) => {
(ret as string[]).forEach((record) =>
records.push({ type: "AAAA", address: record, ttl: 0 })
);
}),
this.#query(name, "CAA").then(({ ret }) => {
(ret as Deno.CaaRecord[]).forEach(({ critical, tag, value }) =>
// deno-lint-ignore no-explicit-any
this.#query(name, "ANY" as any, true).then(({ code, ret }) => {
if (!this.#pendingQueries.has(req)) return;
this.#pendingQueries.delete(req);

if (code !== 0) {
req.oncomplete(code, []);
return;
}

const records: { type: string; [key: string]: unknown }[] = [];
for (const entry of ret) {
const data = entry?.data ?? entry;
const ttl = entry?.ttl ?? 0;
const rt = entry?.recordType;

switch (rt) {
case "A":
records.push({ type: "A", address: data, ttl });
break;
case "AAAA":
records.push({ type: "AAAA", address: data, ttl });
break;
case "MX":
records.push({
type: "MX",
priority: data.preference,
exchange: fqdnToHostname(data.exchange),
});
break;
case "NS":
records.push({ type: "NS", value: fqdnToHostname(data) });
break;
case "TXT":
records.push({ type: "TXT", entries: data });
break;
case "PTR":
records.push({ type: "PTR", value: fqdnToHostname(data) });
break;
case "SOA":
records.push({
type: "SOA",
nsname: fqdnToHostname(data.mname),
hostmaster: fqdnToHostname(data.rname),
serial: data.serial,
refresh: data.refresh,
retry: data.retry,
expire: data.expire,
minttl: data.minimum,
});
break;
case "CAA":
records.push({
type: "CAA",
[tag]: value,
critical: +critical && 128,
})
);
}),
this.#query(name, "CNAME").then(({ ret }) => {
ret.forEach((record) =>
records.push({ type: "CNAME", value: record })
);
}),
this.#query(name, "MX").then(({ ret }) => {
(ret as Deno.MxRecord[]).forEach(({ preference, exchange }) =>
[data.tag]: data.value,
critical: +data.critical && 128,
});
break;
case "CNAME":
records.push({ type: "CNAME", value: data });
break;
case "NAPTR":
records.push({
type: "MX",
priority: preference,
exchange: fqdnToHostname(exchange),
})
);
}),
this.#query(name, "NAPTR").then(({ ret }) => {
(ret as Deno.NaptrRecord[]).forEach(
({ order, preference, flags, services, regexp, replacement }) =>
records.push({
type: "NAPTR",
order,
preference,
flags,
service: services,
regexp,
replacement,
}),
);
}),
this.#query(name, "NS").then(({ ret }) => {
(ret as string[]).forEach((record) =>
records.push({ type: "NS", value: fqdnToHostname(record) })
);
}),
this.#query(name, "PTR").then(({ ret }) => {
(ret as string[]).forEach((record) =>
records.push({ type: "PTR", value: fqdnToHostname(record) })
);
}),
this.#query(name, "SOA").then(({ ret }) => {
(ret as Deno.SoaRecord[]).forEach(
({ mname, rname, serial, refresh, retry, expire, minimum }) =>
records.push({
type: "SOA",
nsname: fqdnToHostname(mname),
hostmaster: fqdnToHostname(rname),
serial,
refresh,
retry,
expire,
minttl: minimum,
}),
);
}),
this.#query(name, "SRV").then(({ ret }) => {
(ret as Deno.SrvRecord[]).forEach(
({ priority, weight, port, target }) =>
records.push({
type: "SRV",
priority,
weight,
port,
name: fqdnToHostname(target),
}),
);
}),
this.#query(name, "TXT").then(({ ret }) => {
ret.forEach((record) =>
records.push({ type: "TXT", entries: record })
);
}),
]);

if (!this.#pendingQueries.has(req)) return;
this.#pendingQueries.delete(req);
type: "NAPTR",
order: data.order,
preference: data.preference,
flags: data.flags,
service: data.services,
regexp: data.regexp,
replacement: data.replacement,
});
break;
case "SRV":
records.push({
type: "SRV",
priority: data.priority,
weight: data.weight,
port: data.port,
name: fqdnToHostname(data.target),
});
break;
}
}

const err = records.length ? 0 : codeMap.get("EAI_NODATA")!;

req.oncomplete(err, records);
})();
});

return 0;
}
Expand Down
3 changes: 3 additions & 0 deletions tests/node_compat/config.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,10 @@
"ignore": true,
"reason": "Requires --expose-internals and internalBinding('cares_wrap') which is not supported in Deno"
},
"parallel/test-dns-resolveany.js": {},
"parallel/test-dns-resolveany-bad-ancount.js": {},
"parallel/test-dns-resolvens-typeerror.js": {},
"parallel/test-dns-resolver-max-timeout.js": {},
"parallel/test-dns-resolvesrv.js": {},
"parallel/test-dns-resolvesrv-econnrefused.js": {},
"parallel/test-dns-set-default-order.js": {},
Expand Down
Loading
Loading