From a71df7630e63de98bd88bbbd7171a8ffbd7850ef Mon Sep 17 00:00:00 2001
From: Ouyang Yadong <oyydoibh@gmail.com>
Date: Tue, 6 Apr 2021 16:09:11 +0800
Subject: [PATCH] dns: allow `--dns-result-order` to change default dns
 verbatim

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: https://github.com/nodejs/node/pull/38099
Refs: https://github.com/nodejs/node/issues/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>
---
 doc/api/cli.md                                | 17 ++++
 doc/api/dns.md                                | 48 +++++++++-
 lib/dns.js                                    | 10 +-
 lib/internal/dns/promises.js                  |  8 +-
 lib/internal/dns/utils.js                     | 21 +++++
 src/node_options.cc                           |  7 ++
 src/node_options.h                            |  1 +
 .../test-dns-default-verbatim-false.js        | 51 ++++++++++
 .../test-dns-default-verbatim-true.js         | 51 ++++++++++
 test/parallel/test-dns-set-default-order.js   | 93 +++++++++++++++++++
 10 files changed, 298 insertions(+), 9 deletions(-)
 create mode 100644 test/parallel/test-dns-default-verbatim-false.js
 create mode 100644 test/parallel/test-dns-default-verbatim-true.js
 create mode 100644 test/parallel/test-dns-set-default-order.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 8fce8ac430da1e..23c54f2e3bfb95 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -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
@@ -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`
@@ -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
diff --git a/doc/api/dns.md b/doc/api/dns.md
index cdb948309f173f..b1f6c8212f1132 100644
--- a/doc/api/dns.md
+++ b/doc/api/dns.md
@@ -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.
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/lib/dns.js b/lib/dns.js
index b75ca1ca7da082..4eadbd04158707 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -41,6 +41,8 @@ const {
   Resolver,
   validateHints,
   emitInvalidHostnameWarning,
+  getDefaultVerbatim,
+  setDefaultResultOrder,
 } = require('internal/dns/utils');
 const {
   ERR_INVALID_ARG_TYPE,
@@ -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) {
@@ -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 {
@@ -286,6 +290,7 @@ module.exports = {
   lookupService,
 
   Resolver,
+  setDefaultResultOrder,
   setServers: defaultResolverSetServers,
 
   // uv_getaddrinfo flags
@@ -330,6 +335,7 @@ ObjectDefineProperties(module.exports, {
       if (promises === null) {
         promises = require('internal/dns/promises');
         promises.setServers = defaultResolverSetServers;
+        promises.setDefaultResultOrder = setDefaultResultOrder;
       }
       return promises;
     }
diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js
index e0158eef81307e..035e7f354efc7e 100644
--- a/lib/internal/dns/promises.js
+++ b/lib/internal/dns/promises.js
@@ -1,5 +1,4 @@
 'use strict';
-
 const {
   ArrayPrototypeMap,
   ObjectCreate,
@@ -14,6 +13,7 @@ const {
   validateHints,
   validateTimeout,
   emitInvalidHostnameWarning,
+  getDefaultVerbatim,
 } = require('internal/dns/utils');
 const { codes, dnsException } = require('internal/errors');
 const { toASCII } = require('internal/idna');
@@ -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') {
@@ -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 {
diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js
index f228ef38e270ad..4f5cdf1104c243 100644
--- a/lib/internal/dns/utils.js
+++ b/lib/internal/dns/utils.js
@@ -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 {
@@ -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,
@@ -192,4 +211,6 @@ module.exports = {
   validateTimeout,
   Resolver,
   emitInvalidHostnameWarning,
+  getDefaultVerbatim,
+  setDefaultResultOrder,
 };
diff --git a/src/node_options.cc b/src/node_options.cc
index b8809df58f0552..bf18d77d7d104b 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -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,
diff --git a/src/node_options.h b/src/node_options.h
index f20234daf7e5f8..a91dbd259784e0 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -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;
diff --git a/test/parallel/test-dns-default-verbatim-false.js b/test/parallel/test-dns-default-verbatim-false.js
new file mode 100644
index 00000000000000..06b8f66a610998
--- /dev/null
+++ b/test/parallel/test-dns-default-verbatim-false.js
@@ -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());
diff --git a/test/parallel/test-dns-default-verbatim-true.js b/test/parallel/test-dns-default-verbatim-true.js
new file mode 100644
index 00000000000000..1cdaa44a2dcb71
--- /dev/null
+++ b/test/parallel/test-dns-default-verbatim-true.js
@@ -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());
diff --git a/test/parallel/test-dns-set-default-order.js b/test/parallel/test-dns-set-default-order.js
new file mode 100644
index 00000000000000..43980c5dd8716c
--- /dev/null
+++ b/test/parallel/test-dns-set-default-order.js
@@ -0,0 +1,93 @@
+// Flags: --expose-internals
+'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.setDefaultResultOrder()` and
+// `dns.promises.setDefaultResultOrder()` 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) => {
+    //
+  });
+}
+
+assert.throws(() => dns.setDefaultResultOrder('my_order'), {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+assert.throws(() => dns.promises.setDefaultResultOrder('my_order'), {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+assert.throws(() => dns.setDefaultResultOrder(4), {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+assert.throws(() => dns.promises.setDefaultResultOrder(4), {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+(async () => {
+  let callsLength = 0;
+  const checkParameter = (expected) => {
+    assert.strictEqual(calls.length, callsLength + 1);
+    verbatim = calls[callsLength][4];
+    assert.strictEqual(verbatim, expected);
+    callsLength += 1;
+  };
+
+  dns.setDefaultResultOrder('verbatim');
+  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);
+
+  dns.setDefaultResultOrder('ipv4first');
+  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);
+
+  dns.promises.setDefaultResultOrder('verbatim');
+  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);
+
+  dns.promises.setDefaultResultOrder('ipv4first');
+  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());