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

PR-URL: #23173
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>

Author: oyyd

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.
 
+<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
 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
 [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array

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 {

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;

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,
 };

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');
 }, {

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);