Skip to content

Commit

Permalink
dns: allow --dns-result-order to change default dns verbatim
Browse files Browse the repository at this point in the history
Allow the `--dns-result-order` option to change the default value
of verbatim in `dns.lookup()`. This is useful when running
Node.js in ipv6-only environments to avoid possible ENETUNREACH
errors.

PR-URL: #38099
Refs: #31566
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
oyyd authored and targos committed Jun 11, 2021
1 parent 84c3990 commit a71df76
Show file tree
Hide file tree
Showing 10 changed files with 298 additions and 9 deletions.
17 changes: 17 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,19 @@ Make built-in language features like `eval` and `new Function` that generate
code from strings throw an exception instead. This does not affect the Node.js
`vm` module.

### `--dns-result-order=order`
<!-- YAML
added: REPLACEME
-->

Set the default value of `verbatim` in [`dns.lookup()`][] and
[`dnsPromises.lookup()`][]. The value could be:
* `ipv4first`: sets default `verbatim` `false`.
* `verbatim`: sets default `verbatim` `true`.

The default is `ipv4first` and [`dns.setDefaultResultOrder()`][] have higher
priority than `--dns-result-order`.

### `--enable-fips`
<!-- YAML
added: v6.0.0
Expand Down Expand Up @@ -1377,6 +1390,7 @@ Node.js options that are allowed are:
* `--conditions`, `-C`
* `--diagnostic-dir`
* `--disable-proto`
* `--dns-result-order`
* `--enable-fips`
* `--enable-source-maps`
* `--experimental-abortcontroller`
Expand Down Expand Up @@ -1765,6 +1779,9 @@ $ node --max-old-space-size=1536 index.js
[`NODE_OPTIONS`]: #cli_node_options_options
[`NO_COLOR`]: https://no-color.org
[`SlowBuffer`]: buffer.md#buffer_class_slowbuffer
[`dns.lookup()`]: dns.md#dns_dns_lookup_hostname_options_callback
[`dns.setDefaultResultOrder()`]: dns.md#dns_dns_setdefaultresultorder_order
[`dnsPromises.lookup()`]: dns.md#dns_dnspromises_lookup_hostname_options
[`process.setUncaughtExceptionCaptureCallback()`]: process.md#process_process_setuncaughtexceptioncapturecallback_fn
[`tls.DEFAULT_MAX_VERSION`]: tls.md#tls_tls_default_max_version
[`tls.DEFAULT_MIN_VERSION`]: tls.md#tls_tls_default_min_version
Expand Down
48 changes: 44 additions & 4 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,9 @@ changes:
addresses in the order the DNS resolver returned them. When `false`,
IPv4 addresses are placed before IPv6 addresses.
**Default:** currently `false` (addresses are reordered) but this is
expected to change in the not too distant future.
New code should use `{ verbatim: true }`.
expected to change in the not too distant future. Default value is
configurable using [`dns.setDefaultResultOrder()`][] or
[`--dns-result-order`][]. New code should use `{ verbatim: true }`.
* `callback` {Function}
* `err` {Error}
* `address` {string} A string representation of an IPv4 or IPv6 address.
Expand Down Expand Up @@ -629,6 +630,23 @@ array of host names.
On error, `err` is an [`Error`][] object, where `err.code` is
one of the [DNS error codes][].

## `dns.setDefaultResultOrder(order)`
<!-- YAML
added: REPLACEME
-->

* `order` {string} must be `'ipv4first'` or `'verbatim'`.

Set the default value of `verbatim` in [`dns.lookup()`][] and
[`dnsPromises.lookup()`][]. The value could be:
* `ipv4first`: sets default `verbatim` `false`.
* `verbatim`: sets default `verbatim` `true`.

The default is `ipv4first` and [`dns.setDefaultResultOrder()`][] have higher
priority than [`--dns-result-order`][]. When using [worker threads][],
[`dns.setDefaultResultOrder()`][] from the main thread won't affect the default
dns orders in workers.

## `dns.setServers(servers)`
<!-- YAML
added: v0.11.3
Expand Down Expand Up @@ -777,8 +795,9 @@ added: v10.6.0
IPv6 addresses in the order the DNS resolver returned them. When `false`,
IPv4 addresses are placed before IPv6 addresses.
**Default:** currently `false` (addresses are reordered) but this is
expected to change in the not too distant future.
New code should use `{ verbatim: true }`.
expected to change in the not too distant future. Default value is
configurable using [`dns.setDefaultResultOrder()`][] or
[`--dns-result-order`][]. New code should use `{ verbatim: true }`.

