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.

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

maxBuffer of Infinity aborts child_process.execSync #10768

Closed
sam-github opened this issue Jan 12, 2017 · 9 comments
Closed

maxBuffer of Infinity aborts child_process.execSync #10768

sam-github opened this issue Jan 12, 2017 · 9 comments

Comments

@sam-github
Copy link
Contributor

w/core % nvm install 4; node --version; node -e "require('child_process').execSync('date', {maxBuffer: Infinity})"
v4.7.2 is already installed.
Now using node v4.7.2 (npm v2.15.11)
v4.7.2
node: ../deps/uv/src/unix/core.c:168: uv_close: Assertion `0' failed.
zsh: abort (core dumped)  node -e 
w/core % nvm install 5; node --version; node -e "require('child_process').execSync('date', {maxBuffer: Infinity})"                                                    
v5.12.0 is already installed.
Now using node v5.12.0 (npm v3.8.6)
v5.12.0
node: ../deps/uv/src/unix/core.c:165: uv_close: Assertion `0' failed.
zsh: abort (core dumped)  node -e 
w/core % nvm install 6; node --version; node -e "require('child_process').execSync('date', {maxBuffer: Infinity})"                                                    
v6.9.4 is already installed.
Now using node v6.9.4 (npm v3.10.10)
v6.9.4
node: ../deps/uv/src/unix/core.c:168: uv_close: Assertion `0' failed.
zsh: abort (core dumped)  node -e 
w/core % nvm install 7; node --version; node -e "require('child_process').execSync('date', {maxBuffer: Infinity})"
v7.4.0 is already installed.
Now using node v7.4.0 (npm v4.0.5)
v7.4.0
node: ../deps/uv/src/unix/core.c:166: uv_close: Assertion `0' failed.
zsh: abort (core dumped)  node -e 
w/core % ./node/node --version; ./node/node -e "require('child_process').execSync('date', {maxBuffer: Infinity})" 
v8.0.0-pre
child_process.js:477
    throw new TypeError('"maxBuffer" must be an unsigned integer');
    ^

TypeError: "maxBuffer" must be an unsigned integer

8.x-pre behaves well.

@evanlucas
Copy link
Contributor

With #10769 being semver-major, should we mark this as wont-fix?

@sam-github
Copy link
Contributor Author

#10769 is probably semver-major because it added argument validation. That was a nice cleanup, but not necessary to fix this bug, it should be possible to re-express #10769 as a semver-minor and backport it.

@nodejs/lts what do you think of me backporting #10769 to fix this? Worth it?

@gibfahn
Copy link
Member

gibfahn commented Apr 12, 2017

@nodejs/lts what do you think of me backporting #10769 to fix this? Worth it?

@sam-github maybe add it to nodejs/Release#188?

@mgrant0
Copy link

mgrant0 commented May 2, 2017

Just a reminder to please also fix this in the documentation. I ran across this, found this bug, discovered that I could do var child = exec(cmd, {maxBuffer:Infinity}) and it works, but isn't documented. Thanks.

@Trott
Copy link
Member

Trott commented Aug 10, 2017

Removing v7.x label as that release line is no longer supported.

@Trott Trott removed the v7.x label Aug 10, 2017
@gireeshpunathil
Copy link
Member

I removed v4.x as it is not longer supported and have no intention to fix in that.
#node --version; node -e "require('child_process').execSync('date', {maxBuffer: Infinity})"
v10.0.0

So I guess once this is back ported to v6.x and doc updated, we are good to close this.

/cc @nodejs/release

@gireeshpunathil
Copy link
Member

@apapirovski - a one liner rationale for closing please?

@targos
Copy link
Member

targos commented Aug 19, 2018

Looks like this was closed by mistake

@targos targos reopened this Aug 19, 2018
@apapirovski
Copy link
Member

I closed this because realistically no one will fix it after being open for a year and a half and not getting a fix in place. There are less than 6 months of maintenance support for v6.x. If someone actually wants to handle fixing / backporting then that's a good start rather than just going back and forth on opening and reopening this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants