Skip to content

Commit 8cd52f8

Browse files
committed
dns: add tests for ENODATA states
This adds new tests to codify the existing behaviour of throwing ENODATA when there are no matching DNS records. It also updates an existing test which was broken by #64355 - previously requesting DNS records for a domain which has a CNAME to another domain, neither of which contain the requested type of record, would return an empty array. Since #64355, it will instead throw ENODATA. Finally, this updates the trigger for the relevant pipeline so that the internet tests will run if the cares_wrap.cc (and various other related files) are changed, which should reduce the chance of similar changes breaking main in the future. Refs: #64355 Signed-off-by: David Evans <davidje13@users.noreply.github.com>
1 parent fe3b3b8 commit 8cd52f8

4 files changed

Lines changed: 82 additions & 7 deletions

File tree

.github/workflows/test-internet.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@ on:
1010
paths:
1111
- .github/workflows/test-internet.yml
1212
- test/internet/**
13+
- test/common/internet.js
1314
- internal/dns/**
1415
- lib/dns.js
16+
- lib/internal/dns/**
1517
- lib/net.js
18+
- src/cares_wrap.cc
19+
- src/cares_wrap.h
1620
push:
1721
branches:
1822
- main
@@ -22,9 +26,13 @@ on:
2226
paths:
2327
- .github/workflows/test-internet.yml
2428
- test/internet/**
29+
- test/common/internet.js
2530
- internal/dns/**
2631
- lib/dns.js
32+
- lib/internal/dns/**
2733
- lib/net.js
34+
- src/cares_wrap.cc
35+
- src/cares_wrap.h
2836

2937
concurrency:
3038
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}

test/common/internet.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ const addresses = {
4646
DNS4_SERVER: '8.8.8.8',
4747
// An accessible IPv4 DNS server
4848
DNS6_SERVER: '2001:4860:4860::8888',
49+
// A valid host with no records (except SOA)
50+
NO_RECORD_HOST: 'void.example.com',
51+
// A valid host with no TXT records, and a CNAME to a host with no TXT records
52+
CNAME_TO_NO_TXT_HOST: 'www.microsoft.com',
4953
};
5054

5155
for (const key of Object.keys(addresses)) {
Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
'use strict';
22
const common = require('../common');
3+
const { addresses } = require('../common/internet');
34
const assert = require('assert');
45
const dns = require('dns');
56
const dnsPromises = dns.promises;
67

7-
(async function() {
8-
const result = await dnsPromises.resolveTxt('www.microsoft.com');
9-
assert.strictEqual(result.length, 0);
10-
})().then(common.mustCall());
8+
assert.rejects(
9+
dnsPromises.resolveTxt(addresses.CNAME_TO_NO_TXT_HOST),
10+
{ code: 'ENODATA' },
11+
).then(common.mustCall());
1112

12-
dns.resolveTxt('www.microsoft.com', common.mustCall((err, records) => {
13-
assert.strictEqual(err, null);
14-
assert.strictEqual(records.length, 0);
13+
dns.resolveTxt(addresses.CNAME_TO_NO_TXT_HOST, common.mustCall((err, records) => {
14+
assert.ok(err instanceof Error);
15+
assert.strictEqual(err.code, 'ENODATA');
16+
assert.strictEqual(records, undefined);
1517
}));

test/internet/test-dns.js

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,67 @@ test(function test_resolve_failure(done) {
724724
});
725725

726726

727+
async function testEmptyRecords(method, done) {
728+
await assert.rejects(
729+
dnsPromises[method](addresses.NO_RECORD_HOST),
730+
{ code: 'ENODATA' },
731+
);
732+
733+
const req = dns[method](addresses.NO_RECORD_HOST, common.mustCall((err, result) => {
734+
assert.ok(err instanceof Error);
735+
assert.strictEqual(err.code, 'ENODATA');
736+
assert.strictEqual(result, undefined);
737+
done();
738+
}));
739+
740+
checkWrap(req);
741+
}
742+
743+
test(function test_empty_4_records(done) {
744+
testEmptyRecords('resolve4', done);
745+
});
746+
747+
test(function test_empty_6_records(done) {
748+
testEmptyRecords('resolve6', done);
749+
});
750+
751+
test(function test_empty_caa_records(done) {
752+
testEmptyRecords('resolveCaa', done);
753+
});
754+
755+
test(function test_empty_cname_records(done) {
756+
testEmptyRecords('resolveCname', done);
757+
});
758+
759+
test(function test_empty_mx_records(done) {
760+
testEmptyRecords('resolveMx', done);
761+
});
762+
763+
test(function test_empty_naptr_records(done) {
764+
testEmptyRecords('resolveNaptr', done);
765+
});
766+
767+
test(function test_empty_ns_records(done) {
768+
testEmptyRecords('resolveNs', done);
769+
});
770+
771+
test(function test_empty_ptr_records(done) {
772+
testEmptyRecords('resolvePtr', done);
773+
});
774+
775+
test(function test_empty_srv_records(done) {
776+
testEmptyRecords('resolveSrv', done);
777+
});
778+
779+
test(function test_empty_tlsa_records(done) {
780+
testEmptyRecords('resolveTlsa', done);
781+
});
782+
783+
test(function test_empty_txt_records(done) {
784+
testEmptyRecords('resolveTxt', done);
785+
});
786+
787+
727788
let getaddrinfoCallbackCalled = false;
728789

729790
console.log(`looking up ${addresses.INET4_HOST}..`);

0 commit comments

Comments
 (0)