diff --git a/lib/child_process.js b/lib/child_process.js index 6fe5351fd6fdda..e9cf426b9ecfbb 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -146,6 +146,12 @@ exports.execFile = function(file /*, args, options, callback*/) { throw new TypeError('Incorrect value of args option'); } + // Validate the timeout, if present. + validateTimeout(options.timeout); + + // Validate maxBuffer, if present. + validateMaxBuffer(options.maxBuffer); + var child = spawn(file, args, { cwd: options.cwd, env: options.env, @@ -466,31 +472,17 @@ function spawnSync(/*file, args, options*/) { debug('spawnSync', opts.args, options); // Validate the timeout, if present. - if (options.timeout != null && - !(Number.isInteger(options.timeout) && options.timeout >= 0)) { - throw new TypeError('"timeout" must be an unsigned integer'); - } + validateTimeout(options.timeout); // Validate maxBuffer, if present. - if (options.maxBuffer != null && - !(Number.isInteger(options.maxBuffer) && options.maxBuffer >= 0)) { - throw new TypeError('"maxBuffer" must be an unsigned integer'); - } + validateMaxBuffer(options.maxBuffer); options.file = opts.file; options.args = opts.args; options.envPairs = opts.envPairs; - // Validate the kill signal, if present. - if (typeof options.killSignal === 'string' || - typeof options.killSignal === 'number') { - options.killSignal = lookupSignal(options.killSignal); - - if (options.killSignal === 0) - throw new RangeError('"killSignal" cannot be 0'); - } else if (options.killSignal != null) { - throw new TypeError('"killSignal" must be a string or number'); - } + // Validate and translate the kill signal, if present. + options.killSignal = validateKillSignal(options.killSignal); options.stdio = _validateStdio(options.stdio || 'pipe', true).stdio; @@ -600,3 +592,31 @@ function execSync(command /*, options*/) { return ret.stdout; } exports.execSync = execSync; + + +function validateTimeout(timeout) { + if (timeout != null && !(Number.isInteger(timeout) && timeout >= 0)) { + throw new TypeError('"timeout" must be an unsigned integer'); + } +} + + +function validateMaxBuffer(maxBuffer) { + if (maxBuffer != null && !(typeof maxBuffer === 'number' && maxBuffer >= 0)) { + throw new TypeError('"maxBuffer" must be a positive number'); + } +} + + +function validateKillSignal(killSignal) { + if (typeof killSignal === 'string' || typeof killSignal === 'number') { + killSignal = lookupSignal(killSignal); + + if (killSignal === 0) + throw new RangeError('"killSignal" cannot be 0'); + } else if (killSignal != null) { + throw new TypeError('"killSignal" must be a string or number'); + } + + return killSignal; +} diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 68e78147578ff1..93e51af38f65ae 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -765,8 +765,8 @@ int SyncProcessRunner::ParseOptions(Local js_value) { Local js_max_buffer = js_options->Get(env()->max_buffer_string()); if (IsSet(js_max_buffer)) { - CHECK(js_max_buffer->IsUint32()); - max_buffer_ = js_max_buffer->Uint32Value(); + CHECK(js_max_buffer->IsNumber()); + max_buffer_ = js_max_buffer->NumberValue(); } Local js_kill_signal = js_options->Get(env()->kill_signal_string()); diff --git a/src/spawn_sync.h b/src/spawn_sync.h index 8f4c05aa5f4768..676df43b759bf1 100644 --- a/src/spawn_sync.h +++ b/src/spawn_sync.h @@ -184,7 +184,7 @@ class SyncProcessRunner { static void KillTimerCallback(uv_timer_t* handle); static void KillTimerCloseCallback(uv_handle_t* handle); - size_t max_buffer_; + double max_buffer_; uint64_t timeout_; int kill_signal_; diff --git a/test/parallel/test-child-process-exec-maxBuffer.js b/test/parallel/test-child-process-exec-maxBuffer.js index 714e029d728b1e..65674a8503c3d2 100644 --- a/test/parallel/test-child-process-exec-maxBuffer.js +++ b/test/parallel/test-child-process-exec-maxBuffer.js @@ -10,6 +10,17 @@ function checkFactory(streamName) { }); } +{ + const cmd = `"${process.execPath}" -e "console.log('hello world');"`; + const options = { maxBuffer: Infinity }; + + cp.exec(cmd, options, common.mustCall((err, stdout, stderr) => { + assert.ifError(err); + assert.strictEqual(stdout.trim(), 'hello world'); + assert.strictEqual(stderr, ''); + })); +} + { const cmd = 'echo "hello world"'; diff --git a/test/parallel/test-child-process-spawnsync-maxbuf.js b/test/parallel/test-child-process-spawnsync-maxbuf.js index 87099737ecb5b0..d9c2c4d23e17fd 100644 --- a/test/parallel/test-child-process-spawnsync-maxbuf.js +++ b/test/parallel/test-child-process-spawnsync-maxbuf.js @@ -1,9 +1,7 @@ 'use strict'; require('../common'); const assert = require('assert'); - const spawnSync = require('child_process').spawnSync; - const msgOut = 'this is stdout'; // This is actually not os.EOL? @@ -14,14 +12,21 @@ const args = [ `console.log("${msgOut}");` ]; -const options = { - maxBuffer: 1 -}; +// Verify that an error is returned if maxBuffer is surpassed. +{ + const ret = spawnSync(process.execPath, args, { maxBuffer: 1 }); + + assert.ok(ret.error, 'maxBuffer should error'); + assert.strictEqual(ret.error.errno, 'ENOBUFS'); + // We can have buffers larger than maxBuffer because underneath we alloc 64k + // that matches our read sizes. + assert.deepStrictEqual(ret.stdout, msgOutBuf); +} -const ret = spawnSync(process.execPath, args, options); +// Verify that a maxBuffer size of Infinity works. +{ + const ret = spawnSync(process.execPath, args, { maxBuffer: Infinity }); -assert.ok(ret.error, 'maxBuffer should error'); -assert.strictEqual(ret.error.errno, 'ENOBUFS'); -// We can have buffers larger than maxBuffer because underneath we alloc 64k -// that matches our read sizes -assert.deepStrictEqual(ret.stdout, msgOutBuf); + assert.ifError(ret.error); + assert.deepStrictEqual(ret.stdout, msgOutBuf); +} diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js index e10137624863a6..8260e15512d109 100644 --- a/test/parallel/test-child-process-spawnsync-validation-errors.js +++ b/test/parallel/test-child-process-spawnsync-validation-errors.js @@ -163,16 +163,17 @@ if (!common.isWindows) { { // Validate the maxBuffer option - const err = /^TypeError: "maxBuffer" must be an unsigned integer$/; + const err = /^TypeError: "maxBuffer" must be a positive number$/; pass('maxBuffer', undefined); pass('maxBuffer', null); pass('maxBuffer', 1); pass('maxBuffer', 0); - fail('maxBuffer', 3.14, err); + pass('maxBuffer', Infinity); + pass('maxBuffer', 3.14); fail('maxBuffer', -1, err); fail('maxBuffer', NaN, err); - fail('maxBuffer', Infinity, err); + fail('maxBuffer', -Infinity, err); fail('maxBuffer', true, err); fail('maxBuffer', false, err); fail('maxBuffer', __dirname, err);