dns: deprecate passing falsy hostname to dns.lookup

We can `dns.lookup` a falsy `hostname` like `dns.lookup(false)`
for the reason of backwards compatibility long before(see #13119
for detail). This behavior is undocumented and seems useless in
real world apps.

We could also make invalid `hostname` throw in the future and the
change might be semver-major.

Fixes: https://github.com/nodejs/node/issues/13119

PR-URL: https://github.com/nodejs/node/pull/23173
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit is contained in:
Ouyang Yadong 2018-09-30 18:17:36 +08:00 committed by Anna Henningsen
parent 6f7fd7f9bc
commit 26af728994
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
6 changed files with 61 additions and 7 deletions

View File

@ -2214,6 +2214,22 @@ the `_handle` property of the `Cipher`, `Decipher`, `DiffieHellman`,
Using the `_handle` property to access the native object is deprecated because Using the `_handle` property to access the native object is deprecated because
improper use of the native object can lead to crashing the application. improper use of the native object can lead to crashing the application.
<a id="DEP0118"></a>
### DEP0118: dns.lookup() support for a falsy hostname
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23173
description: Runtime deprecation.
-->
Type: Runtime
Previous versions of Node.js supported `dns.lookup()` with a falsy hostname
like `dns.lookup(false)` due to backward compatibility.
This behavior is undocumented and is thought to be unused in real world apps.
It will become an error in future versions of Node.js.
[`--pending-deprecation`]: cli.html#cli_pending_deprecation [`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array

View File

@ -30,7 +30,8 @@ const {
getDefaultResolver, getDefaultResolver,
setDefaultResolver, setDefaultResolver,
Resolver, Resolver,
validateHints validateHints,
emitInvalidHostnameWarning,
} = require('internal/dns/utils'); } = require('internal/dns/utils');
const { const {
ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_TYPE,
@ -92,7 +93,7 @@ function lookup(hostname, options, callback) {
// Parse arguments // Parse arguments
if (hostname && typeof hostname !== 'string') { if (hostname && typeof hostname !== 'string') {
throw new ERR_INVALID_ARG_TYPE('hostname', ['string', 'falsy'], hostname); throw new ERR_INVALID_ARG_TYPE('hostname', 'string', hostname);
} else if (typeof options === 'function') { } else if (typeof options === 'function') {
callback = options; callback = options;
family = 0; family = 0;
@ -113,6 +114,7 @@ function lookup(hostname, options, callback) {
throw new ERR_INVALID_OPT_VALUE('family', family); throw new ERR_INVALID_OPT_VALUE('family', family);
if (!hostname) { if (!hostname) {
emitInvalidHostnameWarning(hostname);
if (all) { if (all) {
process.nextTick(callback, null, []); process.nextTick(callback, null, []);
} else { } else {

View File

@ -2,7 +2,8 @@
const { const {
bindDefaultResolver, bindDefaultResolver,
Resolver: CallbackResolver, Resolver: CallbackResolver,
validateHints validateHints,
emitInvalidHostnameWarning,
} = require('internal/dns/utils'); } = require('internal/dns/utils');
const { codes, dnsException } = require('internal/errors'); const { codes, dnsException } = require('internal/errors');
const { isIP, isIPv4, isLegalPort } = require('internal/net'); const { isIP, isIPv4, isLegalPort } = require('internal/net');
@ -55,6 +56,7 @@ function onlookupall(err, addresses) {
function createLookupPromise(family, hostname, all, hints, verbatim) { function createLookupPromise(family, hostname, all, hints, verbatim) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
if (!hostname) { if (!hostname) {
emitInvalidHostnameWarning(hostname);
if (all) if (all)
resolve([]); resolve([]);
else else
@ -99,7 +101,7 @@ function lookup(hostname, options) {
// Parse arguments // Parse arguments
if (hostname && typeof hostname !== 'string') { if (hostname && typeof hostname !== 'string') {
throw new ERR_INVALID_ARG_TYPE('hostname', ['string', 'falsy'], hostname); throw new ERR_INVALID_ARG_TYPE('hostname', 'string', hostname);
} else if (options !== null && typeof options === 'object') { } else if (options !== null && typeof options === 'object') {
hints = options.hints >>> 0; hints = options.hints >>> 0;
family = options.family >>> 0; family = options.family >>> 0;

View File

@ -140,10 +140,26 @@ function validateHints(hints) {
} }
} }
let invalidHostnameWarningEmitted = false;
function emitInvalidHostnameWarning(hostname) {
if (invalidHostnameWarningEmitted) {
return;
}
invalidHostnameWarningEmitted = true;
process.emitWarning(
`The provided hostname "${hostname}" is not a valid ` +
'hostname, and is supported in the dns module solely for compatibility.',
'DeprecationWarning',
'DEP0118'
);
}
module.exports = { module.exports = {
bindDefaultResolver, bindDefaultResolver,
getDefaultResolver, getDefaultResolver,
setDefaultResolver, setDefaultResolver,
validateHints, validateHints,
Resolver Resolver,
emitInvalidHostnameWarning,
}; };

View File

@ -14,13 +14,31 @@ cares.getaddrinfo = () => internalBinding('uv').UV_ENOENT;
const err = { const err = {
code: 'ERR_INVALID_ARG_TYPE', code: 'ERR_INVALID_ARG_TYPE',
type: TypeError, type: TypeError,
message: /^The "hostname" argument must be one of type string or falsy/ message: /^The "hostname" argument must be of type string\. Received type number/
}; };
common.expectsError(() => dns.lookup(1, {}), err); common.expectsError(() => dns.lookup(1, {}), err);
common.expectsError(() => dnsPromises.lookup(1, {}), err); common.expectsError(() => dnsPromises.lookup(1, {}), err);
} }
common.expectWarning({
// For 'internal/test/binding' module.
'internal/test/binding': [
'These APIs are exposed only for testing and are not ' +
'tracked by any versioning system or deprecation process.'
],
// For dns.promises.
'ExperimentalWarning': [
'The dns.promises API is experimental'
],
// For calling `dns.lookup` with falsy `hostname`.
'DeprecationWarning': [
'The provided hostname "false" is not a valid ' +
'hostname, and is supported in the dns module solely for compatibility.',
'DEP0118',
],
});
common.expectsError(() => { common.expectsError(() => {
dns.lookup(false, 'cb'); dns.lookup(false, 'cb');
}, { }, {

View File

@ -166,7 +166,7 @@ assert.deepStrictEqual(dns.getServers(), []);
const errorReg = common.expectsError({ const errorReg = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE', code: 'ERR_INVALID_ARG_TYPE',
type: TypeError, type: TypeError,
message: /^The "hostname" argument must be one of type string or falsy/ message: /^The "hostname" argument must be of type string\. Received type .*/
}, 10); }, 10);
assert.throws(() => dns.lookup({}, common.mustNotCall()), errorReg); assert.throws(() => dns.lookup({}, common.mustNotCall()), errorReg);