From f26240fdb7a8f026351f1fdd20291dd6e897c17b Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Sat, 24 Aug 2024 09:21:33 +0200 Subject: [PATCH] timers: fix validation --- doc/api/timers.md | 5 +- lib/timers/promises.js | 94 +++++++++++-------- .../test-timers-timeout-promisified.js | 38 +++----- 3 files changed, 70 insertions(+), 67 deletions(-) diff --git a/doc/api/timers.md b/doc/api/timers.md index ef4e114fce3adc..6ddea393e4c922 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..45a5946061af07 100644 --- a/lib/timers/promises.js +++ b/lib/timers/promises.js @@ -33,6 +33,7 @@ const { validateAbortSignal, validateBoolean, validateObject, + validateNumber, } = require('internal/validators'); const { @@ -50,34 +51,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'); - } catch (err) { + 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 +93,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'); - } catch (err) { + 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 +132,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 +228,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..1dd382b878698e 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()); + } } {