From 1b2749ecbe3038eab4ac12945ef8f036cb58d4bd Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Wed, 29 Jan 2020 13:12:15 +0100
Subject: [PATCH] dns: default to verbatim=true in dns.lookup()

Switch the default from `ipv4first` to `verbatim` (return them exactly
as the resolver sent them to us).

PR-URL: https://github.com/nodejs/node/pull/39987
Fixes: https://github.com/nodejs/node/issues/31566
Refs: https://github.com/nodejs/node/issues/6307
Refs: https://github.com/nodejs/node/pull/20710
Refs: https://github.com/nodejs/node/pull/38099
Co-authored-by: treysis <treysis@gmx.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
---
 doc/api/dns.md                                |  8 +++--
 lib/internal/dns/utils.js                     | 10 ++----
 test/common/inspector-helper.js               |  3 +-
 test/parallel/test-cluster-message.js         |  2 +-
 test/parallel/test-http-localaddress.js       |  1 +
 test/parallel/test-http-upgrade-client.js     |  2 +-
 test/parallel/test-http2-connect-options.js   |  2 +-
 test/parallel/test-https-localaddress.js      |  1 +
 .../parallel/test-net-connect-options-port.js | 36 ++++++++++++-------
 test/parallel/test-net-dns-lookup.js          |  6 ++--
 test/parallel/test-net-pingpong.js            |  4 +--
 test/parallel/test-net-remote-address-port.js | 18 +++++-----
 test/parallel/test-net-writable.js            |  4 +--
 test/parallel/test-tcp-wrap-listen.js         |  4 ++-
 ...imeout-removes-other-socket-unref-timer.js |  2 +-
 .../test-tls-client-getephemeralkeyinfo.js    |  2 +-
 test/parallel/test-tls-client-mindhsize.js    |  2 +-
 .../test-tls-wrap-econnreset-localaddress.js  |  1 +
 test/pummel/test-http-upload-timeout.js       |  2 +-
 test/pummel/test-net-pingpong.js              |  6 ++--
 .../test-https-connect-localport.js           |  2 +-
 test/sequential/test-inspector-open.js        |  2 +-
 .../test-net-better-error-messages-port.js    |  2 +-
 .../test-net-connect-local-error.js           |  2 ++
 24 files changed, 70 insertions(+), 54 deletions(-)

diff --git a/doc/api/dns.md b/doc/api/dns.md
index af9bb60967e..ef90e15a04d 100644
--- a/doc/api/dns.md
+++ b/doc/api/dns.md
@@ -170,6 +170,9 @@ section if a custom port is used.
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/39987
+    description: The `verbatim` options defaults to `true` now.
   - version: v8.5.0
     pr-url: https://github.com/nodejs/node/pull/14731
     description: The `verbatim` option is supported now.
@@ -190,10 +193,9 @@ changes:
   * `verbatim` {boolean} When `true`, the callback receives IPv4 and 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. Default value is
+    **Default:** `true` (addresses are reordered). Default value is
     configurable using [`dns.setDefaultResultOrder()`][] or
-    [`--dns-result-order`][]. New code should use `{ verbatim: true }`.
+    [`--dns-result-order`][].
 * `callback` {Function}
   * `err` {Error}
   * `address` {string} A string representation of an IPv4 or IPv6 address.
diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js
index f15f8c7a779..28109367e36 100644
--- a/lib/internal/dns/utils.js
+++ b/lib/internal/dns/utils.js
@@ -193,16 +193,10 @@ function emitInvalidHostnameWarning(hostname) {
   );
 }
 
