child_process: preserve argument type

A previous fix for a `maxBuffer` bug resulted in a change to the argument type for the `data` event on `child.stdin` and `child.stdout` when using `child_process.exec()`. This fixes the `maxBuffer` bug in a way that does not have that side effect. PR-URL: #7391 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jackson Tian <[email protected]> Fixes: #7342 Refs: #1901

Author: Trott

lib/child_process.js

@@ -190,12 +190,12 @@ exports.execFile = function(file /*, args, options, callback*/){
 // merge chunks
 varstdout;
 varstderr;
-if(!encoding){
-stdout=Buffer.concat(_stdout);
-stderr=Buffer.concat(_stderr);
-}else{
+if(encoding){
 stdout=_stdout;
 stderr=_stderr;
+}else{
+stdout=Buffer.concat(_stdout);
+stderr=Buffer.concat(_stderr);
 }
 
 if(ex){
@@ -260,16 +260,16 @@ exports.execFile = function(file /*, args, options, callback*/){
 child.stdout.setEncoding(encoding);
 
 child.stdout.addListener('data',function(chunk){
-stdoutLen+=chunk.length;
+stdoutLen+=encoding ? Buffer.byteLength(chunk,encoding) : chunk.length;
 
 if(stdoutLen>options.maxBuffer){
 ex=newError('stdout maxBuffer exceeded');
 kill();
 }else{
-if(!encoding)
-_stdout.push(chunk);
-else
+if(encoding)
 _stdout+=chunk;
+else
+_stdout.push(chunk);
 }
 });
 }
@@ -279,16 +279,16 @@ exports.execFile = function(file /*, args, options, callback*/){
 child.stderr.setEncoding(encoding);
 
 child.stderr.addListener('data',function(chunk){
-stderrLen+=chunk.length;
+stderrLen+=encoding ? Buffer.byteLength(chunk,encoding) : chunk.length;
 
 if(stderrLen>options.maxBuffer){
 ex=newError('stderr maxBuffer exceeded');
 kill();
 }else{
-if(!encoding)
-_stderr.push(chunk);
-else
+if(encoding)
 _stderr+=chunk;
+else
+_stderr.push(chunk);
 }
 });
 }

test/known_issues/test-child-process-max-buffer.js

@@ -0,0 +1,31 @@
+'use strict';
+constcommon=require('../common');
+constassert=require('assert');
+constcp=require('child_process');
+
+functioncheckFactory(streamName){
+returncommon.mustCall((err)=>{
+constmessage=`${streamName} maxBuffer exceeded`;
+assert.strictEqual(err.message,message);
+});
+}
+
+{
+constcmd='echo "hello world"';
+
+cp.exec(cmd,{maxBuffer: 5},checkFactory('stdout'));
+}
+
+constunicode='中文测试';// length = 4, byte length = 12
+
+{
+constcmd=`"${process.execPath}" -e "console.log('${unicode}');"`;
+
+cp.exec(cmd,{maxBuffer: 10},checkFactory('stdout'));
+}
+
+{
+constcmd=`"${process.execPath}" -e "console.('${unicode}');"`;
+
+cp.exec(cmd,{maxBuffer: 10},checkFactory('stderr'));
+}

test/parallel/test-child-process-exec-maxBuffer.js

@@ -4,13 +4,20 @@ const common = require('../common');
 constassert=require('assert');
 constexec=require('child_process').exec;
 
-constexpectedCalls=2;
-
-constcb=common.mustCall((data)=>{
-assert.strictEqual(typeofdata,'string');
-},expectedCalls);
+varstdoutCalls=0;
+varstderrCalls=0;
 
 constcommand=common.isWindows ? 'dir' : 'ls';
-exec(command).stdout.on('data',cb);
+exec(command).stdout.on('data',(data)=>{
+stdoutCalls+=1;
+});
+
+exec('fhqwhgads').stderr.on('data',(data)=>{
+assert.strictEqual(typeofdata,'string');
+stderrCalls+=1;
+});
 
-exec('fhqwhgads').stderr.on('data',cb);
+process.on('exit',()=>{
+assert(stdoutCalls>0);
+assert(stderrCalls>0);
+});