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.

Fixes: nodejs#7342
Refs: nodejs#1901
  • Loading branch information
Trott committed Jun 27, 2016
1 parent 174f7c5 commit 7059a85
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 35 deletions.
4 changes: 2 additions & 2 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ 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');
Expand All @@ -268,7 +268,7 @@ 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');
Expand Down
16 changes: 0 additions & 16 deletions test/known_issues/test-child-process-max-buffer.js

This file was deleted.

27 changes: 27 additions & 0 deletions test/parallel/test-child-process-exec-maxBuffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
// Refs: https://github.com/nodejs/node/issues/1901
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');
const unicode = '中文测试'; // Length = 4, Byte length = 13

if (process.argv[2] === 'stdout') {
console.log(unicode);
return;
}

if (process.argv[2] === 'stderr') {
console.error(unicode);
return;
}

const cmd = `${process.execPath} ${__filename}`;
const outputs = ['stdout', 'stderr'];

outputs.forEach((stream) => {
const cb = common.mustCall((err, stdout, stderr) => {
assert.strictEqual(err.message, `${stream} maxBuffer exceeded`);
});

cp.exec(`${cmd} ${stream}`, {maxBuffer: 10}, cb);
});
17 changes: 0 additions & 17 deletions test/parallel/test-child-process-exec-stdout-data-string.js

This file was deleted.

22 changes: 22 additions & 0 deletions test/parallel/test-child-process-exec-stdout-stderr-data-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
// Refs: https://github.com/nodejs/node/issues/7342
const common = require('../common');
const assert = require('assert');
const exec = require('child_process').exec;

var stdoutIsString = false;
var stderrIsString = false;

const command = common.isWindows ? 'dir' : 'ls';
exec(command).stdout.on('data', (data) => {
stdoutIsString = (typeof data === 'string');
});

exec('fhqwhgads').stderr.on('data', (data) => {
stderrIsString = (typeof data === 'string');
});

process.on('exit', () => {
assert(stdoutIsString);
assert(stderrIsString);
});

0 comments on commit 7059a85

Please sign in to comment.