child_process: allow Infinity as maxBuffer value

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;
+}

src/spawn_sync.cc

@@ -765,8 +765,8 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
 
   Local<Value> 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<Value> js_kill_signal = js_options->Get(env()->kill_signal_string());

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_;
 

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"';
 

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);
+}

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);