Skip to content

Commit

Permalink
dns: runtime deprecate type coercion of dns.lookup options
Browse files Browse the repository at this point in the history
PR-URL: #39793
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
  • Loading branch information
aduh95 authored and nodejs-github-bot committed Sep 15, 2021
1 parent 7b4e6d4 commit f182b9b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 1 deletion.
5 changes: 4 additions & 1 deletion doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
15 changes: 15 additions & 0 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const {
Resolver,
validateHints,
emitInvalidHostnameWarning,
emitTypeCoercionDeprecationWarning,
getDefaultVerbatim,
setDefaultResultOrder,
} = require('internal/dns/utils');
Expand Down Expand Up @@ -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;
}
}
Expand Down
15 changes: 15 additions & 0 deletions lib/internal/dns/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const {
validateTimeout,
validateTries,
emitInvalidHostnameWarning,
emitTypeCoercionDeprecationWarning,
getDefaultVerbatim,
} = require('internal/dns/utils');
const { codes, dnsException } = require('internal/errors');
Expand Down Expand Up @@ -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;
}

Expand Down
13 changes: 13 additions & 0 deletions lib/internal/dns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,18 @@ function emitInvalidHostnameWarning(hostname) {
);
}

let typeCoercionWarningEmitted = false;
function emitTypeCoercionDeprecationWarning() {

This comment has been minimized.

Copy link
@Trott

Trott Nov 21, 2021

Member

And....blocked.

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() {
Expand All @@ -213,6 +225,7 @@ module.exports = {
validateTries,
Resolver,
emitInvalidHostnameWarning,
emitTypeCoercionDeprecationWarning,
getDefaultVerbatim,
setDefaultResultOrder,
};
15 changes: 15 additions & 0 deletions test/internet/test-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
}
);

0 comments on commit f182b9b

Please sign in to comment.