dns: remove dns.lookup and dnsPromises.lookup options type coercion

PR-URL: #41431 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · Apr 12, 2022 · 08773e3 · 08773e3
1 parent 3a26db9
commit 08773e3
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 89 deletions.
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -2943,6 +2943,9 @@ deprecated and should no longer be used.
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/41431
+    description: End-of-Life.
   - version: v17.0.0
     pr-url: https://github.com/nodejs/node/pull/39793
     description: Runtime deprecation.
@@ -2951,12 +2954,13 @@ changes:
     description: Documentation-only deprecation.
 -->
 
-Type: Runtime
+Type: End-of-Life
 
 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`
 option, or a non-nullish non-boolean value for `verbatim` option in
-[`dns.lookup()`][] and [`dnsPromises.lookup()`][] is deprecated.
+[`dns.lookup()`][] and [`dnsPromises.lookup()`][] throws an
+`ERR_INVALID_ARG_TYPE` error.
 
 ### DEP0154: RSA-PSS generate key pair options
 

diff --git a/lib/dns.js b/lib/dns.js
@@ -42,7 +42,6 @@ const {
   Resolver,
   validateHints,
   emitInvalidHostnameWarning,
-  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
   setDefaultResultOrder,
 } = require('internal/dns/utils');
@@ -52,10 +51,12 @@ const {
   ERR_MISSING_ARGS,
 } = errors.codes;
 const {
+  validateBoolean,
   validateFunction,
+  validateNumber,
+  validateOneOf,
   validatePort,
   validateString,
-  validateOneOf,
 } = require('internal/validators');
 
 const {
@@ -107,9 +108,10 @@ function onlookupall(err, addresses) {
 
 // Easy DNS A/AAAA look up
 // lookup(hostname, [options,] callback)
+const validFamilies = [0, 4, 6];
 function lookup(hostname, options, callback) {
   let hints = 0;
-  let family = -1;
+  let family = 0;
   let all = false;
   let verbatim = getDefaultVerbatim();
 
@@ -121,39 +123,36 @@ function lookup(hostname, options, callback) {
   if (typeof options === 'function') {
     callback = options;
     family = 0;
+  } else if (typeof options === 'number') {
+    validateFunction(callback, 'callback');
+
+    validateOneOf(options, 'family', validFamilies, true);
+    family = options;
+  } else if (options !== undefined && typeof options !== 'object') {
+    validateFunction(arguments.length === 2 ? options : callback, 'callback');
+    throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
   } else {
     validateFunction(callback, 'callback');
 
-    if (options !== null && typeof options === 'object') {
-      if (options.hints != null && typeof options.hints !== 'number') {
-        emitTypeCoercionDeprecationWarning();
-      }
+    if (options?.hints != null) {
+      validateNumber(options.hints, 'options.hints');
       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;
+    }
+    if (options?.family != null) {
+      validateOneOf(options.family, 'options.family', validFamilies, true);
+      family = options.family;
+    }
+    if (options?.all != null) {
+      validateBoolean(options.all, 'options.all');
+      all = options.all;
+    }
+    if (options?.verbatim != null) {
+      validateBoolean(options.verbatim, 'options.verbatim');
+      verbatim = options.verbatim;
     }
   }
 
-  validateOneOf(family, 'family', [0, 4, 6]);
-
   if (!hostname) {
     emitInvalidHostnameWarning(hostname);
     if (all) {

diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js
@@ -15,7 +15,6 @@ const {
   validateTimeout,
   validateTries,
   emitInvalidHostnameWarning,
-  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
 } = require('internal/dns/utils');
 const { codes, dnsException } = require('internal/errors');
@@ -30,13 +29,16 @@ const {
   QueryReqWrap
 } = internalBinding('cares_wrap');
 const {
+  ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
   ERR_MISSING_ARGS,
 } = codes;
 const {
+  validateBoolean,
+  validateNumber,
+  validateOneOf,
   validatePort,
   validateString,
-  validateOneOf,
 } = require('internal/validators');
 
 const kPerfHooksDnsLookupContext = Symbol('kPerfHooksDnsLookupContext');
@@ -120,9 +122,10 @@ function createLookupPromise(family, hostname, all, hints, verbatim) {
   });
 }
 
+const validFamilies = [0, 4, 6];
 function lookup(hostname, options) {
   let hints = 0;
-  let family = -1;
+  let family = 0;
   let all = false;
   let verbatim = getDefaultVerbatim();
 
@@ -131,35 +134,31 @@ function lookup(hostname, options) {
     validateString(hostname, 'hostname');
   }
 
-  if (options !== null && typeof options === 'object') {
-    if (options.hints != null && typeof options.hints !== 'number') {
-      emitTypeCoercionDeprecationWarning();
+  if (typeof options === 'number') {
+    validateOneOf(options, 'family', validFamilies, true);
+    family = options;
+  } else if (options !== undefined && typeof options !== 'object') {
+    throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
+  } else {
+    if (options?.hints != null) {
+      validateNumber(options.hints, 'options.hints');
+      hints = options.hints >>> 0;
+      validateHints(hints);
     }
-    hints = options.hints >>> 0;
-    if (options.family != null && typeof options.family !== 'number') {
-      emitTypeCoercionDeprecationWarning();
+    if (options?.family != null) {
+      validateOneOf(options.family, 'options.family', validFamilies, true);
+      family = options.family;
     }
-    family = options.family >>> 0;
-    if (options.all != null && typeof options.all !== 'boolean') {
-      emitTypeCoercionDeprecationWarning();
+    if (options?.all != null) {
+      validateBoolean(options.all, 'options.all');
+      all = options.all;
     }
-    all = options.all === true;
-    if (typeof options.verbatim === 'boolean') {
-      verbatim = options.verbatim === true;
-    } else if (options.verbatim != null) {
-      emitTypeCoercionDeprecationWarning();
+    if (options?.verbatim != null) {
+      validateBoolean(options.verbatim, 'options.verbatim');
+      verbatim = options.verbatim;
     }
-
-    validateHints(hints);
-  } else {
-    if (options != null && typeof options !== 'number') {
-      emitTypeCoercionDeprecationWarning();
-    }
-    family = options >>> 0;
   }
 
-  validateOneOf(family, 'family', [0, 4, 6], true);
-
   return createLookupPromise(family, hostname, all, hints, verbatim);
 }
 

diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js
@@ -190,18 +190,6 @@ 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() {
@@ -222,7 +210,6 @@ module.exports = {
   validateTries,
   Resolver,
   emitInvalidHostnameWarning,
-  emitTypeCoercionDeprecationWarning,
   getDefaultVerbatim,
   setDefaultResultOrder,
 };
diff --git a/test/internet/test-dns-lookup.js b/test/internet/test-dns-lookup.js
@@ -45,17 +45,10 @@ dns.lookup(addresses.NOT_FOUND, {
   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, {
+assert.throws(
+  () => dnsPromises.lookup(addresses.NOT_FOUND, {
     family: 'IPv4',
     all: 'all'
   }),
-  {
-    code: 'ENOTFOUND',
-    message: `getaddrinfo ENOTFOUND ${addresses.NOT_FOUND}`
-  }
+  { code: 'ERR_INVALID_ARG_VALUE' }
 );
diff --git a/test/parallel/test-dns-lookup-promises-options-deprecated.js b/test/parallel/test-dns-lookup-promises-options-deprecated.js
@@ -15,18 +15,21 @@ common.expectWarning({
   'internal/test/binding': [
     'These APIs are for internal testing only. Do not use them.',
   ],
-  'DeprecationWarning': {
-    DEP0153: 'Type coercion of dns.lookup options is deprecated'
-  }
 });
 
 assert.throws(() => {
   dnsPromises.lookup('127.0.0.1', { hints: '-1' });
 }, {
-  code: 'ERR_INVALID_ARG_VALUE',
+  code: 'ERR_INVALID_ARG_TYPE',
   name: 'TypeError'
 });
-dnsPromises.lookup('127.0.0.1', { family: '6' });
-dnsPromises.lookup('127.0.0.1', { all: 'true' });
-dnsPromises.lookup('127.0.0.1', { verbatim: 'true' });
-dnsPromises.lookup('127.0.0.1', '6');
+assert.throws(() => dnsPromises.lookup('127.0.0.1', { hints: -1 }),
+              { code: 'ERR_INVALID_ARG_VALUE' });
+assert.throws(() => dnsPromises.lookup('127.0.0.1', { family: '6' }),
+              { code: 'ERR_INVALID_ARG_VALUE' });
+assert.throws(() => dnsPromises.lookup('127.0.0.1', { all: 'true' }),
+              { code: 'ERR_INVALID_ARG_TYPE' });
+assert.throws(() => dnsPromises.lookup('127.0.0.1', { verbatim: 'true' }),
+              { code: 'ERR_INVALID_ARG_TYPE' });
+assert.throws(() => dnsPromises.lookup('127.0.0.1', '6'),
+              { code: 'ERR_INVALID_ARG_TYPE' });
diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js
@@ -69,14 +69,15 @@ assert.throws(() => {
 }
 
 {
+  const family = 20;
   const err = {
     code: 'ERR_INVALID_ARG_VALUE',
     name: 'TypeError',
-    message: "The argument 'family' must be one of: 0, 4, 6. Received 20"
+    message: `The property 'options.family' must be one of: 0, 4, 6. Received ${family}`
   };
   const options = {
     hints: 0,
-    family: 20,
+    family,
     all: false
   };
 
@@ -86,6 +87,52 @@ assert.throws(() => {
   }, err);
 }
 
+[1, 0n, 1n, '', '0', Symbol(), true, false, {}, [], () => {}]
+  .forEach((family) => {
+    const err = { code: 'ERR_INVALID_ARG_VALUE' };
+    const options = { family };
+    assert.throws(() => { dnsPromises.lookup(false, options); }, err);
+    assert.throws(() => {
+      dns.lookup(false, options, common.mustNotCall());
+    }, err);
+  });
+[0n, 1n, '', '0', Symbol(), true, false].forEach((family) => {
+  const err = { code: 'ERR_INVALID_ARG_TYPE' };
+  assert.throws(() => { dnsPromises.lookup(false, family); }, err);
+  assert.throws(() => {
+    dns.lookup(false, family, common.mustNotCall());
+  }, err);
+});
+assert.throws(() => dnsPromises.lookup(false, () => {}),
+              { code: 'ERR_INVALID_ARG_TYPE' });
+
+[0n, 1n, '', '0', Symbol(), true, false, {}, [], () => {}].forEach((hints) => {
+  const err = { code: 'ERR_INVALID_ARG_TYPE' };
+  const options = { hints };
+  assert.throws(() => { dnsPromises.lookup(false, options); }, err);
+  assert.throws(() => {
+    dns.lookup(false, options, common.mustNotCall());
+  }, err);
+});
+
+[0, 1, 0n, 1n, '', '0', Symbol(), {}, [], () => {}].forEach((all) => {
+  const err = { code: 'ERR_INVALID_ARG_TYPE' };
+  const options = { all };
+  assert.throws(() => { dnsPromises.lookup(false, options); }, err);
+  assert.throws(() => {
+    dns.lookup(false, options, common.mustNotCall());
+  }, err);
+});
+
+[0, 1, 0n, 1n, '', '0', Symbol(), {}, [], () => {}].forEach((verbatim) => {
+  const err = { code: 'ERR_INVALID_ARG_TYPE' };
+  const options = { verbatim };
+  assert.throws(() => { dnsPromises.lookup(false, options); }, err);
+  assert.throws(() => {
+    dns.lookup(false, options, common.mustNotCall());
+  }, err);
+});
+
 (async function() {
   let res;