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 29, 2021
1 parent e937662 commit 4d09127
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 15 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
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
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 4d09127

Please sign in to comment.