From 0ae969f2efce42d1e7ae6d95374be15432f42d0f Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Sun, 30 Sep 2018 18:17:36 +0800 Subject: [PATCH 1/7] 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: #13119 --- lib/dns.js | 6 ++++-- lib/internal/dns/promises.js | 6 ++++-- lib/internal/dns/utils.js | 11 ++++++++++- test/parallel/test-dns-lookup.js | 25 ++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 44fdb1764a13c0..bef14a469e7cc6 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -31,7 +31,8 @@ const { getDefaultResolver, setDefaultResolver, Resolver, - validateHints + validateHints, + emitInvalidHostnameWarning, } = require('internal/dns/utils'); const { ERR_INVALID_ARG_TYPE, @@ -93,7 +94,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; @@ -114,6 +115,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 408dec712d821f..c38a7c591da35c 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'); @@ -56,6 +57,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 @@ -100,7 +102,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 2362f28a821b9a..1f72f81b2e6d4c 100644 --- a/lib/internal/dns/utils.js +++ b/lib/internal/dns/utils.js @@ -141,10 +141,19 @@ function validateHints(hints) { } } +function emitInvalidHostnameWarning(hostname) { + process.emitWarning( + `The provided hostname "${hostname}" is not a valid ` + + 'hostname, and is supported in the dns module solely for compatibility.', + 'DeprecationWarning', + ); +} + 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..d5f2a9507723f4 100644 --- a/test/parallel/test-dns-lookup.js +++ b/test/parallel/test-dns-lookup.js @@ -14,13 +14,36 @@ 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 call `dns.lookup` with falsy `hostname`, twice. + 'DeprecationWarning': [ + [ + 'The provided hostname "false" is not a valid ' + + 'hostname, and is supported in the dns module solely for compatibility.', + ], + [ + 'The provided hostname "false" is not a valid ' + + 'hostname, and is supported in the dns module solely for compatibility.', + ] + ], +}); + common.expectsError(() => { dns.lookup(false, 'cb'); }, { From 197af1c9099938c1f75362e30d5e05be8b3faeba Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Sun, 30 Sep 2018 21:08:25 +0800 Subject: [PATCH 2/7] dns: add deprecation code and fix tests --- doc/api/deprecations.md | 16 ++++++++++++++++ lib/internal/dns/utils.js | 1 + test/parallel/test-dns-lookup.js | 2 ++ test/parallel/test-dns.js | 2 +- 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index dc12efb9069f12..743054d474dcc6 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 supports for falsy hostname + + +Type: Runtime + +Previous versions of Node.js support `dns.lookup` a falsy hostname like +`dns.lookup(false)` for the reason of backward compatibility long before. +This behavior is undocumented and seems useless in real world apps. We +might make invalid hostname throw in the future. + [`--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/internal/dns/utils.js b/lib/internal/dns/utils.js index 1f72f81b2e6d4c..43a7ccc53de0b6 100644 --- a/lib/internal/dns/utils.js +++ b/lib/internal/dns/utils.js @@ -146,6 +146,7 @@ function emitInvalidHostnameWarning(hostname) { `The provided hostname "${hostname}" is not a valid ` + 'hostname, and is supported in the dns module solely for compatibility.', 'DeprecationWarning', + 'DEP0118' ); } diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js index d5f2a9507723f4..05dd711521010f 100644 --- a/test/parallel/test-dns-lookup.js +++ b/test/parallel/test-dns-lookup.js @@ -36,10 +36,12 @@ common.expectWarning({ [ 'The provided hostname "false" is not a valid ' + 'hostname, and is supported in the dns module solely for compatibility.', + 'DEP0118', ], [ 'The provided hostname "false" is not a valid ' + 'hostname, and is supported in the dns module solely for compatibility.', + 'DEP0118', ] ], }); 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); From 8b0fceeeef71a9c7aa4db1a8185a112f875e3b26 Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Sun, 30 Sep 2018 22:10:48 +0800 Subject: [PATCH 3/7] dns: replace the deprecation number with a temporal placeholder --- doc/api/deprecations.md | 6 +++--- lib/internal/dns/utils.js | 2 +- test/parallel/test-dns-lookup.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 743054d474dcc6..931b9e3d268bcc 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2214,8 +2214,8 @@ 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 supports for falsy hostname + +### DEP0XXX: dns.lookup() supports for falsy hostname