From 8e53d29680ab656860c4751f91cf106765d624d9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 7 Jan 2022 12:00:10 +0100 Subject: [PATCH] dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion --- doc/api/deprecations.md | 8 ++- lib/dns.js | 55 +++++++++--------- lib/internal/dns/promises.js | 57 +++++++++---------- lib/internal/dns/utils.js | 13 ----- test/internet/test-dns-lookup.js | 13 +---- ...-dns-lookup-promises-options-deprecated.js | 19 ++++--- test/parallel/test-dns-lookup.js | 51 ++++++++++++++++- 7 files changed, 124 insertions(+), 92 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 32b25ee9b87f88..156db647b434d4 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2943,6 +2943,9 @@ deprecated and should no longer be used. -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 index 14937769d1d4d9..af5416c62478cc 100644 --- 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 index 314b121be5d152..af64cae389a1b2 100644 --- 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,46 +122,43 @@ function createLookupPromise(family, hostname, all, hints, verbatim) { }); } +const validFamilies = [0, 4, 6]; function lookup(hostname, options) { - var hints = 0; - var family = -1; - var all = false; - var verbatim = getDefaultVerbatim(); + let hints = 0; + let family = 0; + let all = false; + let verbatim = getDefaultVerbatim(); // Parse arguments if (hostname) { 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 index 58d3eaafcaa6c9..62606e6f7f0aaf 100644 --- 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 index 6efa946f9d7522..d4e3d6d1eb762b 100644 --- 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 index 6f3b9314b9db13..2761b616620cca 100644 --- 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 index 461e705a5e3cb6..a847a91d655196 100644 --- 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;