Skip to content

Commit 5356403

Browse files
authored
fix(node:dns): fix three bugs in dns lookup and resolver (#6128)
1 parent edacbb5 commit 5356403

File tree

3 files changed

+127
-4
lines changed

3 files changed

+127
-4
lines changed

src/node/internal/internal_dns.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,10 @@ export function lookup(
154154
address: answer.data,
155155
family: 4,
156156
})) ?? [];
157-
const ipv6Addresses: { address: string; family: 4 }[] =
157+
const ipv6Addresses: { address: string; family: 6 }[] =
158158
ipv6Response.Answer?.map((answer) => ({
159159
address: answer.data,
160-
family: 4,
160+
family: 6,
161161
})) ?? [];
162162

163163
// No addresses found
@@ -178,10 +178,43 @@ export function lookup(
178178
.catch((error: unknown): void => {
179179
process.nextTick(callback, error);
180180
});
181+
} else if (family === 0) {
182+
// family=0, all=false: query both A and AAAA, return first result based on dnsOrder
183+
Promise.all([
184+
sendDnsRequest(hostname, 'A').catch(() => ({ Answer: [] })),
185+
sendDnsRequest(hostname, 'AAAA').catch(() => ({ Answer: [] })),
186+
])
187+
.then(([ipv4Response, ipv6Response]): void => {
188+
const ipv4 = ipv4Response.Answer?.at(0)?.data;
189+
const ipv6 = ipv6Response.Answer?.at(0)?.data;
190+
191+
if (ipv4 == null && ipv6 == null) {
192+
callback(new DnsError(hostname, errorCodes.NOTFOUND, 'queryA'));
193+
return;
194+
}
195+
196+
// Return the preferred address based on dnsOrder
197+
if (dnsOrder === 'ipv6first') {
198+
if (ipv6 != null) {
199+
callback(null, ipv6, 6);
200+
} else {
201+
callback(null, ipv4 as string, 4);
202+
}
203+
} else {
204+
if (ipv4 != null) {
205+
callback(null, ipv4, 4);
206+
} else {
207+
callback(null, ipv6 as string, 6);
208+
}
209+
}
210+
})
211+
.catch((error: unknown): void => {
212+
process.nextTick(callback, error);
213+
});
181214
} else {
182215
const requestType = family === 4 ? 'A' : 'AAAA';
183216

184-
// Single request for all other cases (including when all=true but family is specified)
217+
// Single request when family is specified (with or without all=true)
185218
sendDnsRequest(hostname, requestType)
186219
.then((json): void => {
187220
validateAnswer(json.Answer, hostname, `query${requestType}`);

src/node/internal/internal_dns_promises.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class Resolver implements dns.Resolver {
126126
return resolveNs(name);
127127
}
128128

129-
esolvePtr(name: string): Promise<string[]> {
129+
resolvePtr(name: string): Promise<string[]> {
130130
return resolvePtr(name);
131131
}
132132

src/workerd/api/node/tests/dns-nodejs-test.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,96 @@ export const getServers = {
371371
},
372372
};
373373

374+
// Regression: dns.lookup() with all:true and family:0 must return family:6 for
375+
// IPv6 addresses (was returning family:4 due to copy-paste bug).
376+
export const lookupAllFamilyZeroIPv6Family = {
377+
async test() {
378+
const results = await dnsPromises.lookup(addresses.INET_HOST, {
379+
all: true,
380+
family: 0,
381+
});
382+
ok(Array.isArray(results), 'expected array of addresses');
383+
ok(results.length > 0, 'expected at least one address');
384+
385+
for (const entry of results) {
386+
strictEqual(typeof entry.address, 'string');
387+
ok(
388+
entry.family === 4 || entry.family === 6,
389+
`family must be 4 or 6, got ${entry.family}`
390+
);
391+
}
392+
393+
// If any IPv6 addresses are returned, they must have family:6
394+
const ipv6Entries = results.filter((e) => e.address.includes(':'));
395+
for (const entry of ipv6Entries) {
396+
strictEqual(
397+
entry.family,
398+
6,
399+
`IPv6 address ${entry.address} should have family:6 but got family:${entry.family}`
400+
);
401+
}
402+
403+
// If any IPv4 addresses are returned, they must have family:4
404+
const ipv4Entries = results.filter((e) => !e.address.includes(':'));
405+
for (const entry of ipv4Entries) {
406+
strictEqual(
407+
entry.family,
408+
4,
409+
`IPv4 address ${entry.address} should have family:4 but got family:${entry.family}`
410+
);
411+
}
412+
},
413+
};
414+
415+
// Regression: dns.lookup() with default family (0) and all:false must resolve
416+
// hosts that only have A records (was only querying AAAA, failing for IPv4-only).
417+
export const lookupDefaultFamilyResolvesIPv4 = {
418+
async test() {
419+
// Default options (family:0, all:false) — should return an address
420+
const result = await dnsPromises.lookup(addresses.INET4_HOST);
421+
ok(result != null, 'expected a result');
422+
strictEqual(typeof result.address, 'string');
423+
ok(result.address.length > 0, 'expected non-empty address');
424+
ok(
425+
result.family === 4 || result.family === 6,
426+
`family must be 4 or 6, got ${result.family}`
427+
);
428+
429+
// Also verify via callback API
430+
const { promise, resolve, reject } = Promise.withResolvers();
431+
dns.lookup(addresses.INET4_HOST, (error, address, family) => {
432+
if (error) {
433+
reject(error);
434+
return;
435+
}
436+
strictEqual(typeof address, 'string');
437+
ok(address.length > 0, 'expected non-empty address from callback');
438+
ok(family === 4 || family === 6, `family must be 4 or 6, got ${family}`);
439+
resolve();
440+
});
441+
await promise;
442+
},
443+
};
444+
445+
// Regression: Resolver.resolvePtr must exist (was misspelled as 'esolvePtr').
446+
export const resolverResolvePtrExists = {
447+
async test() {
448+
const resolver = new dnsPromises.Resolver();
449+
strictEqual(
450+
typeof resolver.resolvePtr,
451+
'function',
452+
'Resolver.resolvePtr should be a function'
453+
);
454+
// Actually call it to verify it works end-to-end
455+
const result = await resolver.resolvePtr(addresses.PTR_HOST);
456+
ok(Array.isArray(result), 'expected array result');
457+
ok(result.length > 0, 'expected at least one PTR record');
458+
for (const item of result) {
459+
strictEqual(typeof item, 'string');
460+
}
461+
},
462+
};
463+
374464
// Tests are taken from
375465
// https://github.com/nodejs/node/blob/3153c8333e3a8f2015b795642def4d81ec7cd7b3/test/parallel/test-dns-lookup.js
376466
export const testDnsLookup = {

0 commit comments

Comments
 (0)