Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

the maxBuffer isn't complete accurate in child_process #1901

Closed
JacksonTian opened this issue Jun 5, 2015 · 3 comments
Closed

the maxBuffer isn't complete accurate in child_process #1901

JacksonTian opened this issue Jun 5, 2015 · 3 comments
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. repro-exists Issues with reproductions.

Comments

@JacksonTian
Copy link
Contributor

The maxBuffer is not complete accurate when data include unicode and encoding is set.
@brendanashworth brendanashworth added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Jun 5, 2015
@cjihrig cjihrig mentioned this issue Jun 5, 2015
Closed
@Trott
Copy link
Member

Trott commented Mar 10, 2016

#1902 would (if landed) be a complete fix for this, right?

@cjihrig cjihrig mentioned this issue Mar 11, 2016
Merged
4 tasks
@JacksonTian
Copy link
Contributor Author

@Trott the #1902 depends on bfb2cd0 , but it was reverted because it not correct in all cases.

cjihrig added a commit to cjihrig/node that referenced this issue Mar 12, 2016
@cjihrig
test: add batch of known issue tests
10bc673 
This commit adds tests for several known issues.

Refs: nodejs#1901
Refs: nodejs#728
Refs: nodejs#4778
Refs: nodejs#947
Refs: nodejs#2734
PR-URL: nodejs#5653
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@cjihrig cjihrig added the repro-exists Issues with reproductions. label Mar 12, 2016
evanlucas pushed a commit that referenced this issue Mar 14, 2016
@cjihrig @evanlucas
test: add batch of known issue tests
c783dd4 
This commit adds tests for several known issues.

Refs: #1901
Refs: #728
Refs: #4778
Refs: #947
Refs: #2734
PR-URL: #5653
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
rvagg pushed a commit that referenced this issue Mar 16, 2016
@cjihrig @rvagg
test: add batch of known issue tests
9a44c8c 
This commit adds tests for several known issues.

Refs: #1901
Refs: #728
Refs: #4778
Refs: #947
Refs: #2734
PR-URL: #5653
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
@cjihrig
test: add batch of known issue tests
cc2690d 
This commit adds tests for several known issues.

Refs: #1901
Refs: #728
Refs: #4778
Refs: #947
Refs: #2734
PR-URL: #5653
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
@cjihrig
test: add batch of known issue tests
78effc3 
This commit adds tests for several known issues.

Refs: #1901
Refs: #728
Refs: #4778
Refs: #947
Refs: #2734
PR-URL: #5653
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@jasnell jasnell mentioned this issue Apr 3, 2016
Closed
4 tasks
jasnell added a commit to jasnell/node that referenced this issue Apr 10, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
b968cc3 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: nodejs#1901
jasnell added a commit that referenced this issue Apr 10, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
ad2df3a 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: #1901
PR-URL: #6030
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 19, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
660de1d 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: #1901
PR-URL: #6030
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
79d612f 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: #1901
PR-URL: #6030
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
daaad47 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: #1901
PR-URL: #6030
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
jasnell added a commit that referenced this issue Apr 26, 2016
@jasnell
doc: clarification for maxBuffer and Unicode output
aa1220d 
Clarify caveats on `maxBuffer` with regards to Unicode output.

Refs: #1901
PR-URL: #6030
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue May 30, 2016
@Trott @Fishrock123
child_process: measure buffer length in bytes
8757631 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: nodejs#6764
Fixes: nodejs#1901
Reviewed-By: Brian White <[email protected]>
rvagg pushed a commit that referenced this issue Jun 2, 2016
@Trott @rvagg
child_process: measure buffer length in bytes
0503681 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
Trott added a commit to Trott/io.js that referenced this issue Jun 23, 2016
@Trott
child_process: measure buffer length in bytes
0e3c527 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

This necessarily changes default behavior of `stdout` and `stderr`
callbacks such that they receive buffers rather than strings. The
alternative would be a performance hit on the defaults.

Refs: nodejs#6764
Refs: nodejs#1901
@Trott Trott mentioned this issue Jun 23, 2016
Closed
3 tasks
Trott added a commit to Trott/io.js that referenced this issue Jun 27, 2016
@Trott
child_process: preserve argument type
7059a85 
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
Trott added a commit that referenced this issue Jun 30, 2016
@Trott
child_process: preserve argument type
0268fd0 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
Fishrock123 pushed a commit that referenced this issue Jul 5, 2016
@Trott @Fishrock123
child_process: preserve argument type
2aa06b9 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
MylesBorins pushed a commit that referenced this issue Jul 11, 2016
@Trott
child_process: measure buffer length in bytes
625d56a 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott @MylesBorins
child_process: measure buffer length in bytes
ad702bc 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott
child_process: preserve argument type
4e6cbde 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott
child_process: measure buffer length in bytes
b8f1a55 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott
child_process: preserve argument type
97b50d3 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
child_process: measure buffer length in bytes
3ed7683 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
child_process: preserve argument type
0923c00 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
child_process: measure buffer length in bytes
8eb18e4 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: #6764
Fixes: #1901
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
child_process: preserve argument type
6a08535 
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 <[email protected]>
Reviewed-By: Jackson Tian <[email protected]>
Fixes: #7342
Refs: #1901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. repro-exists Issues with reproductions.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

child_process: measure buffer length in bytes Trott/io.js
4 participants
@JacksonTian @Trott @cjihrig @brendanashworth