From 68862c494a08fe8f6d494b9001bec7483c54f4e2 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Thu, 5 Dec 2024 17:45:22 +0100 Subject: [PATCH 1/2] test: increase coverage of argument validation of cp methods --- .../parallel/test-child-process-exec-error.js | 40 +++++++++++++++++-- test/parallel/test-child-process-execfile.js | 30 ++++++++++++++ .../test-child-process-spawn-error.js | 15 +++++++ test/parallel/test-child-process-spawnsync.js | 15 +++++++ 4 files changed, 96 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-child-process-exec-error.js b/test/parallel/test-child-process-exec-error.js index cd45f3071c2920..59e661171556ca 100644 --- a/test/parallel/test-child-process-exec-error.js +++ b/test/parallel/test-child-process-exec-error.js @@ -22,7 +22,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const child_process = require('child_process'); +const { exec, execSync, execFile } = require('child_process'); function test(fn, code, expectPidType = 'number') { const child = fn('does-not-exist', common.mustCall(function(err) { @@ -35,10 +35,42 @@ function test(fn, code, expectPidType = 'number') { // With `shell: true`, expect pid (of the shell) if (common.isWindows) { - test(child_process.exec, 1, 'number'); // Exit code of cmd.exe + test(exec, 1, 'number'); // Exit code of cmd.exe } else { - test(child_process.exec, 127, 'number'); // Exit code of /bin/sh + test(exec, 127, 'number'); // Exit code of /bin/sh } // With `shell: false`, expect no pid -test(child_process.execFile, 'ENOENT', 'undefined'); +test(execFile, 'ENOENT', 'undefined'); + + +// Verify that the exec() function throws when command parameter is not a valid string +{ + assert.throws(() => { + exec(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + exec('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + exec('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} + + +// Verify that the execSync() function throws when command parameter is not a valid string +{ + assert.throws(() => { + execSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js index 9c1733e2603900..cfe6619f737d5b 100644 --- a/test/parallel/test-child-process-execfile.js +++ b/test/parallel/test-child-process-execfile.js @@ -131,3 +131,33 @@ common.expectWarning( })); }); } + +// Verify that the execFile() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + execFile(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execFile('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execFile('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} + +// Verify that the execFileSync() function throws when file parameter is not a valid string +{ + assert.throws(() => { + execFileSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execFileSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execFileSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-spawn-error.js b/test/parallel/test-child-process-spawn-error.js index ed1c8fac9f97ed..44b09c62a49642 100644 --- a/test/parallel/test-child-process-spawn-error.js +++ b/test/parallel/test-child-process-spawn-error.js @@ -53,3 +53,18 @@ enoentChild.on('error', common.mustCall(function(err) { assert.strictEqual(err.path, enoentPath); assert.deepStrictEqual(err.spawnargs, spawnargs); })); + +// Verify that the spawn() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + spawn(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + spawn('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + spawn('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-spawnsync.js b/test/parallel/test-child-process-spawnsync.js index 9ec125ea891689..5d89af35db0de1 100644 --- a/test/parallel/test-child-process-spawnsync.js +++ b/test/parallel/test-child-process-spawnsync.js @@ -65,3 +65,18 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']); ]; assert.deepStrictEqual(retUTF8.output, stringifiedDefault); } + +// Verify that the spawnSync() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + spawnSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + spawnSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + spawnSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} From fce9b3a93e447be5105e9cad2da6ed80aade8285 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Thu, 9 Jan 2025 09:43:21 +0100 Subject: [PATCH 2/2] child_process: refactor string validation in exec and spawn --- lib/child_process.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 824af65556e32b..90a47e469e0eaf 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -189,9 +189,6 @@ function _forkChild(fd, serializationMode) { } function normalizeExecArgs(command, options, callback) { - validateString(command, 'command'); - validateArgumentNullCheck(command, 'command'); - if (typeof options === 'function') { callback = options; options = undefined; @@ -568,9 +565,9 @@ let emittedDEP0190Already = false; function normalizeSpawnArguments(file, args, options) { validateString(file, 'file'); validateArgumentNullCheck(file, 'file'); - - if (file.length === 0) + if (file.length === 0) { throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty'); + } if (ArrayIsArray(args)) { args = ArrayPrototypeSlice(args);