Resolves a host name (e.g. `'nodejs.org'`) into the first found A (IPv4) or
AAAA (IPv6) record. All `option` properties are optional. If `options` is an
Expand Down Expand Up @@ -1132,6 +1151,23 @@ array of host names.
On error, the `Promise` is rejected with an [`Error`][] object, where `err.code`
is one of the [DNS error codes](#dns_error_codes).

### `dnsPromises.setDefaultResultOrder(order)`
<!-- YAML
added: REPLACEME
-->

* `order` {string} must be `'ipv4first'` or `'verbatim'`.

Set the default value of `verbatim` in [`dns.lookup()`][] and
[`dnsPromises.lookup()`][]. The value could be:
* `ipv4first`: sets default `verbatim` `false`.
* `verbatim`: sets default `verbatim` `true`.

The default is `ipv4first` and [`dnsPromises.setDefaultResultOrder()`][] have
higher priority than [`--dns-result-order`][]. When using [worker threads][],
[`dnsPromises.setDefaultResultOrder()`][] from the main thread won't affect the
default dns orders in workers.

### `dnsPromises.setServers(servers)`
<!-- YAML
added: v10.6.0
Expand Down Expand Up @@ -1241,6 +1277,7 @@ uses. For instance, _they do not use the configuration from `/etc/hosts`_.
[Implementation considerations section]: #dns_implementation_considerations
[RFC 5952]: https://tools.ietf.org/html/rfc5952#section-6
[RFC 8482]: https://tools.ietf.org/html/rfc8482
[`--dns-result-order`]: cli.md#cli_dns_result_order_order
[`Error`]: errors.md#errors_class_error
[`UV_THREADPOOL_SIZE`]: cli.md#cli_uv_threadpool_size_size
[`dgram.createSocket()`]: dgram.md#dgram_dgram_createsocket_options_callback
Expand All @@ -1260,6 +1297,7 @@ uses. For instance, _they do not use the configuration from `/etc/hosts`_.
[`dns.resolveSrv()`]: #dns_dns_resolvesrv_hostname_callback
[`dns.resolveTxt()`]: #dns_dns_resolvetxt_hostname_callback
[`dns.reverse()`]: #dns_dns_reverse_ip_callback
[`dns.setDefaultResultOrder()`]: #dns_dns_setdefaultresultorder_order
[`dns.setServers()`]: #dns_dns_setservers_servers
[`dnsPromises.getServers()`]: #dns_dnspromises_getservers
[`dnsPromises.lookup()`]: #dns_dnspromises_lookup_hostname_options
Expand All @@ -1277,7 +1315,9 @@ uses. For instance, _they do not use the configuration from `/etc/hosts`_.
[`dnsPromises.resolveSrv()`]: #dns_dnspromises_resolvesrv_hostname
[`dnsPromises.resolveTxt()`]: #dns_dnspromises_resolvetxt_hostname
[`dnsPromises.reverse()`]: #dns_dnspromises_reverse_ip
[`dnsPromises.setDefaultResultOrder()`]: #dns_dnspromises_setdefaultresultorder_order
[`dnsPromises.setServers()`]: #dns_dnspromises_setservers_servers
[`socket.connect()`]: net.md#net_socket_connect_options_connectlistener
[`util.promisify()`]: util.md#util_util_promisify_original
[supported `getaddrinfo` flags]: #dns_supported_getaddrinfo_flags
[worker threads]: worker_threads.md
10 changes: 8 additions & 2 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const {
Resolver,
validateHints,
emitInvalidHostnameWarning,
getDefaultVerbatim,
setDefaultResultOrder,
} = require('internal/dns/utils');
const {
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -96,7 +98,7 @@ function lookup(hostname, options, callback) {
let hints = 0;
let family = -1;
let all = false;
let verbatim = false;
let verbatim = getDefaultVerbatim();

// Parse arguments
if (hostname) {
Expand All @@ -113,7 +115,9 @@ function lookup(hostname, options, callback) {
hints = options.hints >>> 0;
family = options.family >>> 0;
all = options.all === true;
verbatim = options.verbatim === true;
if (typeof options.verbatim === 'boolean') {
verbatim = options.verbatim === true;
}

validateHints(hints);
} else {
Expand Down Expand Up @@ -286,6 +290,7 @@ module.exports = {
lookupService,

Resolver,
setDefaultResultOrder,
setServers: defaultResolverSetServers,

// uv_getaddrinfo flags
Expand Down Expand Up @@ -330,6 +335,7 @@ ObjectDefineProperties(module.exports, {
if (promises === null) {
promises = require('internal/dns/promises');
promises.setServers = defaultResolverSetServers;
promises.setDefaultResultOrder = setDefaultResultOrder;
}
return promises;
}
Expand Down
8 changes: 5 additions & 3 deletions lib/internal/dns/promises.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';

const {
ArrayPrototypeMap,
ObjectCreate,
Expand All @@ -14,6 +13,7 @@ const {
validateHints,
validateTimeout,
emitInvalidHostnameWarning,
getDefaultVerbatim,
} = require('internal/dns/utils');
const { codes, dnsException } = require('internal/errors');
const { toASCII } = require('internal/idna');
Expand Down Expand Up @@ -103,7 +103,7 @@ function lookup(hostname, options) {
var hints = 0;
var family = -1;
var all = false;
var verbatim = false;
var verbatim = getDefaultVerbatim();

// Parse arguments
if (hostname && typeof hostname !== 'string') {
Expand All @@ -112,7 +112,9 @@ function lookup(hostname, options) {
hints = options.hints >>> 0;
family = options.family >>> 0;
all = options.all === true;
verbatim = options.verbatim === true;
if (typeof options.verbatim === 'boolean') {
verbatim = options.verbatim === true;
}

validateHints(hints);
} else {
Expand Down
21 changes: 21 additions & 0 deletions lib/internal/dns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ const {

const errors = require('internal/errors');
const { isIP } = require('internal/net');
const { getOptionValue } = require('internal/options');
const {
validateArray,
validateInt32,
validateOneOf,
validateString,
} = require('internal/validators');
const {
Expand Down Expand Up @@ -184,6 +186,23 @@ function emitInvalidHostnameWarning(hostname) {
);
}

let dnsOrder = getOptionValue('--dns-result-order') || 'ipv4first';

function getDefaultVerbatim() {
switch (dnsOrder) {
case 'verbatim':
return true;
case 'ipv4first':
default:
return false;
}
}

function setDefaultResultOrder(value) {
validateOneOf(value, 'dnsOrder', ['verbatim', 'ipv4first']);
dnsOrder = value;
}

module.exports = {
bindDefaultResolver,
getDefaultResolver,
Expand All @@ -192,4 +211,6 @@ module.exports = {
validateTimeout,
Resolver,
emitInvalidHostnameWarning,
getDefaultVerbatim,
setDefaultResultOrder,
};
7 changes: 7 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
" (default: current working directory)",
&EnvironmentOptions::diagnostic_dir,
kAllowedInEnvironment);
AddOption("--dns-result-order",
"set default value of verbatim in dns.lookup. Options are "
"'ipv4first' (IPv4 addresses are placed before IPv6 addresses) "
"'verbatim' (addresses are in the order the DNS resolver "
"returned)",
&EnvironmentOptions::dns_result_order,
kAllowedInEnvironment);
AddOption("--enable-source-maps",
"Source Map V3 support for stack traces",
&EnvironmentOptions::enable_source_maps,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class EnvironmentOptions : public Options {
public:
bool abort_on_uncaught_exception = false;
std::vector<std::string> conditions;
std::string dns_result_order;
bool enable_source_maps = false;
bool experimental_json_modules = false;
bool experimental_modules = false;
Expand Down
51 changes: 51 additions & 0 deletions test/parallel/test-dns-default-verbatim-false.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Flags: --expose-internals --dns-result-order=ipv4first
'use strict';
const common = require('../common');
const assert = require('assert');
const { internalBinding } = require('internal/test/binding');
const cares = internalBinding('cares_wrap');
const { promisify } = require('util');

// Test that --dns-result-order=ipv4first works as expected.

const originalGetaddrinfo = cares.getaddrinfo;
const calls = [];
cares.getaddrinfo = common.mustCallAtLeast((...args) => {
calls.push(args);
originalGetaddrinfo(...args);
}, 1);

const dns = require('dns');
const dnsPromises = dns.promises;

let verbatim;

// We want to test the parameter of verbatim only so that we
// ignore possible errors here.
function allowFailed(fn) {
return fn.catch((_err) => {
//
});
}

(async () => {
let callsLength = 0;
const checkParameter = (expected) => {
assert.strictEqual(calls.length, callsLength + 1);
verbatim = calls[callsLength][4];
assert.strictEqual(verbatim, expected);
callsLength += 1;
};

await allowFailed(promisify(dns.lookup)('example.org'));
checkParameter(false);

await allowFailed(dnsPromises.lookup('example.org'));
checkParameter(false);

await allowFailed(promisify(dns.lookup)('example.org', {}));
checkParameter(false);

await allowFailed(dnsPromises.lookup('example.org', {}));
checkParameter(false);
})().then(common.mustCall());
51 changes: 51 additions & 0 deletions test/parallel/test-dns-default-verbatim-true.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Flags: --expose-internals --dns-result-order=verbatim
'use strict';
const common = require('../common');
const assert = require('assert');
const { internalBinding } = require('internal/test/binding');
const cares = internalBinding('cares_wrap');
const { promisify } = require('util');

// Test that --dns-result-order=verbatim works as expected.

const originalGetaddrinfo = cares.getaddrinfo;
const calls = [];
cares.getaddrinfo = common.mustCallAtLeast((...args) => {
calls.push(args);
originalGetaddrinfo(...args);
}, 1);

const dns = require('dns');
const dnsPromises = dns.promises;

let verbatim;

// We want to test the parameter of verbatim only so that we
// ignore possible errors here.
function allowFailed(fn) {
return fn.catch((_err) => {
//
});
}

(async () => {
let callsLength = 0;
const checkParameter = (expected) => {
assert.strictEqual(calls.length, callsLength + 1);
verbatim = calls[callsLength][4];
assert.strictEqual(verbatim, expected);
callsLength += 1;
};

await allowFailed(promisify(dns.lookup)('example.org'));
checkParameter(true);

await allowFailed(dnsPromises.lookup('example.org'));
checkParameter(true);

await allowFailed(promisify(dns.lookup)('example.org', {}));
checkParameter(true);

await allowFailed(dnsPromises.lookup('example.org', {}));
checkParameter(true);
})().then(common.mustCall());
Loading

0 comments on commit a71df76

Please sign in to comment.