Revert "child_process: measure buffer length in bytes"

This reverts commit c9a5990. PR-URL: #7391 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
nodejs · Jul 5, 2016 · 4a0fb6f · 4a0fb6f
1 parent 7a7b8f7
commit 4a0fb6f
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 57 deletions.
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -146,13 +146,15 @@ exports.execFile = function(file /*, args, options, callback*/) {
   });
 
   var encoding;
-  var stdoutState;
-  var stderrState;
-  var _stdout = [];
-  var _stderr = [];
+  var _stdout;
+  var _stderr;
   if (options.encoding !== 'buffer' && Buffer.isEncoding(options.encoding)) {
     encoding = options.encoding;
+    _stdout = '';
+    _stderr = '';
   } else {
+    _stdout = [];
+    _stderr = [];
     encoding = null;
   }
   var stdoutLen = 0;
@@ -174,23 +176,16 @@ exports.execFile = function(file /*, args, options, callback*/) {
 
     if (!callback) return;
 
-    var stdout = Buffer.concat(_stdout, stdoutLen);
-    var stderr = Buffer.concat(_stderr, stderrLen);
-
-    var stdoutEncoding = encoding;
-    var stderrEncoding = encoding;
-
-    if (stdoutState && stdoutState.decoder)
-      stdoutEncoding = stdoutState.decoder.encoding;
-
-    if (stderrState && stderrState.decoder)
-      stderrEncoding = stderrState.decoder.encoding;
-
-    if (stdoutEncoding)
-      stdout = stdout.toString(stdoutEncoding);
-
-    if (stderrEncoding)
-      stderr = stderr.toString(stderrEncoding);
+    // merge chunks
+    var stdout;
+    var stderr;
+    if (!encoding) {
+      stdout = Buffer.concat(_stdout);
+      stderr = Buffer.concat(_stderr);
+    } else {
+      stdout = _stdout;
+      stderr = _stderr;
+    }
 
     if (ex) {
       // Will be handled later
@@ -250,45 +245,39 @@ exports.execFile = function(file /*, args, options, callback*/) {
   }
 
   if (child.stdout) {
-    stdoutState = child.stdout._readableState;
+    if (encoding)
+      child.stdout.setEncoding(encoding);
 
     child.stdout.addListener('data', function(chunk) {
-      // If `child.stdout.setEncoding()` happened in userland, convert string to
-      // Buffer.
-      if (stdoutState.decoder) {
-        chunk = Buffer.from(chunk, stdoutState.decoder.encoding);
-      }
-
-      stdoutLen += chunk.byteLength;
+      stdoutLen += chunk.length;
 
       if (stdoutLen > options.maxBuffer) {
         ex = new Error('stdout maxBuffer exceeded');
-        stdoutLen -= chunk.byteLength;
         kill();
       } else {
-        _stdout.push(chunk);
+        if (!encoding)
+          _stdout.push(chunk);
+        else
+          _stdout += chunk;
       }
     });
   }
 
   if (child.stderr) {
-    stderrState = child.stderr._readableState;
+    if (encoding)
+      child.stderr.setEncoding(encoding);
 
     child.stderr.addListener('data', function(chunk) {
-      // If `child.stderr.setEncoding()` happened in userland, convert string to
-      // Buffer.
-      if (stderrState.decoder) {
-        chunk = Buffer.from(chunk, stderrState.decoder.encoding);
-      }
-
-      stderrLen += chunk.byteLength;
+      stderrLen += chunk.length;
 
       if (stderrLen > options.maxBuffer) {
         ex = new Error('stderr maxBuffer exceeded');
-        stderrLen -= chunk.byteLength;
         kill();
       } else {
-        _stderr.push(chunk);
+        if (!encoding)
+          _stderr.push(chunk);
+        else
+          _stderr += chunk;
       }
     });
   }

diff --git a/...el/test-child-process-maxBuffer-stdout.js → ...n_issues/test-child-process-max-buffer.js b/...el/test-child-process-maxBuffer-stdout.js → ...n_issues/test-child-process-max-buffer.js
@@ -1,4 +1,5 @@
 'use strict';
+// Refs: https://github.com/nodejs/node/issues/1901
 const common = require('../common');
 const assert = require('assert');
 const cp = require('child_process');
@@ -9,7 +10,7 @@ if (process.argv[2] === 'child') {
 } else {
   const cmd = `${process.execPath} ${__filename} child`;
 
-  cp.exec(cmd, {maxBuffer: 10}, common.mustCall((err, stdout, stderr) => {
+  cp.exec(cmd, { maxBuffer: 10 }, common.mustCall((err, stdout, stderr) => {
     assert.strictEqual(err.message, 'stdout maxBuffer exceeded');
   }));
 }
diff --git a/...-child-process-exec-stdout-data-string.js → ...-child-process-exec-stdout-data-string.js b/...-child-process-exec-stdout-data-string.js → ...-child-process-exec-stdout-data-string.js
@@ -14,3 +14,4 @@ const command = common.isWindows ? 'dir' : 'ls';
 exec(command).stdout.on('data', cb);
 
 exec('fhqwhgads').stderr.on('data', cb);
+
diff --git a/test/parallel/test-child-process-maxBuffer-stderr.js b/test/parallel/test-child-process-maxBuffer-stderr.js
Original file line number	Diff line number	Diff line change
Expand Up		@@ -14,3 +14,4 @@ const command = common.isWindows ? 'dir' : 'ls';
		exec(command).stdout.on('data', cb);

		exec('fhqwhgads').stderr.on('data', cb);