dns: runtime deprecate type coercion of dns.lookup options

PR-URL: #39793 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
nodejs · Sep 15, 2021 · f182b9b · Trott · Nov 21, 2021 · f182b9b
1 parent 7b4e6d4
commit f182b9b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 1 deletion.
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -2789,12 +2789,15 @@ deprecated and should no longer be used.
 ### DEP0153: `dns.lookup` and `dnsPromises.lookup` options type coercion
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/39793
+    description: Runtime deprecation.
   - version: v16.8.0
     pr-url: https://github.com/nodejs/node/pull/38906
     description: Documentation-only deprecation.
 -->
 
-Type: Documentation-only
+Type: Runtime
 
 Using a non-nullish non-integer value for `family` option, a non-nullish
 non-number value for `hints` option, a non-nullish non-boolean value for `all`

diff --git a/lib/dns.js b/lib/dns.js
@@ -41,6 +41,7 @@ const {
   Resolver,
   validateHints,
   emitInvalidHostnameWarning,
+  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
   setDefaultResultOrder,
 } = require('internal/dns/utils');
@@ -112,15 +113,29 @@ function lookup(hostname, options, callback) {
     validateCallback(callback);
 
     if (options !== null && typeof options === 'object') {
+      if (options.hints != null && typeof options.hints !== 'number') {
+        emitTypeCoercionDeprecationWarning();
+      }
       hints = options.hints >>> 0;
+      if (options.family != null && typeof options.family !== 'number') {
+        emitTypeCoercionDeprecationWarning();
+      }
       family = options.family >>> 0;
+      if (options.all != null && typeof options.all !== 'boolean') {
+        emitTypeCoercionDeprecationWarning();
+      }
       all = options.all === true;
       if (typeof options.verbatim === 'boolean') {
         verbatim = options.verbatim === true;
+      } else if (options.verbatim != null) {
+        emitTypeCoercionDeprecationWarning();
       }
 
       validateHints(hints);
     } else {
+      if (options != null && typeof options !== 'number') {
+        emitTypeCoercionDeprecationWarning();
+      }
       family = options >>> 0;
     }
   }

diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js
@@ -14,6 +14,7 @@ const {
   validateTimeout,
   validateTries,
   emitInvalidHostnameWarning,
+  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
 } = require('internal/dns/utils');
 const { codes, dnsException } = require('internal/errors');
@@ -110,15 +111,29 @@ function lookup(hostname, options) {
   if (hostname && typeof hostname !== 'string') {
     throw new ERR_INVALID_ARG_TYPE('hostname', 'string', hostname);
   } else if (options !== null && typeof options === 'object') {
+    if (options.hints != null && typeof options.hints !== 'number') {
+      emitTypeCoercionDeprecationWarning();
+    }
     hints = options.hints >>> 0;
+    if (options.family != null && typeof options.family !== 'number') {
+      emitTypeCoercionDeprecationWarning();
+    }
     family = options.family >>> 0;
+    if (options.all != null && typeof options.all !== 'boolean') {
+      emitTypeCoercionDeprecationWarning();
+    }
     all = options.all === true;
     if (typeof options.verbatim === 'boolean') {
       verbatim = options.verbatim === true;
+    } else if (options.verbatim != null) {
+      emitTypeCoercionDeprecationWarning();
     }
 
     validateHints(hints);
   } else {
+    if (options != null && typeof options !== 'number') {
+      emitTypeCoercionDeprecationWarning();
+    }
     family = options >>> 0;
   }
 

diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js
@@ -193,6 +193,18 @@ function emitInvalidHostnameWarning(hostname) {
   );
 }
 
+let typeCoercionWarningEmitted = false;
+function emitTypeCoercionDeprecationWarning() {
+  if (!typeCoercionWarningEmitted) {
+    process.emitWarning(
+      'Type coercion of dns.lookup options is deprecated',
+      'DeprecationWarning',
+      'DEP0153'
+    );
+    typeCoercionWarningEmitted = true;
+  }
+}
+
 let dnsOrder = getOptionValue('--dns-result-order') || 'verbatim';
 
 function getDefaultVerbatim() {
@@ -213,6 +225,7 @@ module.exports = {
   validateTries,
   Resolver,
   emitInvalidHostnameWarning,
+  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
   setDefaultResultOrder,
 };
diff --git a/test/internet/test-dns-lookup.js b/test/internet/test-dns-lookup.js
@@ -44,3 +44,18 @@ dns.lookup(addresses.NOT_FOUND, {
   assert.strictEqual(error.syscall, 'getaddrinfo');
   assert.strictEqual(error.hostname, addresses.NOT_FOUND);
 }));
+
+common.expectWarning('DeprecationWarning',
+                     'Type coercion of dns.lookup options is deprecated',
+                     'DEP0153');
+
+assert.rejects(
+  dnsPromises.lookup(addresses.NOT_FOUND, {
+    family: 'IPv4',
+    all: 'all'
+  }),
+  {
+    code: 'ENOTFOUND',
+    message: `getaddrinfo ENOTFOUND ${addresses.NOT_FOUND}`
+  }
+);