From 7d81f5d1435a208f34f0d3ca15034ff8eaf060ce Mon Sep 17 00:00:00 2001 From: killagu Date: Sat, 24 Feb 2018 22:00:32 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/18976 Fixes: https://github.com/nodejs/node/issues/18969 Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater --- lib/_stream_readable.js | 3 +- lib/child_process.js | 33 +++++++++++-------- .../test-child-process-exec-maxBuffer.js | 22 +++++++++++++ .../test-child-process-exec-std-encoding.js | 23 +++++++++++++ .../test-stream-readable-setEncoding-null.js | 15 +++++++++ 5 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-child-process-exec-std-encoding.js create mode 100644 test/parallel/test-stream-readable-setEncoding-null.js diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 8073e174cc5..3b614a7924f 100644 --- a/lib/_stream_readable.js +++ b/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; }; diff --git a/lib/child_process.js b/lib/child_process.js index 734f67d7a51..b2835b0a8f5 100644 --- a/lib/child_process.js +++ b/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); } diff --git a/test/parallel/test-child-process-exec-maxBuffer.js b/test/parallel/test-child-process-exec-maxBuffer.js index 4a65ccf5be7..94545e719ba 100644 --- a/test/parallel/test-child-process-exec-maxBuffer.js +++ b/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'); +} diff --git a/test/parallel/test-child-process-exec-std-encoding.js b/test/parallel/test-child-process-exec-std-encoding.js new file mode 100644 index 00000000000..11cb66cf42c --- /dev/null +++ b/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'); +} diff --git a/test/parallel/test-stream-readable-setEncoding-null.js b/test/parallel/test-stream-readable-setEncoding-null.js new file mode 100644 index 00000000000..b95b26bb795 --- /dev/null +++ b/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'); +}