Skip to content

Commit 66674b8

Browse files
titanismtitanism
committed
feat: added validation, better ipv6 support, docs, fixed dns blackhole in tests
1 parent dcb19fc commit 66674b8

File tree

3 files changed

+83
-38
lines changed

3 files changed

+83
-38
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ Similar to the `options` argument from `new dns.promises.Resolver(options)` invo
255255
| ------------------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
256256
| `timeout` | `Number` | `5000` | Number of milliseconds for requests to timeout. |
257257
| `tries` | `Number` | `4` | Number of tries per `server` in `servers` to attempt. |
258-
| `servers` | `Set` | `new Set(['1.1.1.1', '1.0.0.1'])` | A set containing IP addresses for DNS queries. Defaults to Cloudflare's of `1.1.1.1` and `1.0.0.1`. |
258+
| `servers` | `Set` or `Array` | `new Set(['1.1.1.1', '1.0.0.1'])` | A Set or Array of [RFC 5952](https://tools.ietf.org/html/rfc5952#section-6) formatted addresses for DNS queries (matches default Node.js dns module behavior). Duplicates will be removed as this is converted to a `Set` internally. Defaults to Cloudflare's of `1.1.1.1` and `1.0.0.1`. If an `Array` is passed, then it will be converted to a `Set`. |
259259
| `undici` | `Object` | Defaults to an Object with `undici.method` and `undici.headers` properties and values below | Default options to pass to [undici](https://github.com/nodejs/undici). |
260260
| `undici.method` | `String` | Defaults to `"GET"` (must be either `"GET"` or `"POST"`). | Default HTTP method to use for DNS over HTTP ("DoH") requests. |
261261
| `undici.headers` | `Object` | Defaults to `{ 'content-type': 'application/dns-message', 'user-agent': pkg.name + "/" + pkg.version, accept: 'application/dns-message' }`. | Default HTTP headers to use for DNS over HTTP ("DoH") requests. |

‎index.js

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,31 @@ class Resolver extends dns.promises.Resolver {
283283
options
284284
);
285285

286+
// timeout must be >= 0
287+
if (!Number.isFinite(this.options.timeout) || this.options.timeout < 0)
288+
throw new Error('Timeout must be >= 0');
289+
290+
// tries must be >= 1
291+
if (!Number.isFinite(this.options.tries) || this.options.tries < 1)
292+
throw new Error('Tries must be >= 1');
293+
294+
// perform validation by re-using `setServers` method
295+
this.setServers([...this.options.servers]);
296+
297+
if (
298+
!(this.options.servers instanceof Set) ||
299+
this.options.servers.size === 0
300+
)
301+
throw new Error(
302+
'Servers must be an Array or Set with at least one server'
303+
);
304+
305+
if (!['http', 'https'].includes(this.options.protocol))
306+
throw new Error('Protocol must be http or https');
307+
308+
if (!['verbatim', 'ipv4first'].includes(this.options.dnsOrder))
309+
throw new Error('DNS order must be either verbatim or ipv4first');
310+
286311
// if `cache: false` then caching is disabled
287312
// but note that this doesn't disable `got` dnsCache which is separate
288313
// so to turn that off, you need to supply `dnsCache: undefined` in `got` object (?)
@@ -702,7 +727,7 @@ class Resolver extends dns.promises.Resolver {
702727
//
703728
async #request(
704729
pkt,
705-
ip,
730+
server,
706731
abortController,
707732
requestTimeout = this.options.timeout
708733
) {
@@ -711,7 +736,8 @@ class Resolver extends dns.promises.Resolver {
711736

712737
let localAddress;
713738
let localPort;
714-
if (isIPv4(ip)) {
739+
let url = `${this.options.protocol}://${server}/dns-query`;
740+
if (isIPv4(new URL(url).hostname)) {
715741
localAddress = this.options.ipv4;
716742
if (this.options.ipv4LocalPort) localPort = this.options.ipv4LocalPort;
717743
} else {
@@ -728,7 +754,6 @@ class Resolver extends dns.promises.Resolver {
728754
if (localPort) options.localPort = localPort;
729755

730756
// <https://github.com/hildjj/dohdec/blob/43564118c40f2127af871bdb4d40f615409d4b9c/pkg/dohdec/lib/doh.js#L117-L120>
731-
let url = `${this.options.protocol}://${ip}/dns-query`;
732757
if (this.options.undici.method === 'GET') {
733758
if (!dohdec) await pWaitFor(() => Boolean(dohdec));
734759
url += `?dns=${dohdec.DNSoverHTTPS.base64urlEncode(pkt)}`;
@@ -765,15 +790,16 @@ class Resolver extends dns.promises.Resolver {
765790
let buffer;
766791
const errors = [];
767792
// NOTE: we would have used `p-map-series` but it did not support abort/break
768-
for (const ip of this.options.servers) {
793+
const servers = [...this.options.servers];
794+
for (const server of servers) {
769795
const ipErrors = [];
770796
for (let i = 0; i < this.options.tries; i++) {
771797
try {
772798
// <https://github.com/sindresorhus/p-map-series/blob/bc1b9f5e19ed62363bff3d7dc5ecc1fd820ccb51/index.js#L1-L11>
773799
// eslint-disable-next-line no-await-in-loop
774800
const response = await this.#request(
775801
pkt,
776-
ip,
802+
server,
777803
abortController,
778804
this.options.timeout * 2 ** i
779805
);
@@ -828,17 +854,17 @@ class Resolver extends dns.promises.Resolver {
828854
// break out if we had a response
829855
if (buffer) break;
830856
if (ipErrors.length > 0) {
831-
// if the `ip` had all errors, then remove it and add to end
857+
// if the `server` had all errors, then remove it and add to end
832858
// (this ensures we don't keep retrying servers that keep timing out)
833859
// (which improves upon default c-ares behavior)
834860
if (this.options.servers.size > 1 && this.options.smartRotate) {
835861
const err = this.constructor.combineErrors([
836862
new Error('Rotating DNS servers due to issues'),
837863
...ipErrors
838864
]);
839-
this.options.logger.error(err, { ip });
840-
this.options.servers.delete(ip);
841-
this.options.servers.add(ip);
865+
this.options.logger.error(err, { server });
866+
this.options.servers.delete(server);
867+
this.options.servers.add(server);
842868
}
843869

844870
errors.push(...ipErrors);
@@ -1028,17 +1054,18 @@ class Resolver extends dns.promises.Resolver {
10281054
}
10291055

10301056
setServers(servers) {
1031-
if (
1032-
!Array.isArray(servers) ||
1033-
servers.length === 0 ||
1034-
servers.some((s) => typeof s !== 'string' || s.trim() === '' || !isIP(s))
1035-
) {
1057+
if (!Array.isArray(servers) || servers.length === 0) {
10361058
const err = new TypeError(
10371059
'The "name" argument must be an instance of Array.'
10381060
);
10391061
err.code = 'ERR_INVALID_ARG_TYPE';
10401062
}
10411063

1064+
//
1065+
// TODO: every address must be ipv4 or ipv6 (use `new URL` to parse and check)
1066+
// servers [ string ] - array of RFC 5952 formatted addresses
1067+
//
1068+
10421069
// <https://github.com/nodejs/node/blob/9bbde3d7baef584f14569ef79f116e9d288c7aaa/lib/internal/dns/utils.js#L87-L95>
10431070
this.options.servers = new Set(servers);
10441071
}

‎test/test.js

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,26 @@ const { Resolver } = dns.promises;
1212
//
1313
// NOTE: tests won't work if you're behind a VPN with DNS blackholed
1414
//
15+
test.before(async (t) => {
16+
// attempt to setServers and perform a DNS lookup
17+
const tangerine = new Tangerine();
18+
const resolver = new Resolver({ timeout: 3000, tries: 1 });
19+
resolver.setServers(tangerine.getServers());
20+
21+
t.deepEqual(resolver.getServers(), tangerine.getServers());
22+
23+
try {
24+
t.log('Testing VPN with DNS blackhole');
25+
await resolver.resolve('cloudflare.com', 'A');
26+
} catch (err) {
27+
if (err.code === dns.TIMEOUT) {
28+
t.context.isBlackholed = true;
29+
t.log('VPN with DNS blackholed detected');
30+
} else {
31+
throw err;
32+
}
33+
}
34+
});
1535

1636
// new Tangerine(options)
1737
test('instance', (t) => {
@@ -47,24 +67,22 @@ test('getServers and setServers', (t) => {
4767
t.deepEqual(tangerine.getServers(), resolver.getServers());
4868
});
4969

50-
test.todo('getServers with ::0 returns accurate response');
51-
// test('getServers with ::0 returns accurate response', (t) => {
52-
// const servers = ['1.1.1.1', '::0'];
70+
test.todo('getServers with [::0] returns accurate response');
71+
// test('getServers with [::0] returns accurate response', (t) => {
72+
// const servers = ['1.1.1.1', '[::0]'];
73+
// const tangerine = new Tangerine();
74+
// const resolver = new Resolver();
5375
// resolver.setServers(servers);
5476
// tangerine.setServers(servers);
55-
// // t.log('tangerine.getServers', tangerine.getServers());
56-
// // t.log('resolver.getServers', resolver.getServers());
5777
// t.deepEqual(tangerine.getServers(), resolver.getServers());
5878
// });
5979

6080
test('getServers with IPv6 returns accurate response', (t) => {
6181
const tangerine = new Tangerine();
6282
const resolver = new Resolver();
63-
const servers = ['1.1.1.1', '2606:4700:4700::1002'];
83+
const servers = ['1.1.1.1', '2001:db8::1:80', '[2001:db8::1]:8080'];
6484
resolver.setServers(servers);
6585
tangerine.setServers(servers);
66-
// t.log('tangerine.getServers', tangerine.getServers());
67-
// t.log('resolver.getServers', resolver.getServers());
6886
t.deepEqual(tangerine.getServers(), resolver.getServers());
6987
});
7088

@@ -230,7 +248,7 @@ for (const host of [
230248
test(`resolve("${host}")`, async (t) => {
231249
const tangerine = new Tangerine();
232250
const resolver = new Resolver();
233-
resolver.setServers(tangerine.getServers());
251+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
234252
let r1 = await tangerine.resolve(host);
235253
let r2 = await resolver.resolve(host);
236254
// see explanation below regarding this under "A" and "AAAA" in switch/case
@@ -245,7 +263,7 @@ for (const host of [
245263
const resolver = new Resolver();
246264

247265
// mirror DNS servers for accuracy (e.g. SOA)
248-
resolver.setServers(tangerine.getServers());
266+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
249267

250268
let h = host;
251269
if (type === 'SRV') {
@@ -277,7 +295,7 @@ for (const host of [
277295
test(`resolve4("${host}")`, async (t) => {
278296
const tangerine = new Tangerine();
279297
const resolver = new Resolver();
280-
resolver.setServers(tangerine.getServers());
298+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
281299
let r1;
282300
try {
283301
r1 = await tangerine.resolve4(host);
@@ -299,7 +317,7 @@ for (const host of [
299317
test(`resolve6("${host}")`, async (t) => {
300318
const tangerine = new Tangerine();
301319
const resolver = new Resolver();
302-
resolver.setServers(tangerine.getServers());
320+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
303321
let r1;
304322
try {
305323
r1 = await tangerine.resolve6(host);
@@ -321,7 +339,7 @@ for (const host of [
321339
test(`resolveAny("${host}")`, async (t) => {
322340
const tangerine = new Tangerine();
323341
const resolver = new Resolver();
324-
resolver.setServers(tangerine.getServers());
342+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
325343

326344
let r1;
327345
try {
@@ -344,7 +362,7 @@ for (const host of [
344362
test(`resolveCaa("${host}")`, async (t) => {
345363
const tangerine = new Tangerine();
346364
const resolver = new Resolver();
347-
resolver.setServers(tangerine.getServers());
365+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
348366

349367
let r1;
350368
try {
@@ -367,7 +385,7 @@ for (const host of [
367385
test(`resolveCname("${host}")`, async (t) => {
368386
const tangerine = new Tangerine();
369387
const resolver = new Resolver();
370-
resolver.setServers(tangerine.getServers());
388+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
371389

372390
let r1;
373391
try {
@@ -390,7 +408,7 @@ for (const host of [
390408
test(`resolveMx("${host}")`, async (t) => {
391409
const tangerine = new Tangerine();
392410
const resolver = new Resolver();
393-
resolver.setServers(tangerine.getServers());
411+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
394412

395413
let r1;
396414
try {
@@ -413,7 +431,7 @@ for (const host of [
413431
test(`resolveNaptr("${host}")`, async (t) => {
414432
const tangerine = new Tangerine();
415433
const resolver = new Resolver();
416-
resolver.setServers(tangerine.getServers());
434+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
417435

418436
let r1;
419437
try {
@@ -436,7 +454,7 @@ for (const host of [
436454
test(`resolveNs("${host}")`, async (t) => {
437455
const tangerine = new Tangerine();
438456
const resolver = new Resolver();
439-
resolver.setServers(tangerine.getServers());
457+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
440458

441459
let r1;
442460
try {
@@ -459,7 +477,7 @@ for (const host of [
459477
test(`resolvePtr("${host}")`, async (t) => {
460478
const tangerine = new Tangerine();
461479
const resolver = new Resolver();
462-
resolver.setServers(tangerine.getServers());
480+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
463481

464482
let r1;
465483
try {
@@ -482,7 +500,7 @@ for (const host of [
482500
test(`resolveSoa("${host}")`, async (t) => {
483501
const tangerine = new Tangerine();
484502
const resolver = new Resolver();
485-
resolver.setServers(tangerine.getServers());
503+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
486504

487505
let r1;
488506
try {
@@ -505,7 +523,7 @@ for (const host of [
505523
test(`resolveSrv("${host}")`, async (t) => {
506524
const tangerine = new Tangerine();
507525
const resolver = new Resolver();
508-
resolver.setServers(tangerine.getServers());
526+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
509527

510528
let r1;
511529
try {
@@ -528,7 +546,7 @@ for (const host of [
528546
test(`resolveTxt("${host}")`, async (t) => {
529547
const tangerine = new Tangerine();
530548
const resolver = new Resolver();
531-
resolver.setServers(tangerine.getServers());
549+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
532550

533551
let r1;
534552
try {
@@ -564,7 +582,7 @@ test('reverse', async (t) => {
564582
// returns an array of reversed hostnames from IP address
565583
const tangerine = new Tangerine();
566584
const resolver = new Resolver();
567-
resolver.setServers(tangerine.getServers());
585+
if (!t.context.isBlackholed) resolver.setServers(tangerine.getServers());
568586

569587
let r1;
570588
try {

0 commit comments

Comments
 (0)