Skip to content

Commit 08773e3

Browse files
aduh95aduh95
committed
dns: remove dns.lookup and dnsPromises.lookup options type coercion
PR-URL: #41431 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 3a26db9 commit 08773e3

File tree

7 files changed

+121
-89
lines changed

7 files changed

+121
-89
lines changed

‎doc/api/deprecations.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2943,6 +2943,9 @@ deprecated and should no longer be used.
29432943

29442944
<!-- YAML
29452945
changes:
2946+
- version: REPLACEME
2947+
pr-url: https://github.com/nodejs/node/pull/41431
2948+
description: End-of-Life.
29462949
- version: v17.0.0
29472950
pr-url: https://github.com/nodejs/node/pull/39793
29482951
description: Runtime deprecation.
@@ -2951,12 +2954,13 @@ changes:
29512954
description: Documentation-only deprecation.
29522955
-->
29532956

2954-
Type: Runtime
2957+
Type: End-of-Life
29552958

29562959
Using a non-nullish non-integer value for `family` option, a non-nullish
29572960
non-number value for `hints` option, a non-nullish non-boolean value for `all`
29582961
option, or a non-nullish non-boolean value for `verbatim` option in
2959-
[`dns.lookup()`][] and [`dnsPromises.lookup()`][] is deprecated.
2962+
[`dns.lookup()`][] and [`dnsPromises.lookup()`][] throws an
2963+
`ERR_INVALID_ARG_TYPE` error.
29602964

29612965
### DEP0154: RSA-PSS generate key pair options
29622966

‎lib/dns.js‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ const {
4242
Resolver,
4343
validateHints,
4444
emitInvalidHostnameWarning,
45-
emitTypeCoercionDeprecationWarning,
4645
getDefaultVerbatim,
4746
setDefaultResultOrder,
4847
} = require('internal/dns/utils');
@@ -52,10 +51,12 @@ const {
5251
ERR_MISSING_ARGS,
5352
} = errors.codes;
5453
const {
54+
validateBoolean,
5555
validateFunction,
56+
validateNumber,
57+
validateOneOf,
5658
validatePort,
5759
validateString,
58-
validateOneOf,
5960
} = require('internal/validators');
6061

