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

child_process: fork does not accept null or undefined args array #10819

Closed
trendsetter37 opened this issue Jan 15, 2017 · 10 comments
Closed

child_process: fork does not accept null or undefined args array #10819

trendsetter37 opened this issue Jan 15, 2017 · 10 comments
Labels
child_process Issues and PRs related to the child_process subsystem.

Comments

@trendsetter37
Copy link
Contributor

  • Version: 8.0.0-pre

  • Platform: Linux jsully 4.8.0-040800-generic #201610022031 SMP Mon Oct 3 00:32:57 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

  • Subsystem: child_process

It appears as though fork assumes there are no options args when the arg array is null or undefined. This results in ignored parameters set in the options object.

More info here

I will update with further details once I dig into the code.

ref. #10793

@joyeecheung joyeecheung added the child_process Issues and PRs related to the child_process subsystem. label Jan 15, 2017
@Fishrock123
Copy link
Member

I think it would make more sense to throw a TypeError over seeing it as no-args? What does everyone else think?

@sam-github
Copy link
Contributor

The handling should be the same across the child_process APIs for when argv arrays are passed null or undefined. What do execFile/spawn[Sync]() do in this case?

@trendsetter37
Copy link
Contributor Author

trendsetter37 commented Jan 16, 2017

@sam-github spawn and execFile uses a normalizeSpawnArguments function to parse arguments.

var spawn = exports.spawn = function(/*file, args, options*/) {
  var opts = normalizeSpawnArguments.apply(null, arguments);
  var options = opts.options;
  var child = new ChildProcess();

  debug('spawn', opts.args, options);

  child.spawn({
    file: opts.file,
    args: opts.args,
    cwd: options.cwd,
    windowsVerbatimArguments: !!options.windowsVerbatimArguments,
    detached: !!options.detached,
    envPairs: opts.envPairs,
    stdio: options.stdio,
    uid: options.uid,
    gid: options.gid
  });

where as fork does something different...or it's own thing entirely rather.

@sam-github
Copy link
Contributor

How those functions are implemented internally is their own business, how they behave should be consistent. spawn and exec didn't used to share implementation, and their behaviour was bafflingly inconsistent, the code sharing helps ensure they behave identically, as do the tests.

@Trott
Copy link
Member

Trott commented Jul 16, 2017

@sam-github Should this remain open?

@sam-github
Copy link
Contributor

Yes, it sounds like there is an inconsistency, but no one has looked into it yet.

@gireeshpunathil
Copy link
Member

this is addressed through #10866 and is a close candidate? I may be wrong.

@laykun
Copy link

laykun commented May 29, 2018

I wasted a whole bunch of time figuring out why my child processes were crashing due to invalid debug options even when I was passing in a new inspect port to use. Turns on I was using null as my args argument assuming it would NOT ignore the third argument in the fork function. This is very odd behaviour and will cost more time. The function should process arguments on a case by case basis and not be dependent on prior arguments. Seeing as this behaviour is not mentioned in the documentation and using null / undefined as an argument should be perfectly acceptable then this behaviour needs to change.

@gireeshpunathil
Copy link
Member

#20882 is a work in progress to address the ambiguity. Feel free to review that PR and provide any inputs, thanks!

@juanarbol
Copy link
Member

This was fixed on #22416

Closing this issue, thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem.
Projects
None yet
Development

No branches or pull requests

8 participants