child_process: fix exec set stdout.setEncoding#18976
child_process: fix exec set stdout.setEncoding#18976killagu wants to merge 3 commits intonodejs:masterfrom
Conversation
There was a problem hiding this comment.
What do you think about using process.{stdout,stderr}.write()?
|
The |
|
@hstarorg Thanks for the advice, you are right.I have a better solution. |
lib/_stream_readable.js
Outdated
There was a problem hiding this comment.
const readStream = new ReadableStream();
readStream.setEncoding(null);
// readStream._readableState.decoder.encoding equals 'utf8'
// readStream._readableState.encoding equals null;_readableState.encoding should be same as this._readableState.decoder.encoding
|
@nodejs/streams PTAL |
mcollina
left a comment
There was a problem hiding this comment.
Can you please add a test for Readable alone?
LGTM with that test included.
|
Ping @killagu |
|
@BridgeAR I'm sorry it's taken me so long to get back to you. I'll add the test ASAP. |
cp.exec decide to use `_stdout`(_stdout is string) or `Buffer.concat(_stdout)`(_stdout is buffer array) by options.encoding. but std(out|err) encoding can be changed. If encoding is changed to not null, `_stdout` will become string, and `Buffer.concat(_stdout)` will throw TypeError. This patch will fix it, use options.encoding and `std(out|err)._readableState.encoding`. Fixes: nodejs#18969
fix max-buffer fix _stream_readable.setEncoding
daf4cae to
72ebad2
Compare
|
@nodejs/child_process PTAL |
cp.exec decide to use `_stdout`(_stdout is string) or `Buffer.concat(_stdout)`(_stdout is buffer array) by options.encoding. but std(out|err) encoding can be changed. If encoding is changed to not null, `_stdout` will become string, and `Buffer.concat(_stdout)` will throw TypeError. This patch will fix it, use options.encoding and `std(out|err)._readableState.encoding`. PR-URL: nodejs#18976 Fixes: nodejs#18969 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
Landed in 7d81f5d 🎉 |
cp.exec decide to use `_stdout`(_stdout is string) or `Buffer.concat(_stdout)`(_stdout is buffer array) by options.encoding. but std(out|err) encoding can be changed. If encoding is changed to not null, `_stdout` will become string, and `Buffer.concat(_stdout)` will throw TypeError. This patch will fix it, use options.encoding and `std(out|err)._readableState.encoding`. PR-URL: #18976 Fixes: #18969 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
cp.exec decide to use
_stdout(_stdout is string) orBuffer.concat(_stdout)(_stdout is buffer array) by options.encoding.but std(out|err) encoding can be changed. If encoding is changed to
not null,
_stdoutwill become string, andBuffer.concat(_stdout)will throw TypeError. This patch will fix it, use
options.encoding and
std(out|err)._readableState.encoding.Fixes: #18969
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
child_process