Skip to content

Commit

Permalink
dns: default to verbatim=true in dns.lookup()
Browse files Browse the repository at this point in the history
Switch the default from `ipv4first` to `verbatim` (return them exactly
as the resolver sent them to us).

PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
bnoordhuis authored and aduh95 committed Sep 12, 2021
1 parent b6ac7e6 commit 169da5d
Show file tree
Hide file tree
Showing 24 changed files with 70 additions and 54 deletions.
8 changes: 5 additions & 3 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
10 changes: 2 additions & 8 deletions lib/internal/dns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion test/common/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ if (cluster.isWorker) {
maybeReply();
});

server.listen(0, '127.0.0.1');
server.listen(0);
} else if (cluster.isPrimary) {

const checks = {
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-http-localaddress.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-upgrade-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-connect-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-https-localaddress.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
36 changes: 24 additions & 12 deletions test/parallel/test-net-connect-options-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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();
}
Expand All @@ -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());
}
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-net-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}));
}));
4 changes: 1 addition & 3 deletions test/parallel/test-net-pingpong.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
18 changes: 10 additions & 8 deletions test/parallel/test-net-remote-address-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-net-writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-tcp-wrap-listen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

{
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-client-getephemeralkeyinfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-client-mindhsize.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-tls-wrap-econnreset-localaddress.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion test/pummel/test-http-upload-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;

Expand Down
6 changes: 3 additions & 3 deletions test/pummel/test-net-pingpong.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-https-connect-localport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-inspector-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -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); });

Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-net-better-error-messages-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)$/);
}));
2 changes: 2 additions & 0 deletions test/sequential/test-net-connect-local-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 169da5d

Please sign in to comment.