From 0cf699933317fc75f032552c258a68a5e7de7a56 Mon Sep 17 00:00:00 2001 From: voltrexmaster Date: Fri, 29 Oct 2021 07:55:37 -0700 Subject: [PATCH] lib: make validateAbortSignal stricter We should not force-check if the passed signal is **NOT** undefined and then proceed to check whether the value is an abort signal or not as the validator won't throw an error if the value is undefined as we straight up validate the values instead of checking if they're not undefined first. This unnecessary check can be done explicitly outside of the validator to only pass the value to the validator if its not undefined. --- lib/child_process.js | 4 +++- lib/events.js | 6 ++++-- lib/internal/fs/promises.js | 3 ++- lib/internal/fs/watchers.js | 3 ++- lib/internal/streams/end-of-stream.js | 3 ++- lib/internal/streams/pipeline.js | 3 ++- lib/internal/validators.js | 6 ++---- lib/timers/promises.js | 23 ++++++++++++++--------- 8 files changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 62c552d567ea..7f6621734b10 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -690,7 +690,9 @@ function abortChildProcess(child, killSignal) { function spawn(file, args, options) { options = normalizeSpawnArguments(file, args, options); validateTimeout(options.timeout); - validateAbortSignal(options.signal, 'options.signal'); + if (options.signal !== undefined) + validateAbortSignal(options.signal, 'options.signal'); + const killSignal = sanitizeKillSignal(options.killSignal); const child = new ChildProcess(); diff --git a/lib/events.js b/lib/events.js index ef8090e57778..64273507efd0 100644 --- a/lib/events.js +++ b/lib/events.js @@ -808,7 +808,8 @@ function getEventListeners(emitterOrTarget, type) { */ async function once(emitter, name, options = {}) { const signal = options?.signal; - validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined) + validateAbortSignal(signal, 'options.signal'); if (signal?.aborted) throw new AbortError(); return new Promise((resolve, reject) => { @@ -886,7 +887,8 @@ function eventTargetAgnosticAddListener(emitter, name, listener, flags) { */ function on(emitter, event, options) { const signal = options?.signal; - validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined) + validateAbortSignal(signal, 'options.signal'); if (signal?.aborted) throw new AbortError(); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index b9c3b7cb4071..cf77e9ad7f2c 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -813,7 +813,8 @@ async function writeFile(path, data, options) { data = Buffer.from(data, options.encoding || 'utf8'); } - validateAbortSignal(options.signal); + if (options.signal !== undefined) + validateAbortSignal(options.signal); if (path instanceof FileHandle) return writeFileHandle(path, data, options.signal, options.encoding); diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index b45af42d12ff..37fbc06a74c0 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -308,7 +308,8 @@ async function* watch(filename, options = {}) { validateBoolean(persistent, 'options.persistent'); validateBoolean(recursive, 'options.recursive'); - validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined) + validateAbortSignal(signal, 'options.signal'); if (encoding && !isEncoding(encoding)) { const reason = 'is invalid encoding'; diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index c224b44eac66..da8fe36fd751 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -45,7 +45,8 @@ function eos(stream, options, callback) { validateObject(options, 'options'); } validateFunction(callback, 'callback'); - validateAbortSignal(options.signal, 'options.signal'); + if (options.signal !== undefined) + validateAbortSignal(options.signal, 'options.signal'); callback = once(callback); diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js index 8dc4e5792c47..85a5e8025a05 100644 --- a/lib/internal/streams/pipeline.js +++ b/lib/internal/streams/pipeline.js @@ -182,7 +182,8 @@ function pipelineImpl(streams, callback, opts) { const signal = ac.signal; const outerSignal = opts?.signal; - validateAbortSignal(outerSignal, 'options.signal'); + if (outerSignal !== undefined) + validateAbortSignal(outerSignal, 'options.signal'); function abort() { finishImpl(new AbortError()); diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 81329160f073..a9d884f0bf07 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -224,10 +224,8 @@ const validateCallback = hideStackFrames((callback) => { }); const validateAbortSignal = hideStackFrames((signal, name) => { - if (signal !== undefined && - (signal === null || - typeof signal !== 'object' || - !('aborted' in signal))) { + if (signal === null || typeof signal !== 'object' || + !('aborted' in signal)) { throw new ERR_INVALID_ARG_TYPE(name, 'AbortSignal', signal); } }); diff --git a/lib/timers/promises.js b/lib/timers/promises.js index 162f465da29d..52314c3e81f9 100644 --- a/lib/timers/promises.js +++ b/lib/timers/promises.js @@ -41,10 +41,12 @@ function setTimeout(after, value, options = {}) { options)); } const { signal, ref = true } = options; - try { - validateAbortSignal(signal, 'options.signal'); - } catch (err) { - return PromiseReject(err); + if (signal !== undefined) { + try { + validateAbortSignal(signal, 'options.signal'); + } catch (err) { + return PromiseReject(err); + } } if (typeof ref !== 'boolean') { return PromiseReject( @@ -85,10 +87,12 @@ function setImmediate(value, options = {}) { options)); } const { signal, ref = true } = options; - try { - validateAbortSignal(signal, 'options.signal'); - } catch (err) { - return PromiseReject(err); + if (signal !== undefined) { + try { + validateAbortSignal(signal, 'options.signal'); + } catch (err) { + return PromiseReject(err); + } } if (typeof ref !== 'boolean') { return PromiseReject( @@ -123,7 +127,8 @@ function setImmediate(value, options = {}) { async function* setInterval(after, value, options = {}) { validateObject(options, 'options'); const { signal, ref = true } = options; - validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined) + validateAbortSignal(signal, 'options.signal'); validateBoolean(ref, 'options.ref'); if (signal?.aborted)