From bb46bec64e1ec27c1dafb5797ba31356bd84288e Mon Sep 17 00:00:00 2001 From: meteorqz6 Date: Sat, 9 Aug 2025 18:46:20 +0900 Subject: [PATCH 1/2] lib: use validators for argument validation This refactors internal validation helpers in `child_process` to use the common validators in `lib/internal/validators.js` where possible. This improves code consistency and maintainability. --- lib/child_process.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index baa0a56d1ecdc7..1a87d5734a35bf 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -33,7 +33,6 @@ const { ArrayPrototypeSort, ArrayPrototypeSplice, ArrayPrototypeUnshift, - NumberIsInteger, ObjectAssign, ObjectDefineProperty, ObjectPrototypeHasOwnProperty, @@ -70,18 +69,19 @@ const { ERR_CHILD_PROCESS_STDIO_MAXBUFFER, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, - ERR_OUT_OF_RANGE, }, genericNodeError, } = require('internal/errors'); const { clearTimeout, setTimeout } = require('timers'); const { getValidatedPath } = require('internal/fs/utils'); const { - isInt32, validateAbortSignal, validateArray, validateBoolean, validateFunction, + validateInteger, + validateInt32, + validateNumber, validateObject, validateString, } = require('internal/validators'); @@ -603,13 +603,13 @@ function normalizeSpawnArguments(file, args, options) { } // Validate the uid, if present. - if (options.uid != null && !isInt32(options.uid)) { - throw new ERR_INVALID_ARG_TYPE('options.uid', 'int32', options.uid); + if (options.uid != null) { + validateInt32(options.uid, 'options.uid'); } // Validate the gid, if present. - if (options.gid != null && !isInt32(options.gid)) { - throw new ERR_INVALID_ARG_TYPE('options.gid', 'int32', options.gid); + if (options.gid != null) { + validateInt32(options.gid, 'options.gid'); } // Validate the shell, if present. @@ -1017,17 +1017,15 @@ function validateArgumentsNullCheck(args, propName) { function validateTimeout(timeout) { - if (timeout != null && !(NumberIsInteger(timeout) && timeout >= 0)) { - throw new ERR_OUT_OF_RANGE('timeout', 'an unsigned integer', timeout); + if (timeout != null) { + validateInteger(timeout, 'timeout', 0); } } function validateMaxBuffer(maxBuffer) { - if (maxBuffer != null && !(typeof maxBuffer === 'number' && maxBuffer >= 0)) { - throw new ERR_OUT_OF_RANGE('options.maxBuffer', - 'a positive number', - maxBuffer); + if (maxBuffer != null) { + validateNumber(maxBuffer, 'options.maxBuffer', 0); } } From 49c5ea111fb07b9498b787f4922f6291c935fa00 Mon Sep 17 00:00:00 2001 From: meteorqz6 Date: Sat, 16 Aug 2025 02:06:04 +0900 Subject: [PATCH 2/2] test: fix validation error assertions in child_process --- ...-child-process-fork-timeout-kill-signal.js | 4 +- ...child-process-spawn-timeout-kill-signal.js | 4 +- .../test-child-process-spawn-typeerror.js | 6 ++- ...ild-process-spawnsync-validation-errors.js | 40 +++++++++---------- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/test/parallel/test-child-process-fork-timeout-kill-signal.js b/test/parallel/test-child-process-fork-timeout-kill-signal.js index ef08d4b12ab94a..f3f605d7f5e24b 100644 --- a/test/parallel/test-child-process-fork-timeout-kill-signal.js +++ b/test/parallel/test-child-process-fork-timeout-kill-signal.js @@ -27,11 +27,11 @@ const { getEventListeners } = require('events'); // Verify timeout verification throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), { timeout: 'badValue', - }), /ERR_OUT_OF_RANGE/); + }), /ERR_INVALID_ARG_TYPE/); throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), { timeout: {}, - }), /ERR_OUT_OF_RANGE/); + }), /ERR_INVALID_ARG_TYPE/); } { diff --git a/test/parallel/test-child-process-spawn-timeout-kill-signal.js b/test/parallel/test-child-process-spawn-timeout-kill-signal.js index 59a974d0e0b448..a03a984b50ebbd 100644 --- a/test/parallel/test-child-process-spawn-timeout-kill-signal.js +++ b/test/parallel/test-child-process-spawn-timeout-kill-signal.js @@ -28,11 +28,11 @@ const aliveForeverFile = 'child-process-stay-alive-forever.js'; // Verify timeout verification throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], { timeout: 'badValue', - }), /ERR_OUT_OF_RANGE/); + }), /ERR_INVALID_ARG_TYPE/); throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], { timeout: {}, - }), /ERR_OUT_OF_RANGE/); + }), /ERR_INVALID_ARG_TYPE/); } { diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js index 545e2e2b3a4818..f7554e4484b48c 100644 --- a/test/parallel/test-child-process-spawn-typeerror.js +++ b/test/parallel/test-child-process-spawn-typeerror.js @@ -39,6 +39,8 @@ const invalidArgTypeError = { name: 'TypeError' }; +const invalidRangeError = { code: 'ERR_OUT_OF_RANGE', name: 'RangeError' }; + assert.throws(function() { spawn(invalidcmd, 'this is not an array'); }, invalidArgTypeError); @@ -77,11 +79,11 @@ assert.throws(function() { assert.throws(function() { spawn(cmd, [], { uid: 2 ** 63 }); -}, invalidArgTypeError); +}, invalidRangeError); assert.throws(function() { spawn(cmd, [], { gid: 2 ** 63 }); -}, invalidArgTypeError); +}, invalidRangeError); // Argument types for combinatorics. const a = []; diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js index a099ecfb63a812..2353e7594f356c 100644 --- a/test/parallel/test-child-process-spawnsync-validation-errors.js +++ b/test/parallel/test-child-process-spawnsync-validation-errors.js @@ -64,10 +64,10 @@ if (!common.isWindows) { fail('uid', [], invalidArgTypeError); fail('uid', {}, invalidArgTypeError); fail('uid', common.mustNotCall(), invalidArgTypeError); - fail('uid', NaN, invalidArgTypeError); - fail('uid', Infinity, invalidArgTypeError); - fail('uid', 3.1, invalidArgTypeError); - fail('uid', -3.1, invalidArgTypeError); + fail('uid', NaN, invalidRangeError); + fail('uid', Infinity, invalidRangeError); + fail('uid', 3.1, invalidRangeError); + fail('uid', -3.1, invalidRangeError); } } @@ -83,10 +83,10 @@ if (!common.isWindows) { fail('gid', [], invalidArgTypeError); fail('gid', {}, invalidArgTypeError); fail('gid', common.mustNotCall(), invalidArgTypeError); - fail('gid', NaN, invalidArgTypeError); - fail('gid', Infinity, invalidArgTypeError); - fail('gid', 3.1, invalidArgTypeError); - fail('gid', -3.1, invalidArgTypeError); + fail('gid', NaN, invalidRangeError); + fail('gid', Infinity, invalidRangeError); + fail('gid', 3.1, invalidRangeError); + fail('gid', -3.1, invalidRangeError); } } } @@ -152,12 +152,12 @@ if (!common.isWindows) { pass('timeout', 1); pass('timeout', 0); fail('timeout', -1, invalidRangeError); - fail('timeout', true, invalidRangeError); - fail('timeout', false, invalidRangeError); - fail('timeout', __dirname, invalidRangeError); - fail('timeout', [], invalidRangeError); - fail('timeout', {}, invalidRangeError); - fail('timeout', common.mustNotCall(), invalidRangeError); + fail('timeout', true, invalidArgTypeError); + fail('timeout', false, invalidArgTypeError); + fail('timeout', __dirname, invalidArgTypeError); + fail('timeout', [], invalidArgTypeError); + fail('timeout', {}, invalidArgTypeError); + fail('timeout', common.mustNotCall(), invalidArgTypeError); fail('timeout', NaN, invalidRangeError); fail('timeout', Infinity, invalidRangeError); fail('timeout', 3.1, invalidRangeError); @@ -175,12 +175,12 @@ if (!common.isWindows) { fail('maxBuffer', -1, invalidRangeError); fail('maxBuffer', NaN, invalidRangeError); fail('maxBuffer', -Infinity, invalidRangeError); - fail('maxBuffer', true, invalidRangeError); - fail('maxBuffer', false, invalidRangeError); - fail('maxBuffer', __dirname, invalidRangeError); - fail('maxBuffer', [], invalidRangeError); - fail('maxBuffer', {}, invalidRangeError); - fail('maxBuffer', common.mustNotCall(), invalidRangeError); + fail('maxBuffer', true, invalidArgTypeError); + fail('maxBuffer', false, invalidArgTypeError); + fail('maxBuffer', __dirname, invalidArgTypeError); + fail('maxBuffer', [], invalidArgTypeError); + fail('maxBuffer', {}, invalidArgTypeError); + fail('maxBuffer', common.mustNotCall(), invalidArgTypeError); } {