Skip to content

dns: add setDefaultResolver and getDefaultResolver #14620

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,32 @@ added: REPLACEME
Cancel all outstanding DNS queries made by this resolver. The corresponding
callbacks will be called with an error with code `ECANCELLED`.

## dns.getDefaultResolver()
<!-- YAML
added: REPLACEME
-->

Get the default resolver which may used by Node.js modules such as `http` and
provides functions like `dns.resolve()`, `dns.setServers()`, etc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just...

Returns the default `dns.Resolver` used by Node.js.


## dns.setDefaultResolver()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

### dns.setDefaultResolver(resolver)
<!-- YAML
added: REPLACEME
-->

* `resolver` {dns.Resolver}

<!-- YAML
added: REPLACEME
-->

Set the default resolver which may used by Node.js modules such as `http` and
provides functions like `dns.resolve()`, `dns.setServers()`, etc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just...

Changes the default `Resolver` used by Node.js.


The only parameter must be a `dns.Resolver` instance that to replace the default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd omit this sentence.

one.

For example:

```js
const resolver = new dns.Resolver();
dns.setDefaultResolver(resolver);
```

## dns.getServers()
<!-- YAML
added: v0.11.3
Expand Down
53 changes: 37 additions & 16 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ const digits = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 96-111
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // 112-127
];
const DEFAULT_RESOLVER_EXPORTED_FUNCTIONS = [
'getServers',
'setServers',
'resolve',
'resolveAny',
'resolve4',
'resolve6',
'resolveCname',
'resolveMx',
'resolveNs',
'resolveTxt',
'resolveSrv',
'resolvePtr',
'resolveNaptr',
'resolveSoa',
'reverse'
];
function isIPv4(str) {
if (!digits[str.charCodeAt(0)]) return false;
if (str.length === 1) return false;
Expand Down Expand Up @@ -377,28 +394,30 @@ function setServers(servers) {
}
}

const defaultResolver = new Resolver();
let defaultResolver;

function getDefaultResolver() {
return defaultResolver;
}

function setDefaultResolver(resolver) {
if (!(resolver instanceof Resolver)) {
throw new errors.TypeError('ERR_INVALID_DNS_RESOLVER');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just reuse the existing ERR_INVALID_ARG_TYPE?

}

defaultResolver = resolver;
DEFAULT_RESOLVER_EXPORTED_FUNCTIONS.forEach((key) => {
module.exports[key] = defaultResolver[key].bind(defaultResolver);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels... hackish. Perhaps changing the module.exports properties into getters that return the equivalent method from defaultResolver would be a cleaner approach?

e.g.

module.exports = {
  /** ... **/
  get getServers() {
    return defaultResolver.getServers.bind(defaultResolver);
  }
  /** ... **/
}

That way the module's exported properties never change.

}

module.exports = {
lookup,
lookupService,

Resolver,
getServers: defaultResolver.getServers.bind(defaultResolver),
setServers: defaultResolver.setServers.bind(defaultResolver),
resolve: defaultResolver.resolve.bind(defaultResolver),
resolveAny: defaultResolver.resolveAny.bind(defaultResolver),
resolve4: defaultResolver.resolve4.bind(defaultResolver),
resolve6: defaultResolver.resolve6.bind(defaultResolver),
resolveCname: defaultResolver.resolveCname.bind(defaultResolver),
resolveMx: defaultResolver.resolveMx.bind(defaultResolver),
resolveNs: defaultResolver.resolveNs.bind(defaultResolver),
resolveTxt: defaultResolver.resolveTxt.bind(defaultResolver),
resolveSrv: defaultResolver.resolveSrv.bind(defaultResolver),
resolvePtr: defaultResolver.resolvePtr.bind(defaultResolver),
resolveNaptr: defaultResolver.resolveNaptr.bind(defaultResolver),
resolveSoa: defaultResolver.resolveSoa.bind(defaultResolver),
reverse: defaultResolver.reverse.bind(defaultResolver),
setDefaultResolver,
getDefaultResolver,

// uv_getaddrinfo flags
ADDRCONFIG: cares.AI_ADDRCONFIG,
Expand Down Expand Up @@ -430,3 +449,5 @@ module.exports = {
ADDRGETNETWORKPARAMS: 'EADDRGETNETWORKPARAMS',
CANCELLED: 'ECANCELLED'
};

setDefaultResolver(new Resolver());
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ E('ERR_INVALID_CALLBACK', 'Callback must be a function');
E('ERR_INVALID_CHAR', 'Invalid character in %s');
E('ERR_INVALID_CURSOR_POS',
'Cannot set cursor row without setting its column');
E('ERR_INVALID_DNS_RESOLVER', 'Not a DNS resolver instance');
E('ERR_INVALID_DOMAIN_NAME', 'Unable to determine the domain name');
E('ERR_INVALID_FD', '"fd" must be a positive integer: %s');
E('ERR_INVALID_FILE_URL_HOST',
Expand Down
68 changes: 68 additions & 0 deletions test/parallel/test-dns-set-default-resolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict';

const common = require('../common');
const dnstools = require('../common/dns');
const dns = require('dns');
const assert = require('assert');
const dgram = require('dgram');

const answers = [
{ type: 'A', address: '1.2.3.4', ttl: 123 },
{ type: 'AAAA', address: '::42', ttl: 123 },
{ type: 'MX', priority: 42, exchange: 'foobar.com', ttl: 124 },
{ type: 'NS', value: 'foobar.org', ttl: 457 },
{ type: 'TXT', entries: [ 'v=spf1 ~all', 'xyz' ] },
{ type: 'PTR', value: 'baz.org', ttl: 987 },
{
type: 'SOA',
nsname: 'ns1.example.com',
hostmaster: 'admin.example.com',
serial: 156696742,
refresh: 900,
retry: 900,
expire: 1800,
minttl: 60
},
];

const server = dgram.createSocket('udp4');

server.on('message', common.mustCall((msg, { address, port }) => {
const parsed = dnstools.parseDNSPacket(msg);
const domain = parsed.questions[0].domain;
assert.strictEqual(domain, 'example.org');

server.send(dnstools.writeDNSPacket({
id: parsed.id,
questions: parsed.questions,
answers: answers.map((answer) => Object.assign({ domain }, answer)),
}), port, address);
}));

server.bind(0, common.mustCall(() => {
const address = server.address();
dns.setServers([`127.0.0.1:${address.port + 1}`]);
const resolver = new dns.Resolver();
resolver.setServers([`127.0.0.1:${address.port}`]);
dns.setDefaultResolver(resolver);

dns.resolveAny('example.org', common.mustCall((err, res) => {
assert.ifError(err);

// Compare copies with ttl removed, c-ares fiddles with that value.
assert.deepStrictEqual(
res.map((r) => Object.assign({}, r, { ttl: null })),
answers.map((r) => Object.assign({}, r, { ttl: null })));
server.close();

assert.strictEqual(dns.getDefaultResolver(), resolver);

common.expectsError(function() {
dns.setDefaultResolver(123);
}, {
code: 'ERR_INVALID_DNS_RESOLVER',
type: TypeError,
message: /^Not a DNS resolver instance$/
})
}));
}));