-let dnsOrder = getOptionValue('--dns-result-order') || 'ipv4first';
+let dnsOrder = getOptionValue('--dns-result-order') || 'verbatim';
 
 function getDefaultVerbatim() {
-  switch (dnsOrder) {
-    case 'verbatim':
-      return true;
-    case 'ipv4first':
-    default:
-      return false;
-  }
+  return dnsOrder !== 'ipv4first';
 }
 
 function setDefaultResultOrder(value) {
diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js
index 9c98ce6252c..fec48ddcba7 100644
--- a/test/common/inspector-helper.js
+++ b/test/common/inspector-helper.js
@@ -392,7 +392,7 @@ class NodeInstance extends EventEmitter {
     console.log('[test]', `Testing ${path}`);
     const headers = hostHeaderValue ? { 'Host': hostHeaderValue } : null;
     return this.portPromise.then((port) => new Promise((resolve, reject) => {
-      const req = http.get({ host, port, path, headers }, (res) => {
+      const req = http.get({ host, port, family: 4, path, headers }, (res) => {
         let response = '';
         res.setEncoding('utf8');
         res
@@ -418,6 +418,7 @@ class NodeInstance extends EventEmitter {
     const port = await this.portPromise;
     return http.get({
       port,
+      family: 4,
       path: parseURL(devtoolsUrl).path,
       headers: {
         'Connection': 'Upgrade',
diff --git a/test/parallel/test-cluster-message.js b/test/parallel/test-cluster-message.js
index 45854c77a5a..35d6c975b28 100644
--- a/test/parallel/test-cluster-message.js
+++ b/test/parallel/test-cluster-message.js
@@ -60,7 +60,7 @@ if (cluster.isWorker) {
     maybeReply();
   });
 
-  server.listen(0, '127.0.0.1');
+  server.listen(0);
 } else if (cluster.isPrimary) {
 
   const checks = {
diff --git a/test/parallel/test-http-localaddress.js b/test/parallel/test-http-localaddress.js
index e237c3bff1a..a0e4bb80a3f 100644
--- a/test/parallel/test-http-localaddress.js
+++ b/test/parallel/test-http-localaddress.js
@@ -42,6 +42,7 @@ const server = http.createServer((req, res) => {
 server.listen(0, '127.0.0.1', () => {
   const options = { host: 'localhost',
                     port: server.address().port,
+                    family: 4,
                     path: '/',
                     method: 'GET',
                     localAddress: '127.0.0.2' };
diff --git a/test/parallel/test-http-upgrade-client.js b/test/parallel/test-http-upgrade-client.js
index ea6972a18c7..e86979fffd0 100644
--- a/test/parallel/test-http-upgrade-client.js
+++ b/test/parallel/test-http-upgrade-client.js
@@ -49,7 +49,7 @@ const server = net.createServer(function(c) {
   });
 });
 
-server.listen(0, '127.0.0.1', common.mustCall(function() {
+server.listen(0, common.mustCall(function() {
   const port = this.address().port;
   const headers = [
     {
diff --git a/test/parallel/test-http2-connect-options.js b/test/parallel/test-http2-connect-options.js
index 0c7ec807b84..233ced01697 100644
--- a/test/parallel/test-http2-connect-options.js
+++ b/test/parallel/test-http2-connect-options.js
@@ -22,7 +22,7 @@ const server = http2.createServer((req, res) => {
 });
 
 server.listen(0, '127.0.0.1', common.mustCall(() => {
-  const options = { localAddress: '127.0.0.2' };
+  const options = { localAddress: '127.0.0.2', family: 4 };
 
   const client = http2.connect(
     'http://localhost:' + server.address().port,
diff --git a/test/parallel/test-https-localaddress.js b/test/parallel/test-https-localaddress.js
index 0ac8414b2d4..0de0974dc69 100644
--- a/test/parallel/test-https-localaddress.js
+++ b/test/parallel/test-https-localaddress.js
@@ -52,6 +52,7 @@ server.listen(0, '127.0.0.1', function() {
   const options = {
     host: 'localhost',
     port: this.address().port,
+    family: 4,
     path: '/',
     method: 'GET',
     localAddress: '127.0.0.2',
diff --git a/test/parallel/test-net-connect-options-port.js b/test/parallel/test-net-connect-options-port.js
index 1225ba20928..b62fe945784 100644
--- a/test/parallel/test-net-connect-options-port.js
+++ b/test/parallel/test-net-connect-options-port.js
@@ -83,7 +83,7 @@ const net = require('net');
     }
   }, expectedConnections));
 
-  server.listen(0, 'localhost', common.mustCall(() => {
+  server.listen(0, common.localhostIPv4, common.mustCall(() => {
     const port = server.address().port;
 
     // Total connections = 3 * 4(canConnect) * 6(doConnect) = 72
@@ -133,28 +133,35 @@ function doConnect(args, getCb) {
 }
 
 function syncFailToConnect(port, assertErr, optOnly) {
+  const family = 4;
   if (!optOnly) {
     // connect(port, cb) and connect(port)
-    const portArgFunctions = doConnect([port], () => common.mustNotCall());
+    const portArgFunctions = doConnect([{ port, family }],
+                                       () => common.mustNotCall());
     for (const fn of portArgFunctions) {
       assert.throws(fn, assertErr, `${fn.name}(${port})`);
     }
 
     // connect(port, host, cb) and connect(port, host)
-    const portHostArgFunctions = doConnect([port, 'localhost'],
+    const portHostArgFunctions = doConnect([{ port,
+                                              host: 'localhost',
+                                              family }],
                                            () => common.mustNotCall());
     for (const fn of portHostArgFunctions) {
       assert.throws(fn, assertErr, `${fn.name}(${port}, 'localhost')`);
     }
   }
   // connect({port}, cb) and connect({port})
-  const portOptFunctions = doConnect([{ port }], () => common.mustNotCall());
+  const portOptFunctions = doConnect([{ port, family }],
+                                     () => common.mustNotCall());
   for (const fn of portOptFunctions) {
     assert.throws(fn, assertErr, `${fn.name}({port: ${port}})`);
   }
 
   // connect({port, host}, cb) and connect({port, host})
-  const portHostOptFunctions = doConnect([{ port: port, host: 'localhost' }],
+  const portHostOptFunctions = doConnect([{ port: port,
+                                            host: 'localhost',
+                                            family: family }],
                                          () => common.mustNotCall());
   for (const fn of portHostOptFunctions) {
     assert.throws(fn,
@@ -165,27 +172,30 @@ function syncFailToConnect(port, assertErr, optOnly) {
 
 function canConnect(port) {
   const noop = () => common.mustCall();
+  const family = 4;
 
   // connect(port, cb) and connect(port)
-  const portArgFunctions = doConnect([port], noop);
+  const portArgFunctions = doConnect([{ port, family }], noop);
   for (const fn of portArgFunctions) {
     fn();
   }
 
   // connect(port, host, cb) and connect(port, host)
-  const portHostArgFunctions = doConnect([port, 'localhost'], noop);
+  const portHostArgFunctions = doConnect([{ port, host: 'localhost', family }],
+                                         noop);
   for (const fn of portHostArgFunctions) {
     fn();
   }
 
   // connect({port}, cb) and connect({port})
-  const portOptFunctions = doConnect([{ port }], noop);
+  const portOptFunctions = doConnect([{ port, family }], noop);
   for (const fn of portOptFunctions) {
     fn();
   }
 
   // connect({port, host}, cb) and connect({port, host})
-  const portHostOptFns = doConnect([{ port, host: 'localhost' }], noop);
+  const portHostOptFns = doConnect([{ port, host: 'localhost', family }],
+                                   noop);
   for (const fn of portHostOptFns) {
     fn();
   }
@@ -198,20 +208,22 @@ function asyncFailToConnect(port) {
   });
 
   const dont = () => common.mustNotCall();
+  const family = 4;
   // connect(port, cb) and connect(port)
-  const portArgFunctions = doConnect([port], dont);
+  const portArgFunctions = doConnect([{ port, family }], dont);
   for (const fn of portArgFunctions) {
     fn().on('error', onError());
   }
 
   // connect({port}, cb) and connect({port})
-  const portOptFunctions = doConnect([{ port }], dont);
+  const portOptFunctions = doConnect([{ port, family }], dont);
   for (const fn of portOptFunctions) {
     fn().on('error', onError());
   }
 
   // connect({port, host}, cb) and connect({port, host})
-  const portHostOptFns = doConnect([{ port, host: 'localhost' }], dont);
+  const portHostOptFns = doConnect([{ port, host: 'localhost', family }],
+                                   dont);
   for (const fn of portHostOptFns) {
     fn().on('error', onError());
   }
diff --git a/test/parallel/test-net-dns-lookup.js b/test/parallel/test-net-dns-lookup.js
index 53052de716e..8c7bbef128d 100644
--- a/test/parallel/test-net-dns-lookup.js
+++ b/test/parallel/test-net-dns-lookup.js
@@ -29,12 +29,12 @@ const server = net.createServer(function(client) {
   server.close();
 });
 
-server.listen(0, '127.0.0.1', common.mustCall(function() {
+server.listen(0, common.mustCall(function() {
   net.connect(this.address().port, 'localhost')
     .on('lookup', common.mustCall(function(err, ip, type, host) {
       assert.strictEqual(err, null);
-      assert.strictEqual(ip, '127.0.0.1');
-      assert.strictEqual(type, 4);
+      assert.match(ip, /^(127\.0\.0\.1|::1)$/);
+      assert.match(type.toString(), /^(4|6)$/);
       assert.strictEqual(host, 'localhost');
     }));
 }));
diff --git a/test/parallel/test-net-pingpong.js b/test/parallel/test-net-pingpong.js
index 7fb6678fe1a..3bbe076b4b1 100644
--- a/test/parallel/test-net-pingpong.js
+++ b/test/parallel/test-net-pingpong.js
@@ -130,6 +130,4 @@ const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 pingPongTest(common.PIPE);
 pingPongTest(0);
-pingPongTest(0, 'localhost');
-if (common.hasIPv6)
-  pingPongTest(0, '::1');
+if (common.hasIPv6) pingPongTest(0, '::1'); else pingPongTest(0, '127.0.0.1');
diff --git a/test/parallel/test-net-remote-address-port.js b/test/parallel/test-net-remote-address-port.js
index 094206f85df..f7162d0f90f 100644
--- a/test/parallel/test-net-remote-address-port.js
+++ b/test/parallel/test-net-remote-address-port.js
@@ -27,15 +27,17 @@ const net = require('net');
 
 let conns_closed = 0;
 
-const remoteAddrCandidates = [ common.localhostIPv4 ];
-if (common.hasIPv6) remoteAddrCandidates.push('::ffff:127.0.0.1');
+const remoteAddrCandidates = [ common.localhostIPv4,
+                               '::1',
+                               '::ffff:127.0.0.1' ];
 
-const remoteFamilyCandidates = ['IPv4'];
-if (common.hasIPv6) remoteFamilyCandidates.push('IPv6');
+const remoteFamilyCandidates = ['IPv4', 'IPv6'];
 
 const server = net.createServer(common.mustCall(function(socket) {
-  assert.ok(remoteAddrCandidates.includes(socket.remoteAddress));
-  assert.ok(remoteFamilyCandidates.includes(socket.remoteFamily));
+  assert.ok(remoteAddrCandidates.includes(socket.remoteAddress),
+            `Invalid remoteAddress: ${socket.remoteAddress}`);
+  assert.ok(remoteFamilyCandidates.includes(socket.remoteFamily),
+            `Invalid remoteFamily: ${socket.remoteFamily}`);
   assert.ok(socket.remotePort);
   assert.notStrictEqual(socket.remotePort, this.address().port);
   socket.on('end', function() {
@@ -48,8 +50,8 @@ const server = net.createServer(common.mustCall(function(socket) {
   socket.resume();
 }, 2));
 
-server.listen(0, 'localhost', function() {
-  const client = net.createConnection(this.address().port, 'localhost');
+server.listen(0, function() {
+  const client = net.createConnection(this.address().port, '127.0.0.1');
   const client2 = net.createConnection(this.address().port);
   client.on('connect', function() {
     assert.ok(remoteAddrCandidates.includes(client.remoteAddress));
diff --git a/test/parallel/test-net-writable.js b/test/parallel/test-net-writable.js
index dcbb64e272b..3659869efbb 100644
--- a/test/parallel/test-net-writable.js
+++ b/test/parallel/test-net-writable.js
@@ -6,8 +6,8 @@ const net = require('net');
 const server = net.createServer(common.mustCall(function(s) {
   server.close();
   s.end();
-})).listen(0, 'localhost', common.mustCall(function() {
-  const socket = net.connect(this.address().port, 'localhost');
+})).listen(0, '127.0.0.1', common.mustCall(function() {
+  const socket = net.connect(this.address().port, '127.0.0.1');
   socket.on('end', common.mustCall(() => {
     assert.strictEqual(socket.writable, true);
     socket.write('hello world');
diff --git a/test/parallel/test-tcp-wrap-listen.js b/test/parallel/test-tcp-wrap-listen.js
index 72981b683cc..0cac545e81d 100644
--- a/test/parallel/test-tcp-wrap-listen.js
+++ b/test/parallel/test-tcp-wrap-listen.js
@@ -14,7 +14,9 @@ const {
 
 const server = new TCP(TCPConstants.SOCKET);
 
-const r = server.bind('0.0.0.0', 0);
+const r = (common.hasIPv6 ?
+  server.bind6('::', 0) :
+  server.bind('0.0.0.0', 0));
 assert.strictEqual(r, 0);
 let port = {};
 server.getsockname(port);
diff --git a/test/parallel/test-timers-socket-timeout-removes-other-socket-unref-timer.js b/test/parallel/test-timers-socket-timeout-removes-other-socket-unref-timer.js
index ccecfe4c63a..01d2367bdb5 100644
--- a/test/parallel/test-timers-socket-timeout-removes-other-socket-unref-timer.js
+++ b/test/parallel/test-timers-socket-timeout-removes-other-socket-unref-timer.js
@@ -28,7 +28,7 @@ const server = net.createServer(function onClient(client) {
   }
 });
 
-server.listen(0, common.localhostIPv4, common.mustCall(() => {
+server.listen(0, common.mustCall(() => {
   const countdown = new Countdown(2, () => server.close());
 
   {
diff --git a/test/parallel/test-tls-client-getephemeralkeyinfo.js b/test/parallel/test-tls-client-getephemeralkeyinfo.js
index 73ac215102d..82f41cfe7e0 100644
--- a/test/parallel/test-tls-client-getephemeralkeyinfo.js
+++ b/test/parallel/test-tls-client-getephemeralkeyinfo.js
@@ -37,7 +37,7 @@ function test(size, type, name, cipher) {
 
   server.on('close', common.mustSucceed());
 
-  server.listen(0, '127.0.0.1', common.mustCall(() => {
+  server.listen(0, common.mustCall(() => {
     const client = tls.connect({
       port: server.address().port,
       rejectUnauthorized: false
diff --git a/test/parallel/test-tls-client-mindhsize.js b/test/parallel/test-tls-client-mindhsize.js
index a6fbc67bd88..1ccf49fa929 100644
--- a/test/parallel/test-tls-client-mindhsize.js
+++ b/test/parallel/test-tls-client-mindhsize.js
@@ -34,7 +34,7 @@ function test(size, err, next) {
     if (next) next();
   });
 
-  server.listen(0, '127.0.0.1', function() {
+  server.listen(0, function() {
     // Client set minimum DH parameter size to 2048 bits so that
     // it fails when it make a connection to the tls server where
     // dhparams is 1024 bits
diff --git a/test/parallel/test-tls-wrap-econnreset-localaddress.js b/test/parallel/test-tls-wrap-econnreset-localaddress.js
index 30d3a8873fa..b51ddb29ba8 100644
--- a/test/parallel/test-tls-wrap-econnreset-localaddress.js
+++ b/test/parallel/test-tls-wrap-econnreset-localaddress.js
@@ -16,6 +16,7 @@ const server = net.createServer((c) => {
   let errored = false;
   tls.connect({
     port: port,
+    family: 4,
     localAddress: common.localhostIPv4
   }, common.localhostIPv4)
     .once('error', common.mustCall((e) => {
diff --git a/test/pummel/test-http-upload-timeout.js b/test/pummel/test-http-upload-timeout.js
index c3ec3ad19d5..a9932306127 100644
--- a/test/pummel/test-http-upload-timeout.js
+++ b/test/pummel/test-http-upload-timeout.js
@@ -44,7 +44,7 @@ server.on('request', function(req, res) {
   req.resume();
 });
 
-server.listen(0, '127.0.0.1', function() {
+server.listen(0, function() {
   for (let i = 0; i < 10; i++) {
     connections++;
 
diff --git a/test/pummel/test-net-pingpong.js b/test/pummel/test-net-pingpong.js
index 0b2947ade41..7db902ddbed 100644
--- a/test/pummel/test-net-pingpong.js
+++ b/test/pummel/test-net-pingpong.js
@@ -38,7 +38,8 @@ function pingPongTest(host, on_complete) {
     if (host === '127.0.0.1') {
       assert.strictEqual(address, '127.0.0.1');
     } else if (host == null || host === 'localhost') {
-      assert(address === '127.0.0.1' || address === '::ffff:127.0.0.1');
+      assert(address === '127.0.0.1' || address === '::ffff:127.0.0.1' ||
+        address === '::1');
     } else {
       console.log(`host = ${host}, remoteAddress = ${address}`);
       assert.strictEqual(address, '::1');
@@ -110,9 +111,8 @@ function pingPongTest(host, on_complete) {
 }
 
 // All are run at once and will run on different ports.
-pingPongTest('localhost');
 pingPongTest(null);
-
+pingPongTest('127.0.0.1');
 if (common.hasIPv6) pingPongTest('::1');
 
 process.on('exit', function() {
diff --git a/test/sequential/test-https-connect-localport.js b/test/sequential/test-https-connect-localport.js
index e703fb2287d..10d358c8a92 100644
--- a/test/sequential/test-https-connect-localport.js
+++ b/test/sequential/test-https-connect-localport.js
@@ -17,7 +17,7 @@ const assert = require('assert');
     res.end();
   }));
 
-  server.listen(0, 'localhost', common.mustCall(() => {
+  server.listen(0, '127.0.0.1', common.mustCall(() => {
     const port = server.address().port;
     const req = https.get({
       host: 'localhost',
diff --git a/test/sequential/test-inspector-open.js b/test/sequential/test-inspector-open.js
index 190a99e7282..847f0ab6926 100644
--- a/test/sequential/test-inspector-open.js
+++ b/test/sequential/test-inspector-open.js
@@ -80,7 +80,7 @@ function reopenAfterClose(msg) {
 }
 
 function ping(port, callback) {
-  net.connect(port)
+  net.connect({ port, family: 4 })
     .on('connect', function() { close(this); })
     .on('error', function(err) { close(this, err); });
 
diff --git a/test/sequential/test-net-better-error-messages-port.js b/test/sequential/test-net-better-error-messages-port.js
index 789b2e31801..c21427ee395 100644
--- a/test/sequential/test-net-better-error-messages-port.js
+++ b/test/sequential/test-net-better-error-messages-port.js
@@ -10,5 +10,5 @@ c.on('connect', common.mustNotCall());
 c.on('error', common.mustCall(function(e) {
   assert.strictEqual(e.code, 'ECONNREFUSED');
   assert.strictEqual(e.port, common.PORT);
-  assert.strictEqual(e.address, '127.0.0.1');
+  assert.match(e.address, /^(127\.0\.0\.1|::1)$/);
 }));
diff --git a/test/sequential/test-net-connect-local-error.js b/test/sequential/test-net-connect-local-error.js
index 030c4de750c..6169c14989b 100644
--- a/test/sequential/test-net-connect-local-error.js
+++ b/test/sequential/test-net-connect-local-error.js
@@ -9,12 +9,14 @@ const expectedErrorCodes = ['ECONNREFUSED', 'EADDRINUSE'];
 
 const optionsIPv4 = {
   port: common.PORT,
+  family: 4,
   localPort: common.PORT + 1,
   localAddress: common.localhostIPv4
 };
 
 const optionsIPv6 = {
   host: '::1',
+  family: 6,
   port: common.PORT + 2,
   localPort: common.PORT + 3,
   localAddress: '::1',