child_process: Allow an unlimited maxBuffer size in child_process

[jwdeitch]

Allow unlimited buffer for child_process'

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

child_process

[mscdex]

This isn't necessary, just pass Infinity as your maxBuffer value.

[cjihrig]

Yes, what @mscdex said. Also, this patch doesn't address stderr.

[addaleax]

Fwiw, passing Infinity seems to give an TypeError: "maxBuffer" must be an unsigned integer error?

[cjihrig]

That should only happen for spawnSync(). execFile() should handle it fine.

[addaleax]

execFile() should handle it fine.

I tested this as:

>child_process.execSync('cat /dev/urandom',{maxBuffer:Infinity})
TypeError: "maxBuffer"mustbeanunsignedinteger

[cjihrig]

Yea, execSync() uses spawnSync() under the hood. execFile() should work though. I'm looking into making spawnSync() support Infinity right now.

[sam-github]

I think allowing Infinity would be better than 0 to mean unlimited.

That Infinity is valid isn't documented anywhere, and apparently, its support is random: yes for execFile, no for spawnSync, and for execFileSync, who knows? Is that accidental, or intentional?

And

> child_process.execSync('date', {maxBuffer: Infinity}, ()=>{})
node: ../deps/uv/src/unix/core.c:166: uv_close: Assertion `0' failed.
zsh: abort (core dumped) node
core/node (master $% u=) % node --version v7.2.0

Hm. EDIT: #10768

execFileSync also doesn't document a default for maxBuffer, its not clear if its defaulting to no limit, or to something else.

[mscdex]

I don't know why the sync counterparts perform different argument checking. I'd personally prefer simpler validation, that the value is just a number (typeof maxBuffer === 'number') rather than strictly Number.isInteger(maxBuffer).

[sam-github]

Btw, rebuilding to see if the abort is reproduceable.

[cjihrig]

The sync calls don't currently support it because it is CHECK()'ed as a uint in C++.

[cjihrig]

#10769