6162
const {
@@ -107,9 +108,10 @@ function onlookupall(err, addresses) {
107108

108109
// Easy DNS A/AAAA look up
109110
// lookup(hostname, [options,] callback)
111+
const validFamilies = [0, 4, 6];
110112
function lookup(hostname, options, callback) {
111113
let hints = 0;
112-
let family = -1;
114+
let family = 0;
113115
let all = false;
114116
let verbatim = getDefaultVerbatim();
115117

@@ -121,39 +123,36 @@ function lookup(hostname, options, callback) {
121123
if (typeof options === 'function') {
122124
callback = options;
123125
family = 0;
126+
} else if (typeof options === 'number') {
127+
validateFunction(callback, 'callback');
128+
129+
validateOneOf(options, 'family', validFamilies, true);
130+
family = options;
131+
} else if (options !== undefined && typeof options !== 'object') {
132+
validateFunction(arguments.length === 2 ? options : callback, 'callback');
133+
throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
124134
} else {
125135
validateFunction(callback, 'callback');
126136

127-
if (options !== null && typeof options === 'object') {
128-
if (options.hints != null && typeof options.hints !== 'number') {
129-
emitTypeCoercionDeprecationWarning();
130-
}
137+
if (options?.hints != null) {
138+
validateNumber(options.hints, 'options.hints');
131139
hints = options.hints >>> 0;
132-
if (options.family != null && typeof options.family !== 'number') {
133-
emitTypeCoercionDeprecationWarning();
134-
}
135-
family = options.family >>> 0;
136-
if (options.all != null && typeof options.all !== 'boolean') {
137-
emitTypeCoercionDeprecationWarning();
138-
}
139-
all = options.all === true;
140-
if (typeof options.verbatim === 'boolean') {
141-
verbatim = options.verbatim === true;
142-
} else if (options.verbatim != null) {
143-
emitTypeCoercionDeprecationWarning();
144-
}
145-
146140
validateHints(hints);
147-
} else {
148-
if (options != null && typeof options !== 'number') {
149-
emitTypeCoercionDeprecationWarning();
150-
}
151-
family = options >>> 0;
141+
}
142+
if (options?.family != null) {
143+
validateOneOf(options.family, 'options.family', validFamilies, true);
144+
family = options.family;
145+
}
146+
if (options?.all != null) {
147+
validateBoolean(options.all, 'options.all');
148+
all = options.all;
149+
}
150+
if (options?.verbatim != null) {
151+
validateBoolean(options.verbatim, 'options.verbatim');
152+
verbatim = options.verbatim;
152153
}
153154
}
154155

155-
validateOneOf(family, 'family', [0, 4, 6]);
156-
157156
if (!hostname) {
158157
emitInvalidHostnameWarning(hostname);
159158
if (all) {

‎lib/internal/dns/promises.js‎

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ const {
1515
validateTimeout,
1616
validateTries,
1717
emitInvalidHostnameWarning,
18-
emitTypeCoercionDeprecationWarning,
1918
getDefaultVerbatim,
2019
} = require('internal/dns/utils');
2120
const { codes, dnsException } = require('internal/errors');
@@ -30,13 +29,16 @@ const {
3029
QueryReqWrap
3130
} = internalBinding('cares_wrap');
3231
const {
32+
ERR_INVALID_ARG_TYPE,
3333
ERR_INVALID_ARG_VALUE,
3434
ERR_MISSING_ARGS,
3535
} = codes;
3636
const {
37+
validateBoolean,
38+
validateNumber,
39+
validateOneOf,
3740
validatePort,
3841
validateString,
39-
validateOneOf,
4042
} = require('internal/validators');
4143

4244
const kPerfHooksDnsLookupContext = Symbol('kPerfHooksDnsLookupContext');
@@ -120,9 +122,10 @@ function createLookupPromise(family, hostname, all, hints, verbatim) {
120122
});
121123
}
122124

125+
const validFamilies = [0, 4, 6];
123126
function lookup(hostname, options) {
124127
let hints = 0;
125-
let family = -1;
128+
let family = 0;
126129
let all = false;
127130
let verbatim = getDefaultVerbatim();
128131

@@ -131,35 +134,31 @@ function lookup(hostname, options) {
131134
validateString(hostname, 'hostname');
132135
}
133136

134-
if (options !== null && typeof options === 'object') {
135-
if (options.hints != null && typeof options.hints !== 'number') {
136-
emitTypeCoercionDeprecationWarning();
137+
if (typeof options === 'number') {
138+
validateOneOf(options, 'family', validFamilies, true);
139+
family = options;
140+
} else if (options !== undefined && typeof options !== 'object') {
141+
throw new ERR_INVALID_ARG_TYPE('options', ['integer', 'object'], options);
142+
} else {
143+
if (options?.hints != null) {
144+
validateNumber(options.hints, 'options.hints');
145+
hints = options.hints >>> 0;
146+
validateHints(hints);
137147
}
138-
hints = options.hints >>> 0;
139-
if (options.family != null && typeof options.family !== 'number') {
140-
emitTypeCoercionDeprecationWarning();
148+
if (options?.family != null) {
149+
validateOneOf(options.family, 'options.family', validFamilies, true);
150+
family = options.family;
141151
}
142-
family = options.family >>> 0;
143-
if (options.all != null && typeof options.all !== 'boolean') {
144-
emitTypeCoercionDeprecationWarning();
152+
if (options?.all != null) {
153+
validateBoolean(options.all, 'options.all');
154+
all = options.all;
145155
}
146-
all = options.all === true;
147-
if (typeof options.verbatim === 'boolean') {
148-
verbatim = options.verbatim === true;
149-
} else if (options.verbatim != null) {
150-
emitTypeCoercionDeprecationWarning();
156+
if (options?.verbatim != null) {
157+
validateBoolean(options.verbatim, 'options.verbatim');
158+
verbatim = options.verbatim;
151159
}
152-
153-
validateHints(hints);
154-
} else {
155-
if (options != null && typeof options !== 'number') {
156-
emitTypeCoercionDeprecationWarning();
157-
}
158-
family = options >>> 0;
159160
}
160161

161-
validateOneOf(family, 'family', [0, 4, 6], true);
162-
163162
return createLookupPromise(family, hostname, all, hints, verbatim);
164163
}
165164

‎lib/internal/dns/utils.js‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -190,18 +190,6 @@ function emitInvalidHostnameWarning(hostname) {
190190
}
191191
}
192192

193-
let typeCoercionWarningEmitted = false;
194-
function emitTypeCoercionDeprecationWarning() {
195-
if (!typeCoercionWarningEmitted) {
196-
process.emitWarning(
197-
'Type coercion of dns.lookup options is deprecated',
198-
'DeprecationWarning',
199-
'DEP0153'
200-
);
201-
typeCoercionWarningEmitted = true;
202-
}
203-
}
204-
205193
let dnsOrder = getOptionValue('--dns-result-order') || 'verbatim';
206194

207195
function getDefaultVerbatim() {
@@ -222,7 +210,6 @@ module.exports = {
222210
validateTries,
223211
Resolver,
224212
emitInvalidHostnameWarning,
225-
emitTypeCoercionDeprecationWarning,
226213
getDefaultVerbatim,
227214
setDefaultResultOrder,
228215
};

‎test/internet/test-dns-lookup.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,10 @@ dns.lookup(addresses.NOT_FOUND, {
4545
assert.strictEqual(error.hostname, addresses.NOT_FOUND);
4646
}));
4747

48-
common.expectWarning('DeprecationWarning',
49-
'Type coercion of dns.lookup options is deprecated',
50-
'DEP0153');
51-
52-
assert.rejects(
53-
dnsPromises.lookup(addresses.NOT_FOUND, {
48+
assert.throws(
49+
() => dnsPromises.lookup(addresses.NOT_FOUND, {
5450
family: 'IPv4',
5551
all: 'all'
5652
}),
57-
{
58-
code: 'ENOTFOUND',
59-
message: `getaddrinfo ENOTFOUND ${addresses.NOT_FOUND}`
60-
}
53+
{ code: 'ERR_INVALID_ARG_VALUE' }
6154
);

‎test/parallel/test-dns-lookup-promises-options-deprecated.js‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,21 @@ common.expectWarning({
1515
'internal/test/binding': [
1616
'These APIs are for internal testing only. Do not use them.',
1717
],
18-
'DeprecationWarning': {
19-
DEP0153: 'Type coercion of dns.lookup options is deprecated'
20-
}
2118
});
2219

2320
assert.throws(() => {
2421
dnsPromises.lookup('127.0.0.1', { hints: '-1' });
2522
}, {
26-
code: 'ERR_INVALID_ARG_VALUE',
23+
code: 'ERR_INVALID_ARG_TYPE',
2724
name: 'TypeError'
2825
});
29-
dnsPromises.lookup('127.0.0.1', { family: '6' });
30-
dnsPromises.lookup('127.0.0.1', { all: 'true' });
31-
dnsPromises.lookup('127.0.0.1', { verbatim: 'true' });
32-
dnsPromises.lookup('127.0.0.1', '6');
26+
assert.throws(() => dnsPromises.lookup('127.0.0.1', { hints: -1 }),
27+
{ code: 'ERR_INVALID_ARG_VALUE' });
28+
assert.throws(() => dnsPromises.lookup('127.0.0.1', { family: '6' }),
29+
{ code: 'ERR_INVALID_ARG_VALUE' });
30+
assert.throws(() => dnsPromises.lookup('127.0.0.1', { all: 'true' }),
31+
{ code: 'ERR_INVALID_ARG_TYPE' });
32+
assert.throws(() => dnsPromises.lookup('127.0.0.1', { verbatim: 'true' }),
33+
{ code: 'ERR_INVALID_ARG_TYPE' });
34+
assert.throws(() => dnsPromises.lookup('127.0.0.1', '6'),
35+
{ code: 'ERR_INVALID_ARG_TYPE' });

‎test/parallel/test-dns-lookup.js‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,15 @@ assert.throws(() => {
6969
}
7070

7171
{
72+
const family = 20;
7273
const err = {
7374
code: 'ERR_INVALID_ARG_VALUE',
7475
name: 'TypeError',
75-
message: "The argument 'family' must be one of: 0, 4, 6. Received 20"
76+
message: `The property 'options.family' must be one of: 0, 4, 6. Received ${family}`
7677
};
7778
const options = {
7879
hints: 0,
79-
family: 20,
80+
family,
8081
all: false
8182
};
8283

@@ -86,6 +87,52 @@ assert.throws(() => {
8687
}, err);
8788
}
8889

90+
[1, 0n, 1n, '', '0', Symbol(), true, false, {}, [], () => {}]
91+
.forEach((family) => {
92+
const err = { code: 'ERR_INVALID_ARG_VALUE' };
93+
const options = { family };
94+
assert.throws(() => { dnsPromises.lookup(false, options); }, err);
95+
assert.throws(() => {
96+
dns.lookup(false, options, common.mustNotCall());
97+
}, err);
98+
});
99+
[0n, 1n, '', '0', Symbol(), true, false].forEach((family) => {
100+
const err = { code: 'ERR_INVALID_ARG_TYPE' };
101+
assert.throws(() => { dnsPromises.lookup(false, family); }, err);
102+
assert.throws(() => {
103+
dns.lookup(false, family, common.mustNotCall());
104+
}, err);
105+
});
106+
assert.throws(() => dnsPromises.lookup(false, () => {}),
107+
{ code: 'ERR_INVALID_ARG_TYPE' });
108+
109+
[0n, 1n, '', '0', Symbol(), true, false, {}, [], () => {}].forEach((hints) => {
110+
const err = { code: 'ERR_INVALID_ARG_TYPE' };
111+
const options = { hints };
112+
assert.throws(() => { dnsPromises.lookup(false, options); }, err);
113+
assert.throws(() => {
114+
dns.lookup(false, options, common.mustNotCall());
115+
}, err);
116+
});
117+
118+
[0, 1, 0n, 1n, '', '0', Symbol(), {}, [], () => {}].forEach((all) => {
119+
const err = { code: 'ERR_INVALID_ARG_TYPE' };
120+
const options = { all };
121+
assert.throws(() => { dnsPromises.lookup(false, options); }, err);
122+
assert.throws(() => {
123+
dns.lookup(false, options, common.mustNotCall());
124+
}, err);
125+
});
126+
127+
[0, 1, 0n, 1n, '', '0', Symbol(), {}, [], () => {}].forEach((verbatim) => {
128+
const err = { code: 'ERR_INVALID_ARG_TYPE' };
129+
const options = { verbatim };
130+
assert.throws(() => { dnsPromises.lookup(false, options); }, err);
131+
assert.throws(() => {
132+
dns.lookup(false, options, common.mustNotCall());
133+
}, err);
134+
});
135+
89136
(async function() {
90137
let res;
91138

0 commit comments

Comments
 (0)