Skip to content

Commit 4710349

Browse files
killaguMylesBorins
authored andcommitted
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: #18976Fixes: #18969 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent aaf1df5 commit 4710349

5 files changed

Lines changed: 82 additions & 14 deletions

‎lib/_stream_readable.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,8 @@ Readable.prototype.setEncoding = function(enc) {
324324
if(!StringDecoder)
325325
StringDecoder=require('string_decoder').StringDecoder;
326326
this._readableState.decoder=newStringDecoder(enc);
327-
this._readableState.encoding=enc;
327+
// if setEncoding(null), decoder.encoding equals utf8
328+
this._readableState.encoding=this._readableState.decoder.encoding;
328329
returnthis;
329330
};
330331

‎lib/child_process.js‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -226,15 +226,11 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
226226
});
227227

228228
varencoding;
229-
var_stdout;
230-
var_stderr;
229+
var_stdout=[];
230+
var_stderr=[];
231231
if(options.encoding!=='buffer'&&Buffer.isEncoding(options.encoding)){
232232
encoding=options.encoding;
233-
_stdout='';
234-
_stderr='';
235233
}else{
236-
_stdout=[];
237-
_stderr=[];
238234
encoding=null;
239235
}
240236
varstdoutLen=0;
@@ -261,11 +257,24 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
261257
// merge chunks
262258
varstdout;
263259
varstderr;
264-
if(encoding){
265-
stdout=_stdout;
266-
stderr=_stderr;
260+
if(encoding||
261+
(
262+
child.stdout&&
263+
child.stdout._readableState&&
264+
child.stdout._readableState.encoding
265+
)){
266+
stdout=_stdout.join('');
267267
}else{
268268
stdout=Buffer.concat(_stdout);
269+
}
270+
if(encoding||
271+
(
272+
child.stderr&&
273+
child.stderr._readableState&&
274+
child.stderr._readableState.encoding
275+
)){
276+
stderr=_stderr.join('');
277+
}else{
269278
stderr=Buffer.concat(_stderr);
270279
}
271280

@@ -329,13 +338,12 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
329338
child.stdout.setEncoding(encoding);
330339

331340
child.stdout.on('data',functiononChildStdout(chunk){
341+
varencoding=child.stdout._readableState.encoding;
332342
stdoutLen+=encoding ? Buffer.byteLength(chunk,encoding) : chunk.length;
333343

334344
if(stdoutLen>options.maxBuffer){
335345
ex=newERR_CHILD_PROCESS_STDIO_MAXBUFFER('stdout');
336346
kill();
337-
}elseif(encoding){
338-
_stdout+=chunk;
339347
}else{
340348
_stdout.push(chunk);
341349
}
@@ -347,13 +355,12 @@ exports.execFile = function execFile(file /* , args, options, callback */) {
347355
child.stderr.setEncoding(encoding);
348356

349357
child.stderr.on('data',functiononChildStderr(chunk){
358+
varencoding=child.stderr._readableState.encoding;
350359
stderrLen+=encoding ? Buffer.byteLength(chunk,encoding) : chunk.length;
351360

352361
if(stderrLen>options.maxBuffer){
353362
ex=newERR_CHILD_PROCESS_STDIO_MAXBUFFER('stderr');
354363
kill();
355-
}elseif(encoding){
356-
_stderr+=chunk;
357364
}else{
358365
_stderr.push(chunk);
359366
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,25 @@ const unicode = '中文测试'; // length = 4, byte length = 12
4141

4242
cp.exec(cmd,{maxBuffer: 10},checkFactory('stderr'));
4343
}
44+
45+
{
46+
constcmd=`"${process.execPath}" -e "console.log('${unicode}');"`;
47+
48+
constchild=cp.exec(
49+
cmd,
50+
{encoding: null,maxBuffer: 10},
51+
checkFactory('stdout'));
52+
53+
child.stdout.setEncoding('utf-8');
54+
}
55+
56+
{
57+
constcmd=`"${process.execPath}" -e "console.error('${unicode}');"`;
58+
59+
constchild=cp.exec(
60+
cmd,
61+
{encoding: null,maxBuffer: 10},
62+
checkFactory('stderr'));
63+
64+
child.stderr.setEncoding('utf-8');
65+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
constassert=require('assert');
4+
constcp=require('child_process');
5+
conststdoutData='foo';
6+
conststderrData='bar';
7+
constexpectedStdout=`${stdoutData}\n`;
8+
constexpectedStderr=`${stderrData}\n`;
9+
10+
if(process.argv[2]==='child'){
11+
// The following console calls are part of the test.
12+
console.log(stdoutData);
13+
console.error(stderrData);
14+
}else{
15+
constcmd=`"${process.execPath}" "${__filename}" child`;
16+
constchild=cp.exec(cmd,common.mustCall((err,stdout,stderr)=>{
17+
assert.ifError(err);
18+
assert.strictEqual(stdout,expectedStdout);
19+
assert.strictEqual(stderr,expectedStderr);
20+
}));
21+
child.stdout.setEncoding('utf-8');
22+
child.stderr.setEncoding('utf-8');
23+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
3+
require('../common');
4+
constassert=require('assert');
5+
const{ Readable }=require('stream');
6+
7+
8+
{
9+
constreadable=newReadable({encoding: 'hex'});
10+
assert.strictEqual(readable._readableState.encoding,'hex');
11+
12+
readable.setEncoding(null);
13+
14+
assert.strictEqual(readable._readableState.encoding,'utf8');
15+
}

0 commit comments

Comments
 (0)