Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
child_process: improve ChildProcess validation
This commit improves input validation for the ChildProcess
internals. It became officially supported API a while back, but
never had any validation.

Refs: #12177
PR-URL: #12348
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
  • Loading branch information
cjihrig committed Apr 17, 2017
1 parent 59c6230 commit 97a7728
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 3 deletions.
22 changes: 19 additions & 3 deletions lib/internal/child_process.js
Expand Up @@ -254,6 +254,9 @@ ChildProcess.prototype.spawn = function(options) {
var ipcFd;
var i;

if (options === null || typeof options !== 'object')
throw new TypeError('"options" must be an object');

// If no `stdio` option was given - use default
var stdio = options.stdio || 'pipe';

Expand All @@ -265,12 +268,25 @@ ChildProcess.prototype.spawn = function(options) {

if (ipc !== undefined) {
// Let child process know about opened IPC channel
options.envPairs = options.envPairs || [];
if (options.envPairs === undefined)
options.envPairs = [];
else if (!Array.isArray(options.envPairs))
throw new TypeError('"envPairs" must be an array');

options.envPairs.push('NODE_CHANNEL_FD=' + ipcFd);
}

this.spawnfile = options.file;
this.spawnargs = options.args;
if (typeof options.file === 'string')
this.spawnfile = options.file;
else
throw new TypeError('"file" must be a string');

if (Array.isArray(options.args))
this.spawnargs = options.args;
else if (options.args === undefined)
this.spawnargs = [];
else
throw new TypeError('"args" must be an array');

var err = this._handle.spawn(options);

Expand Down
44 changes: 44 additions & 0 deletions test/parallel/test-child-process-constructor.js
Expand Up @@ -5,6 +5,50 @@ const assert = require('assert');
const { ChildProcess } = require('child_process');
assert.strictEqual(typeof ChildProcess, 'function');

{
// Verify that invalid options to spawn() throw.
const child = new ChildProcess();

[undefined, null, 'foo', 0, 1, NaN, true, false].forEach((options) => {
assert.throws(() => {
child.spawn(options);
}, /^TypeError: "options" must be an object$/);
});
}

{
// Verify that spawn throws if file is not a string.
const child = new ChildProcess();

[undefined, null, 0, 1, NaN, true, false, {}].forEach((file) => {
assert.throws(() => {
child.spawn({ file });
}, /^TypeError: "file" must be a string$/);
});
}

{
// Verify that spawn throws if envPairs is not an array or undefined.
const child = new ChildProcess();

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((envPairs) => {
assert.throws(() => {
child.spawn({ envPairs, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] });
}, /^TypeError: "envPairs" must be an array$/);
});
}

{
// Verify that spawn throws if args is not an array or undefined.
const child = new ChildProcess();

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((args) => {
assert.throws(() => {
child.spawn({ file: 'foo', args });
}, /^TypeError: "args" must be an array$/);
});
}

// test that we can call spawn
const child = new ChildProcess();
child.spawn({
Expand Down

0 comments on commit 97a7728

Please sign in to comment.