child_process: add type checking for "args" param #8277
Conversation
This commit ensures that the "args" parameter is either: 1. Undefined or null (defaulting to []). 2. The callback parameter (defaulting to []). 3. An array. All other values will throw a TypeError. Fixes nodejs#8249.
| @@ -610,6 +610,10 @@ exports.execFile = function(file /* args, options, callback */) { | |||
| if (Array.isArray(arguments[1])) { | |||
| args = arguments[1]; | |||
| options = util._extend(options, arguments[2]); | |||
| } else if (arguments[1] && callback !== arguments[1]) { | |||
There was a problem hiding this comment.
I believe this will still allow the same problematic behavior if the user passes in the empty string.
There was a problem hiding this comment.
It'll default the args to [] at that point; just tried this with child_process.spawn('echo', '').stdout.pipe(process.stdout); 1 and it doesn't seem to hang.
There was a problem hiding this comment.
I noticed in my testing that not all commands suffered from this problem. For example, ls and echo are fine. I was able to reproduce the original issue after pulling your changes locally and updating the command in your test file to read:
var c = execFile(process.execPath, '');
|
@chrisdickinson ... given that this was opened against v0.10, any reason to keep this open? |
|
@chrisdickinson .. going to close this here. given the current status, there's really no reason to land this in v0.10. |
This commit ensures that the "args" parameter is either:
All other values will throw a TypeError.
Fixes #8249.