Skip to content

Commit

Permalink
child_process: preserve argument type
Browse files Browse the repository at this point in the history
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 <info@bnoordhuis.nl>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Fixes: #7342
Refs: #1901
  • Loading branch information
Trott authored and Myles Borins committed Jul 14, 2016
1 parent fd05b0b commit 6a08535
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 46 deletions.
24 changes: 12 additions & 12 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,12 @@ exports.execFile = function(file /*, args, options, callback*/) {
// merge chunks
var stdout;
var stderr;
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) {
Expand Down Expand Up @@ -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 = new Error('stdout maxBuffer exceeded');
kill();
} else {
if (!encoding)
_stdout.push(chunk);
else
if (encoding)
_stdout += chunk;
else
_stdout.push(chunk);
}
});
}
Expand All @@ -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 = new Error('stderr maxBuffer exceeded');
kill();
} else {
if (!encoding)
_stderr.push(chunk);
else
if (encoding)
_stderr += chunk;
else
_stderr.push(chunk);
}
});
}
Expand Down
16 changes: 0 additions & 16 deletions test/known_issues/test-child-process-max-buffer.js

This file was deleted.

31 changes: 31 additions & 0 deletions test/parallel/test-child-process-exec-maxBuffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');

function checkFactory(streamName) {
return common.mustCall((err) => {
const message = `${streamName} maxBuffer exceeded`;
assert.strictEqual(err.message, message);
});
}

{
const cmd = 'echo "hello world"';

cp.exec(cmd, { maxBuffer: 5 }, checkFactory('stdout'));
}

const unicode = '中文测试'; // length = 4, byte length = 12

{
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;

cp.exec(cmd, {maxBuffer: 10}, checkFactory('stdout'));
}

{
const cmd = `"${process.execPath}" -e "console.('${unicode}');"`;

cp.exec(cmd, {maxBuffer: 10}, checkFactory('stderr'));
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@ const common = require('../common');
const assert = require('assert');
const exec = require('child_process').exec;

const expectedCalls = 2;

const cb = common.mustCall((data) => {
assert.strictEqual(typeof data, 'string');
}, expectedCalls);
var stdoutCalls = 0;
var stderrCalls = 0;

const command = 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(typeof data, 'string');
stderrCalls += 1;
});

exec('fhqwhgads').stderr.on('data', cb);
process.on('exit', () => {
assert(stdoutCalls > 0);
assert(stderrCalls > 0);
});
11 changes: 0 additions & 11 deletions test/parallel/test-exec-max-buffer.js

This file was deleted.

0 comments on commit 6a08535

Please sign in to comment.