From 8a4b26f00c55bbb3217accd1d9e4f75851e5305d Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Sun, 25 Aug 2024 09:30:10 +0200 Subject: [PATCH] timers: fix validation PR-URL: https://github.com/nodejs/node/pull/54404 Reviewed-By: Claudio Wunder --- doc/api/timers.md | 5 +- lib/timers/promises.js | 91 +++++++++++-------- .../test-timers-timeout-promisified.js | 38 +++----- 3 files changed, 68 insertions(+), 66 deletions(-) diff --git a/doc/api/timers.md b/doc/api/timers.md index c9744aaf52cc32..3a4ebe834b934c 100644 --- a/doc/api/timers.md +++ b/doc/api/timers.md @@ -532,9 +532,8 @@ added: An experimental API defined by the [Scheduling APIs][] draft specification being developed as a standard Web Platform API. -Calling `timersPromises.scheduler.wait(delay, options)` is roughly equivalent -to calling `timersPromises.setTimeout(delay, undefined, options)` except that -the `ref` option is not supported. +Calling `timersPromises.scheduler.wait(delay, options)` is equivalent +to calling `timersPromises.setTimeout(delay, undefined, options)`. ```mjs import { scheduler } from 'node:timers/promises'; diff --git a/lib/timers/promises.js b/lib/timers/promises.js index 2bf36d6cc51700..d577ac9240ec17 100644 --- a/lib/timers/promises.js +++ b/lib/timers/promises.js @@ -24,7 +24,6 @@ const { AbortError, codes: { ERR_ILLEGAL_CONSTRUCTOR, - ERR_INVALID_ARG_TYPE, ERR_INVALID_THIS, }, } = require('internal/errors'); @@ -33,6 +32,7 @@ const { validateAbortSignal, validateBoolean, validateObject, + validateNumber, } = require('internal/validators'); const { @@ -50,34 +50,33 @@ function cancelListenerHandler(clear, reject, signal) { } function setTimeout(after, value, options = kEmptyObject) { - const args = value !== undefined ? [value] : value; - if (options == null || typeof options !== 'object') { - return PromiseReject( - new ERR_INVALID_ARG_TYPE( - 'options', - 'Object', - options)); - } - const { signal, ref = true } = options; try { - validateAbortSignal(signal, 'options.signal'); + if (typeof after !== 'undefined') { + validateNumber(after, 'delay'); + } + + validateObject(options, 'options'); + + if (typeof options?.signal !== 'undefined') { + validateAbortSignal(options.signal, 'options.signal'); + } + + if (typeof options?.ref !== 'undefined') { + validateBoolean(options.ref, 'options.ref'); + } } catch (err) { return PromiseReject(err); } - if (typeof ref !== 'boolean') { - return PromiseReject( - new ERR_INVALID_ARG_TYPE( - 'options.ref', - 'boolean', - ref)); - } + + const { signal, ref = true } = options; if (signal?.aborted) { return PromiseReject(new AbortError(undefined, { cause: signal.reason })); } + let oncancel; const ret = new Promise((resolve, reject) => { - const timeout = new Timeout(resolve, after, args, false, ref); + const timeout = new Timeout(resolve, after, [value], false, ref); insert(timeout, timeout._idleTimeout); if (signal) { oncancel = FunctionPrototypeBind(cancelListenerHandler, @@ -93,30 +92,26 @@ function setTimeout(after, value, options = kEmptyObject) { } function setImmediate(value, options = kEmptyObject) { - if (options == null || typeof options !== 'object') { - return PromiseReject( - new ERR_INVALID_ARG_TYPE( - 'options', - 'Object', - options)); - } - const { signal, ref = true } = options; try { - validateAbortSignal(signal, 'options.signal'); + validateObject(options, 'options'); + + if (typeof options?.signal !== 'undefined') { + validateAbortSignal(options.signal, 'options.signal'); + } + + if (typeof options?.ref !== 'undefined') { + validateBoolean(options.ref, 'options.ref'); + } } catch (err) { return PromiseReject(err); } - if (typeof ref !== 'boolean') { - return PromiseReject( - new ERR_INVALID_ARG_TYPE( - 'options.ref', - 'boolean', - ref)); - } + + const { signal, ref = true } = options; if (signal?.aborted) { return PromiseReject(new AbortError(undefined, { cause: signal.reason })); } + let oncancel; const ret = new Promise((resolve, reject) => { const immediate = new Immediate(resolve, [value]); @@ -136,13 +131,29 @@ function setImmediate(value, options = kEmptyObject) { } async function* setInterval(after, value, options = kEmptyObject) { - validateObject(options, 'options'); + try { + if (typeof after !== 'undefined') { + validateNumber(after, 'delay'); + } + + validateObject(options, 'options'); + + if (typeof options?.signal !== 'undefined') { + validateAbortSignal(options.signal, 'options.signal'); + } + + if (typeof options?.ref !== 'undefined') { + validateBoolean(options.ref, 'options.ref'); + } + } catch (err) { + return PromiseReject(err); + } + const { signal, ref = true } = options; - validateAbortSignal(signal, 'options.signal'); - validateBoolean(ref, 'options.ref'); - if (signal?.aborted) + if (signal?.aborted) { throw new AbortError(undefined, { cause: signal?.reason }); + } let onCancel; let interval; @@ -216,7 +227,7 @@ class Scheduler { wait(delay, options) { if (!this[kScheduler]) throw new ERR_INVALID_THIS('Scheduler'); - return setTimeout(delay, undefined, { signal: options?.signal }); + return setTimeout(delay, undefined, options); } } diff --git a/test/parallel/test-timers-timeout-promisified.js b/test/parallel/test-timers-timeout-promisified.js index a63923b7feea86..a89ccad7305f89 100644 --- a/test/parallel/test-timers-timeout-promisified.js +++ b/test/parallel/test-timers-timeout-promisified.js @@ -63,29 +63,21 @@ process.on('multipleResolves', common.mustNotCall()); } { - Promise.all( - [1, '', false, Infinity].map( - (i) => assert.rejects(setPromiseTimeout(10, null, i), { - code: 'ERR_INVALID_ARG_TYPE' - }) - ) - ).then(common.mustCall()); - - Promise.all( - [1, '', false, Infinity, null, {}].map( - (signal) => assert.rejects(setPromiseTimeout(10, null, { signal }), { - code: 'ERR_INVALID_ARG_TYPE' - }) - ) - ).then(common.mustCall()); - - Promise.all( - [1, '', Infinity, null, {}].map( - (ref) => assert.rejects(setPromiseTimeout(10, null, { ref }), { - code: 'ERR_INVALID_ARG_TYPE' - }) - ) - ).then(common.mustCall()); + for (const delay of ['', false]) { + assert.rejects(() => setPromiseTimeout(delay, null, {}), /ERR_INVALID_ARG_TYPE/).then(common.mustCall()); + } + + for (const options of [1, '', false, Infinity]) { + assert.rejects(() => setPromiseTimeout(10, null, options), /ERR_INVALID_ARG_TYPE/).then(common.mustCall()); + } + + for (const signal of [1, '', false, Infinity, null, {}]) { + assert.rejects(() => setPromiseTimeout(10, null, { signal }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall()); + } + + for (const ref of [1, '', Infinity, null, {}]) { + assert.rejects(() => setPromiseTimeout(10, null, { ref }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall()); + } } {