From fd05b0b2899da47264b589caa3bac3a05ff21522 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 22 Jun 2016 16:21:21 -0700 Subject: [PATCH] Revert "child_process: measure buffer length in bytes" This reverts commit c9a5990a76ccb15872234948e23bdc12691c2e70. PR-URL: https://github.com/nodejs/node/pull/7391 Reviewed-By: Ben Noordhuis Reviewed-By: Jackson Tian --- lib/child_process.js | 71 ++++++++----------- .../test-child-process-max-buffer.js} | 3 +- ...t-child-process-exec-stdout-data-string.js | 0 .../test-child-process-maxBuffer-stderr.js | 15 ---- 4 files changed, 32 insertions(+), 57 deletions(-) rename test/{parallel/test-child-process-maxBuffer-stdout.js => known_issues/test-child-process-max-buffer.js} (75%) rename test/{known_issues => parallel}/test-child-process-exec-stdout-data-string.js (100%) delete mode 100644 test/parallel/test-child-process-maxBuffer-stderr.js diff --git a/lib/child_process.js b/lib/child_process.js index 7728b89bdb5fba..e43093ace1fa0d 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -157,13 +157,15 @@ exports.execFile = function(file /*, args, options, callback*/) { }); var encoding; - var stdoutState; - var stderrState; - var _stdout = []; - var _stderr = []; + var _stdout; + var _stderr; if (options.encoding !== 'buffer' && Buffer.isEncoding(options.encoding)) { encoding = options.encoding; + _stdout = ''; + _stderr = ''; } else { + _stdout = []; + _stderr = []; encoding = null; } var stdoutLen = 0; @@ -185,23 +187,16 @@ exports.execFile = function(file /*, args, options, callback*/) { if (!callback) return; - var stdout = Buffer.concat(_stdout, stdoutLen); - var stderr = Buffer.concat(_stderr, stderrLen); - - var stdoutEncoding = encoding; - var stderrEncoding = encoding; - - if (stdoutState && stdoutState.decoder) - stdoutEncoding = stdoutState.decoder.encoding; - - if (stderrState && stderrState.decoder) - stderrEncoding = stderrState.decoder.encoding; - - if (stdoutEncoding) - stdout = stdout.toString(stdoutEncoding); - - if (stderrEncoding) - stderr = stderr.toString(stderrEncoding); + // merge chunks + var stdout; + var stderr; + if (!encoding) { + stdout = Buffer.concat(_stdout); + stderr = Buffer.concat(_stderr); + } else { + stdout = _stdout; + stderr = _stderr; + } if (ex) { // Will be handled later @@ -261,45 +256,39 @@ exports.execFile = function(file /*, args, options, callback*/) { } if (child.stdout) { - stdoutState = child.stdout._readableState; + if (encoding) + child.stdout.setEncoding(encoding); child.stdout.addListener('data', function(chunk) { - // If `child.stdout.setEncoding()` happened in userland, convert string to - // Buffer. - if (stdoutState.decoder) { - chunk = Buffer.from(chunk, stdoutState.decoder.encoding); - } - - stdoutLen += chunk.byteLength; + stdoutLen += chunk.length; if (stdoutLen > options.maxBuffer) { ex = new Error('stdout maxBuffer exceeded'); - stdoutLen -= chunk.byteLength; kill(); } else { - _stdout.push(chunk); + if (!encoding) + _stdout.push(chunk); + else + _stdout += chunk; } }); } if (child.stderr) { - stderrState = child.stderr._readableState; + if (encoding) + child.stderr.setEncoding(encoding); child.stderr.addListener('data', function(chunk) { - // If `child.stderr.setEncoding()` happened in userland, convert string to - // Buffer. - if (stderrState.decoder) { - chunk = Buffer.from(chunk, stderrState.decoder.encoding); - } - - stderrLen += chunk.byteLength; + stderrLen += chunk.length; if (stderrLen > options.maxBuffer) { ex = new Error('stderr maxBuffer exceeded'); - stderrLen -= chunk.byteLength; kill(); } else { - _stderr.push(chunk); + if (!encoding) + _stderr.push(chunk); + else + _stderr += chunk; } }); } diff --git a/test/parallel/test-child-process-maxBuffer-stdout.js b/test/known_issues/test-child-process-max-buffer.js similarity index 75% rename from test/parallel/test-child-process-maxBuffer-stdout.js rename to test/known_issues/test-child-process-max-buffer.js index 122dc499f462bf..14a344c7062a5a 100644 --- a/test/parallel/test-child-process-maxBuffer-stdout.js +++ b/test/known_issues/test-child-process-max-buffer.js @@ -1,4 +1,5 @@ 'use strict'; +// Refs: https://github.com/nodejs/node/issues/1901 const common = require('../common'); const assert = require('assert'); const cp = require('child_process'); @@ -9,7 +10,7 @@ if (process.argv[2] === 'child') { } else { const cmd = `${process.execPath} ${__filename} child`; - cp.exec(cmd, {maxBuffer: 10}, common.mustCall((err, stdout, stderr) => { + cp.exec(cmd, { maxBuffer: 10 }, common.mustCall((err, stdout, stderr) => { assert.strictEqual(err.message, 'stdout maxBuffer exceeded'); })); } diff --git a/test/known_issues/test-child-process-exec-stdout-data-string.js b/test/parallel/test-child-process-exec-stdout-data-string.js similarity index 100% rename from test/known_issues/test-child-process-exec-stdout-data-string.js rename to test/parallel/test-child-process-exec-stdout-data-string.js diff --git a/test/parallel/test-child-process-maxBuffer-stderr.js b/test/parallel/test-child-process-maxBuffer-stderr.js deleted file mode 100644 index ecaea8b152a0ca..00000000000000 --- a/test/parallel/test-child-process-maxBuffer-stderr.js +++ /dev/null @@ -1,15 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const cp = require('child_process'); -const unicode = '中文测试'; // Length = 4, Byte length = 13 - -if (process.argv[2] === 'child') { - console.error(unicode); -} else { - const cmd = `${process.execPath} ${__filename} child`; - - cp.exec(cmd, {maxBuffer: 10}, common.mustCall((err, stdout, stderr) => { - assert.strictEqual(err.message, 'stderr maxBuffer exceeded'); - })); -}