From 563da182857a841310f8aed47e2c40d462d14141 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-order to change default dns verbatim

Allow the "--dns-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.
---
 doc/api/cli.md                                | 13 ++++
 doc/api/dns.md                                | 15 +++++
 lib/dns.js                                    | 19 +++++-
 lib/internal/dns/promises.js                  |  8 ++-
 lib/internal/dns/utils.js                     | 27 +++++++++
 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-verbatim.js            | 59 +++++++++++++++++++
 10 files changed, 246 insertions(+), 5 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-verbatim.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 055eac22f6f0a2..4c4e16b67706a9 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -181,6 +181,17 @@ 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-order=value`
+<!-- YAML
+added: REPLACEME
+-->
+
+Set the default value of `verbatim` in [`dns.lookup()`][]. The value could be:
+* `ipv4`: Set default `verbatim` `false`.
+* `verbatim`: Set default `verbatim` `true`.
+
+Otherwise, default `verbatim` will be decided by Node.js.
+
 ### `--enable-fips`
 <!-- YAML
 added: v6.0.0
@@ -1371,6 +1382,7 @@ Node.js options that are allowed are:
 * `--conditions`
 * `--diagnostic-dir`
 * `--disable-proto`
+* `--dns-order`
 * `--enable-fips`
 * `--enable-source-maps`
 * `--experimental-abortcontroller`
@@ -1728,6 +1740,7 @@ $ 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
 [`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..fe199f4701ea79 100644
--- a/doc/api/dns.md
+++ b/doc/api/dns.md
@@ -665,6 +665,20 @@ That is, if attempting to resolve with the first server provided results in a
 subsequent servers provided. Fallback DNS servers will only be used if the
 earlier ones time out or result in some other error.
 
+## `dns.verbatim`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean}
+
+By default set to `false`. Determines the default value of `verbatim` when
+it's not passed to [`dns.lookup()`][] or [`dnsPromises.lookup()`][].
+
+Configurable using the [`--dns-order`][] CLI option. Change `dns.verbatim`
+will override the [`--dns-order`][] option.
+
 ## DNS promises API
 <!-- YAML
 added: v10.6.0
@@ -1241,6 +1255,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-order`]: cli.md#cli_dns_order_value
 [`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
diff --git a/lib/dns.js b/lib/dns.js
index b75ca1ca7da082..891b59136af423 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -41,6 +41,8 @@ const {
   Resolver,
   validateHints,
   emitInvalidHostnameWarning,
+  getDefaultVerbatim,
+  setDefaultVerbatim,
 } = 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 {
@@ -335,3 +339,14 @@ ObjectDefineProperties(module.exports, {
     }
   }
 });
+
+ObjectDefineProperty(module.exports, 'verbatim', {
+  configurable: true,
+  enumerable: true,
+  get() {
+    return getDefaultVerbatim()
+  },
+  set(value) {
+    setDefaultVerbatim(value)
+  }
+});
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..aa4733cd1dd85e 100644
--- a/lib/internal/dns/utils.js
+++ b/lib/internal/dns/utils.js
@@ -13,10 +13,12 @@ const {
 
 const errors = require('internal/errors');
 const { isIP } = require('internal/net');
+const { getOptionValue } = require('internal/options');
 const {
   validateArray,
   validateInt32,
   validateString,
+  validateBoolean,
 } = require('internal/validators');
 const {
   ChannelWrap,
@@ -184,6 +186,29 @@ function emitInvalidHostnameWarning(hostname) {
   );
 }
 
+let defaultVerbatim;
+
+function getDefaultVerbatim() {
+  if (defaultVerbatim === undefined) {
+    const option = getOptionValue('--dns-order');
+    switch (option) {
+      case 'verbatim':
+        defaultVerbatim = true;
+        break;
+      case 'ipv4':
+      default:
+        defaultVerbatim = false;
+    }
+  }
+
+  return defaultVerbatim;
+}
+
+function setDefaultVerbatim(value) {
+  validateBoolean(value, 'dns.verbatim');
+  defaultVerbatim = value;
+}
+
 module.exports = {
   bindDefaultResolver,
   getDefaultResolver,
@@ -192,4 +217,6 @@ module.exports = {
   validateTimeout,
   Resolver,
   emitInvalidHostnameWarning,
+  getDefaultVerbatim,
+  setDefaultVerbatim,
 };
diff --git a/src/node_options.cc b/src/node_options.cc
index d5c2600688767b..4f233d65e0f5b9 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -297,6 +297,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             " (default: current working directory)",
             &EnvironmentOptions::diagnostic_dir,
             kAllowedInEnvironment);
+  AddOption("--dns-order",
+            "set default value of verbatim in dns.lookup. Options are "
+            "'ipv4' (IPv4 addresses are placed before IPv6 addresses) "
+            "'verbatim' (addresses are in the order the DNS resolver "
+            "returned)",
+            &EnvironmentOptions::dns_verbatim,
+            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..84d617790af2d8 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_verbatim;
   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..e2dade39a9b99d
--- /dev/null
+++ b/test/parallel/test-dns-default-verbatim-false.js
@@ -0,0 +1,51 @@
+// Flags: --expose-internals --dns-order=ipv4
+'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-order=ipv4 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..072deae806a0ac
--- /dev/null
+++ b/test/parallel/test-dns-default-verbatim-true.js
@@ -0,0 +1,51 @@
+// Flags: --expose-internals --dns-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-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-verbatim.js b/test/parallel/test-dns-verbatim.js
new file mode 100644
index 00000000000000..f5ed69f30d59d1
--- /dev/null
+++ b/test/parallel/test-dns-verbatim.js
@@ -0,0 +1,59 @@
+// 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 changing require('dns').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;
+  };
+
+  dns.verbatim = true;
+  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.verbatim = false;
+  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());