FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

child_process: preserve argument type · nodejs/node@6a08535 · GitHub

/ node Public

Commit 6a08535

Browse files
Trott
authored and
Myles Borins
committed
child_process: preserve argument type
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
1 parent fd05b0b commit 6a08535

5 files changed

Lines changed: 57 additions & 46 deletions

File tree

‎lib/child_process.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,12 @@ exports.execFile = function(file /*, args, options, callback*/) {
190190
// merge chunks
191191
var stdout;
192192
var stderr;
193-
if (!encoding) {
194-
stdout = Buffer.concat(_stdout);
195-
stderr = Buffer.concat(_stderr);
196-
} else {
193+
if (encoding) {
197194
stdout = _stdout;
198195
stderr = _stderr;
196+
} else {
197+
stdout = Buffer.concat(_stdout);
198+
stderr = Buffer.concat(_stderr);
199199
}
200200

201201
if (ex) {
@@ -260,16 +260,16 @@ exports.execFile = function(file /*, args, options, callback*/) {
260260
child.stdout.setEncoding(encoding);
261261

262262
child.stdout.addListener('data', function(chunk) {
263-
stdoutLen += chunk.length;
263+
stdoutLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
264264

265265
if (stdoutLen > options.maxBuffer) {
266266
ex = new Error('stdout maxBuffer exceeded');
267267
kill();
268268
} else {
269-
if (!encoding)
270-
_stdout.push(chunk);
271-
else
269+
if (encoding)
272270
_stdout += chunk;
271+
else
272+
_stdout.push(chunk);
273273
}
274274
});
275275
}
@@ -279,16 +279,16 @@ exports.execFile = function(file /*, args, options, callback*/) {
279279
child.stderr.setEncoding(encoding);
280280

281281
child.stderr.addListener('data', function(chunk) {
282-
stderrLen += chunk.length;
282+
stderrLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
283283

284284
if (stderrLen > options.maxBuffer) {
285285
ex = new Error('stderr maxBuffer exceeded');
286286
kill();
287287
} else {
288-
if (!encoding)
289-
_stderr.push(chunk);
290-
else
288+
if (encoding)
291289
_stderr += chunk;
290+
else
291+
_stderr.push(chunk);
292292
}
293293
});
294294
}

‎test/known_issues/test-child-process-max-buffer.js‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const cp = require('child_process');
5+
6+
function checkFactory(streamName) {
7+
return common.mustCall((err) => {
8+
const message = `${streamName} maxBuffer exceeded`;
9+
assert.strictEqual(err.message, message);
10+
});
11+
}
12+
13+
{
14+
const cmd = 'echo "hello world"';
15+
16+
cp.exec(cmd, { maxBuffer: 5 }, checkFactory('stdout'));
17+
}
18+
19+
const unicode = '中文测试'; // length = 4, byte length = 12
20+
21+
{
22+
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;
23+
24+
cp.exec(cmd, {maxBuffer: 10}, checkFactory('stdout'));
25+
}
26+
27+
{
28+
const cmd = `"${process.execPath}" -e "console.('${unicode}');"`;
29+
30+
cp.exec(cmd, {maxBuffer: 10}, checkFactory('stderr'));
31+
}

test/parallel/test-child-process-exec-stdout-data-string.js renamed to test/parallel/test-child-process-exec-stdout-stderr-data-string.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,20 @@ const common = require('../common');
44
const assert = require('assert');
55
const exec = require('child_process').exec;
66

7-
const expectedCalls = 2;
8-
9-
const cb = common.mustCall((data) => {
10-
assert.strictEqual(typeof data, 'string');
11-
}, expectedCalls);
7+
var stdoutCalls = 0;
8+
var stderrCalls = 0;
129

1310
const command = common.isWindows ? 'dir' : 'ls';
14-
exec(command).stdout.on('data', cb);
11+
exec(command).stdout.on('data', (data) => {
12+
stdoutCalls += 1;
13+
});
14+
15+
exec('fhqwhgads').stderr.on('data', (data) => {
16+
assert.strictEqual(typeof data, 'string');
17+
stderrCalls += 1;
18+
});
1519

16-
exec('fhqwhgads').stderr.on('data', cb);
20+
process.on('exit', () => {
21+
assert(stdoutCalls > 0);
22+
assert(stderrCalls > 0);
23+
});

‎test/parallel/test-exec-max-buffer.js‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL