child_process: fix exec set stdout.setEncoding

cp.exec decide to use `_stdout`(_stdout is string) or
`Buffer.concat(_stdout)`(_stdout is buffer array) by options.encoding.
but std(out|err) encoding can be changed. If encoding is changed to
not null, `_stdout` will become string, and `Buffer.concat(_stdout)`
will throw TypeError. This patch will fix it, use
options.encoding and `std(out|err)._readableState.encoding`.

PR-URL: #18976
Fixes: #18969
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>

Author: killagu

lib/_stream_readable.js

@@ -324,7 +324,8 @@ Readable.prototype.setEncoding = function(enc) {
   if (!StringDecoder)
     StringDecoder = require('string_decoder').StringDecoder;
   this._readableState.decoder = new StringDecoder(enc);
-  this._readableState.encoding = enc;
+  // if setEncoding(null), decoder.encoding equals utf8
+  this._readableState.encoding = this._readableState.decoder.encoding;
   return this;
 };
 

lib/child_process.js

@@ -226,15 +226,11 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
   });
 
   var encoding;
-  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;
@@ -261,11 +257,24 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
     // merge chunks
     var stdout;
     var stderr;
-    if (encoding) {
-      stdout = _stdout;
-      stderr = _stderr;
+    if (encoding ||
+      (
+        child.stdout &&
+        child.stdout._readableState &&
+        child.stdout._readableState.encoding
+      )) {
+      stdout = _stdout.join('');
     } else {
       stdout = Buffer.concat(_stdout);
+    }
+    if (encoding ||
+      (
+        child.stderr &&
+        child.stderr._readableState &&
+        child.stderr._readableState.encoding
+      )) {
+      stderr = _stderr.join('');
+    } else {
       stderr = Buffer.concat(_stderr);
     }
 
@@ -329,13 +338,12 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
       child.stdout.setEncoding(encoding);
 
     child.stdout.on('data', function onChildStdout(chunk) {
+      var encoding = child.stdout._readableState.encoding;
       stdoutLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
 
       if (stdoutLen > options.maxBuffer) {
         ex = new ERR_CHILD_PROCESS_STDIO_MAXBUFFER('stdout');
         kill();
-      } else if (encoding) {
-        _stdout += chunk;
       } else {
         _stdout.push(chunk);
       }
@@ -347,13 +355,12 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
       child.stderr.setEncoding(encoding);
 
     child.stderr.on('data', function onChildStderr(chunk) {
+      var encoding = child.stderr._readableState.encoding;
       stderrLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
 
       if (stderrLen > options.maxBuffer) {
         ex = new ERR_CHILD_PROCESS_STDIO_MAXBUFFER('stderr');
         kill();
-      } else if (encoding) {
-        _stderr += chunk;
       } else {
         _stderr.push(chunk);
       }

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

@@ -41,3 +41,25 @@ const unicode = '中文测试'; // length = 4, byte length = 12
 
   cp.exec(cmd, { maxBuffer: 10 }, checkFactory('stderr'));
 }
+
+{
+  const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;
+
+  const child = cp.exec(
+    cmd,
+    { encoding: null, maxBuffer: 10 },
+    checkFactory('stdout'));
+
+  child.stdout.setEncoding('utf-8');
+}
+
+{
+  const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;
+
+  const child = cp.exec(
+    cmd,
+    { encoding: null, maxBuffer: 10 },
+    checkFactory('stderr'));
+
+  child.stderr.setEncoding('utf-8');
+}

test/parallel/test-child-process-exec-std-encoding.js

@@ -0,0 +1,23 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+const stdoutData = 'foo';
+const stderrData = 'bar';
+const expectedStdout = `${stdoutData}\n`;
+const expectedStderr = `${stderrData}\n`;
+
+if (process.argv[2] === 'child') {
+  // The following console calls are part of the test.
+  console.log(stdoutData);
+  console.error(stderrData);
+} else {
+  const cmd = `"${process.execPath}" "${__filename}" child`;
+  const child = cp.exec(cmd, common.mustCall((err, stdout, stderr) => {
+    assert.ifError(err);
+    assert.strictEqual(stdout, expectedStdout);
+    assert.strictEqual(stderr, expectedStderr);
+  }));
+  child.stdout.setEncoding('utf-8');
+  child.stderr.setEncoding('utf-8');
+}

test/parallel/test-stream-readable-setEncoding-null.js

@@ -0,0 +1,15 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { Readable } = require('stream');
+
+
+{
+  const readable = new Readable({ encoding: 'hex' });
+  assert.strictEqual(readable._readableState.encoding, 'hex');
+
+  readable.setEncoding(null);
+
+  assert.strictEqual(readable._readableState.encoding, 'utf8');
+}