Skip to content

Commit

Permalink
lib: make validateAbortSignal stricter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
VoltrexKeyva committed Oct 30, 2021
1 parent f8035ec commit 26cd607
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 18 deletions.
4 changes: 3 additions & 1 deletion lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
6 changes: 4 additions & 2 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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();

Expand Down
3 changes: 2 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion lib/internal/streams/end-of-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 2 additions & 4 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand Down
23 changes: 14 additions & 9 deletions lib/timers/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 26cd607

Please sign in to comment.