Skip to content

Commit 8a4b26f

Browse files
ShogunPandaRafaelGSS
authored andcommittedAug 30, 2024
timers: fix validation
PR-URL: #54404 Reviewed-By: Claudio Wunder <cwunder@gnome.org>
1 parent 02d664b commit 8a4b26f

File tree

3 files changed

+68
-66
lines changed

3 files changed

+68
-66
lines changed
 

‎doc/api/timers.md

+2-3
Original file line numberDiff line numberDiff line change
@@ -532,9 +532,8 @@ added:
532532
An experimental API defined by the [Scheduling APIs][] draft specification
533533
being developed as a standard Web Platform API.
534534

535-
Calling `timersPromises.scheduler.wait(delay, options)` is roughly equivalent
536-
to calling `timersPromises.setTimeout(delay, undefined, options)` except that
537-
the `ref` option is not supported.
535+
Calling `timersPromises.scheduler.wait(delay, options)` is equivalent
536+
to calling `timersPromises.setTimeout(delay, undefined, options)`.
538537

539538
```mjs
540539
import { scheduler } from 'node:timers/promises';

‎lib/timers/promises.js

+51-40
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ const {
2424
AbortError,
2525
codes: {
2626
ERR_ILLEGAL_CONSTRUCTOR,
27-
ERR_INVALID_ARG_TYPE,
2827
ERR_INVALID_THIS,
2928
},
3029
} = require('internal/errors');
@@ -33,6 +32,7 @@ const {
3332
validateAbortSignal,
3433
validateBoolean,
3534
validateObject,
35+
validateNumber,
3636
} = require('internal/validators');
3737

3838
const {
@@ -50,34 +50,33 @@ function cancelListenerHandler(clear, reject, signal) {
5050
}
5151

5252
function setTimeout(after, value, options = kEmptyObject) {
53-
const args = value !== undefined ? [value] : value;
54-
if (options == null || typeof options !== 'object') {
55-
return PromiseReject(
56-
new ERR_INVALID_ARG_TYPE(
57-
'options',
58-
'Object',
59-
options));
60-
}
61-
const { signal, ref = true } = options;
6253
try {
63-
validateAbortSignal(signal, 'options.signal');
54+
if (typeof after !== 'undefined') {
55+
validateNumber(after, 'delay');
56+
}
57+
58+
validateObject(options, 'options');
59+
60+
if (typeof options?.signal !== 'undefined') {
61+
validateAbortSignal(options.signal, 'options.signal');
62+
}
63+
64+
if (typeof options?.ref !== 'undefined') {
65+
validateBoolean(options.ref, 'options.ref');
66+
}
6467
} catch (err) {
6568
return PromiseReject(err);
6669
}
67-
if (typeof ref !== 'boolean') {
68-
return PromiseReject(
69-
new ERR_INVALID_ARG_TYPE(
70-
'options.ref',
71-
'boolean',
72-
ref));
73-
}
70+
71+
const { signal, ref = true } = options;
7472

7573
if (signal?.aborted) {
7674
return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
7775
}
76+
7877
let oncancel;
7978
const ret = new Promise((resolve, reject) => {
80-
const timeout = new Timeout(resolve, after, args, false, ref);
79+
const timeout = new Timeout(resolve, after, [value], false, ref);
8180
insert(timeout, timeout._idleTimeout);
8281
if (signal) {
8382
oncancel = FunctionPrototypeBind(cancelListenerHandler,
@@ -93,30 +92,26 @@ function setTimeout(after, value, options = kEmptyObject) {
9392
}
9493

9594
function setImmediate(value, options = kEmptyObject) {
96-
if (options == null || typeof options !== 'object') {
97-
return PromiseReject(
98-
new ERR_INVALID_ARG_TYPE(
99-
'options',
100-
'Object',
101-
options));
102-
}
103-
const { signal, ref = true } = options;
10495
try {
105-
validateAbortSignal(signal, 'options.signal');
96+
validateObject(options, 'options');
97+
98+
if (typeof options?.signal !== 'undefined') {
99+
validateAbortSignal(options.signal, 'options.signal');
100+
}
101+
102+
if (typeof options?.ref !== 'undefined') {
103+
validateBoolean(options.ref, 'options.ref');
104+
}
106105
} catch (err) {
107106
return PromiseReject(err);
108107
}
109-
if (typeof ref !== 'boolean') {
110-
return PromiseReject(
111-
new ERR_INVALID_ARG_TYPE(
112-
'options.ref',
113-
'boolean',
114-
ref));
115-
}
108+
109+
const { signal, ref = true } = options;
116110

117111
if (signal?.aborted) {
118112
return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
119113
}
114+
120115
let oncancel;
121116
const ret = new Promise((resolve, reject) => {
122117
const immediate = new Immediate(resolve, [value]);
@@ -136,13 +131,29 @@ function setImmediate(value, options = kEmptyObject) {
136131
}
137132

138133
async function* setInterval(after, value, options = kEmptyObject) {
139-
validateObject(options, 'options');
134+
try {
135+
if (typeof after !== 'undefined') {
136+
validateNumber(after, 'delay');
137+
}
138+
139+
validateObject(options, 'options');
140+
141+
if (typeof options?.signal !== 'undefined') {
142+
validateAbortSignal(options.signal, 'options.signal');
143+
}
144+
145+
if (typeof options?.ref !== 'undefined') {
146+
validateBoolean(options.ref, 'options.ref');
147+
}
148+
} catch (err) {
149+
return PromiseReject(err);
150+
}
151+
140152
const { signal, ref = true } = options;
141-
validateAbortSignal(signal, 'options.signal');
142-
validateBoolean(ref, 'options.ref');
143153

144-
if (signal?.aborted)
154+
if (signal?.aborted) {
145155
throw new AbortError(undefined, { cause: signal?.reason });
156+
}
146157

147158
let onCancel;
148159
let interval;
@@ -216,7 +227,7 @@ class Scheduler {
216227
wait(delay, options) {
217228
if (!this[kScheduler])
218229
throw new ERR_INVALID_THIS('Scheduler');
219-
return setTimeout(delay, undefined, { signal: options?.signal });
230+
return setTimeout(delay, undefined, options);
220231
}
221232
}
222233

‎test/parallel/test-timers-timeout-promisified.js

+15-23
Original file line numberDiff line numberDiff line change
@@ -63,29 +63,21 @@ process.on('multipleResolves', common.mustNotCall());
6363
}
6464

6565
{
66-
Promise.all(
67-
[1, '', false, Infinity].map(
68-
(i) => assert.rejects(setPromiseTimeout(10, null, i), {
69-
code: 'ERR_INVALID_ARG_TYPE'
70-
})
71-
)
72-
).then(common.mustCall());
73-
74-
Promise.all(
75-
[1, '', false, Infinity, null, {}].map(
76-
(signal) => assert.rejects(setPromiseTimeout(10, null, { signal }), {
77-
code: 'ERR_INVALID_ARG_TYPE'
78-
})
79-
)
80-
).then(common.mustCall());
81-
82-
Promise.all(
83-
[1, '', Infinity, null, {}].map(
84-
(ref) => assert.rejects(setPromiseTimeout(10, null, { ref }), {
85-
code: 'ERR_INVALID_ARG_TYPE'
86-
})
87-
)
88-
).then(common.mustCall());
66+
for (const delay of ['', false]) {
67+
assert.rejects(() => setPromiseTimeout(delay, null, {}), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
68+
}
69+
70+
for (const options of [1, '', false, Infinity]) {
71+
assert.rejects(() => setPromiseTimeout(10, null, options), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
72+
}
73+
74+
for (const signal of [1, '', false, Infinity, null, {}]) {
75+
assert.rejects(() => setPromiseTimeout(10, null, { signal }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
76+
}
77+
78+
for (const ref of [1, '', Infinity, null, {}]) {
79+
assert.rejects(() => setPromiseTimeout(10, null, { ref }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
80+
}
8981
}
9082

9183
{

0 commit comments

Comments
 (0)
Please sign in to comment.