Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2aa06b9

Browse filesBrowse files
TrottFishrock123
authored andcommitted
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 4a0fb6f commit 2aa06b9
Copy full SHA for 2aa06b9

File tree

Expand file treeCollapse file tree

5 files changed

+56
-46
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+56
-46
lines changed
Open diff view settings
Collapse file

‎lib/child_process.js‎

Copy file name to clipboardExpand all lines: lib/child_process.js
+12-12Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,12 @@ exports.execFile = function(file /*, args, options, callback*/) {
179179
// merge chunks
180180
var stdout;
181181
var stderr;
182-
if (!encoding) {
183-
stdout = Buffer.concat(_stdout);
184-
stderr = Buffer.concat(_stderr);
185-
} else {
182+
if (encoding) {
186183
stdout = _stdout;
187184
stderr = _stderr;
185+
} else {
186+
stdout = Buffer.concat(_stdout);
187+
stderr = Buffer.concat(_stderr);
188188
}
189189

190190
if (ex) {
@@ -249,16 +249,16 @@ exports.execFile = function(file /*, args, options, callback*/) {
249249
child.stdout.setEncoding(encoding);
250250

251251
child.stdout.addListener('data', function(chunk) {
252-
stdoutLen += chunk.length;
252+
stdoutLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
253253

254254
if (stdoutLen > options.maxBuffer) {
255255
ex = new Error('stdout maxBuffer exceeded');
256256
kill();
257257
} else {
258-
if (!encoding)
259-
_stdout.push(chunk);
260-
else
258+
if (encoding)
261259
_stdout += chunk;
260+
else
261+
_stdout.push(chunk);
262262
}
263263
});
264264
}
@@ -268,16 +268,16 @@ exports.execFile = function(file /*, args, options, callback*/) {
268268
child.stderr.setEncoding(encoding);
269269

270270
child.stderr.addListener('data', function(chunk) {
271-
stderrLen += chunk.length;
271+
stderrLen += encoding ? Buffer.byteLength(chunk, encoding) : chunk.length;
272272

273273
if (stderrLen > options.maxBuffer) {
274274
ex = new Error('stderr maxBuffer exceeded');
275275
kill();
276276
} else {
277-
if (!encoding)
278-
_stderr.push(chunk);
279-
else
277+
if (encoding)
280278
_stderr += chunk;
279+
else
280+
_stderr.push(chunk);
281281
}
282282
});
283283
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/known_issues/test-child-process-max-buffer.js
-16Lines changed: 0 additions & 16 deletions
This file was deleted.
Collapse file
+31Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-stdout-stderr-data-string.js
+13-7Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +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+
});
1514

16-
exec('fhqwhgads').stderr.on('data', cb);
15+
exec('fhqwhgads').stderr.on('data', (data) => {
16+
assert.strictEqual(typeof data, 'string');
17+
stderrCalls += 1;
18+
});
1719

20+
process.on('exit', () => {
21+
assert(stdoutCalls > 0);
22+
assert(stderrCalls > 0);
23+
});
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-exec-max-buffer.js
-11Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.