From 26af72899417525dd32372d63c07ee5aa1ab6c5c Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Sun, 30 Sep 2018 18:17:36 +0800 Subject: [PATCH] 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 Reviewed-By: Denys Otrishko Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- doc/api/deprecations.md | 16 ++++++++++++++++ lib/dns.js | 6 ++++-- lib/internal/dns/promises.js | 6 ++++-- lib/internal/dns/utils.js | 18 +++++++++++++++++- test/parallel/test-dns-lookup.js | 20 +++++++++++++++++++- test/parallel/test-dns.js | 2 +- 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index dc12efb9069f12..ca23a4fa322f94 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2214,6 +2214,22 @@ the `_handle` property of the `Cipher`, `Decipher`, `DiffieHellman`, Using the `_handle` property to access the native object is deprecated because improper use of the native object can lead to crashing the application. + +### DEP0118: dns.lookup() support for a falsy hostname + + +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 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array diff --git a/lib/dns.js b/lib/dns.js index 326499497b1580..f3b86a471d74a3 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -30,7 +30,8 @@ const { getDefaultResolver, setDefaultResolver, Resolver, - validateHints + validateHints, + emitInvalidHostnameWarning, } = require('internal/dns/utils'); const { ERR_INVALID_ARG_TYPE, @@ -92,7 +93,7 @@ function lookup(hostname, options, callback) { // Parse arguments 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') { callback = options; family = 0; @@ -113,6 +114,7 @@ function lookup(hostname, options, callback) { throw new ERR_INVALID_OPT_VALUE('family', family); if (!hostname) { + emitInvalidHostnameWarning(hostname); if (all) { process.nextTick(callback, null, []); } else { diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js index 6d2fa3b170418d..8baa0fa0de8241 100644 --- a/lib/internal/dns/promises.js +++ b/lib/internal/dns/promises.js @@ -2,7 +2,8 @@ const { bindDefaultResolver, Resolver: CallbackResolver, - validateHints + validateHints, + emitInvalidHostnameWarning, } = require('internal/dns/utils'); const { codes, dnsException } = require('internal/errors'); const { isIP, isIPv4, isLegalPort } = require('internal/net'); @@ -55,6 +56,7 @@ function onlookupall(err, addresses) { function createLookupPromise(family, hostname, all, hints, verbatim) { return new Promise((resolve, reject) => { if (!hostname) { + emitInvalidHostnameWarning(hostname); if (all) resolve([]); else @@ -99,7 +101,7 @@ function lookup(hostname, options) { // Parse arguments 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') { hints = options.hints >>> 0; family = options.family >>> 0; diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js index 143d36193a352b..d279a58840154e 100644 --- a/lib/internal/dns/utils.js +++ b/lib/internal/dns/utils.js @@ -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 = { bindDefaultResolver, getDefaultResolver, setDefaultResolver, validateHints, - Resolver + Resolver, + emitInvalidHostnameWarning, }; diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js index e67120b0d2521b..b30c5795feb31a 100644 --- a/test/parallel/test-dns-lookup.js +++ b/test/parallel/test-dns-lookup.js @@ -14,13 +14,31 @@ cares.getaddrinfo = () => internalBinding('uv').UV_ENOENT; const err = { code: 'ERR_INVALID_ARG_TYPE', 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(() => 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(() => { dns.lookup(false, 'cb'); }, { diff --git a/test/parallel/test-dns.js b/test/parallel/test-dns.js index a618bccd89ac13..0636d8c28e1fb8 100644 --- a/test/parallel/test-dns.js +++ b/test/parallel/test-dns.js @@ -166,7 +166,7 @@ assert.deepStrictEqual(dns.getServers(), []); const errorReg = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', 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); assert.throws(() => dns.lookup({}, common.mustNotCall()), errorReg);