Child process' execFile and execFileSync handle arguments different when shell is not falsy

[ericcornelissen]

Version

v18.3.0

Platform

Linux *** 5.13.0-44-generic #49~20.04.1-Ubuntu SMP *** x86_64 x86_64 x86_64 GNU/Linux

Subsystem

node:child_process

What steps will reproduce the bug?

1. Create an ESM file with the following content:

   import { execFile, execFileSync } from "node:child_process";
   
   const stdout = execFileSync(
     "echo",
     ["foo", "bar"],
     { shell: "/bin/bash" },
   );
   console.log(`execFileSync: ${stdout}`); // Outputs: "execFileSync: \n"
   
   execFile(
     "echo",
     ["foo", "bar"],
     { shell: "/bin/bash" },
     (_, stdout) => {
       console.log(`execFile: ${stdout}`); // Output: "execFile: foo bar\n"
     },
   );

2. Note that the options object includes a non-falsy value for the shell option. (for reference, see the child_process.execFile documentation).
3. Run the file using the following command to be able to view the args argument to execFile(Sync) after normalization:

   $ env NODE_DEBUG=child_process node t.js 2>&1 | grep -w args

4. Observe the different behaviour:

   args: [ '/bin/bash', '-c', '/bin/bash -c echo foo bar' ],
   args: [ '/bin/bash', '-c', 'echo foo bar' ],
   

How often does it reproduce? Is there a required condition?

It always reproduces.

What is the expected behavior?

child_process.execFileSync and child_process.execFile invoke commands in the same way given the same arguments. In particular, I believe the behaviour of execFileSync is expected.

What do you see instead?

child_process.execFileSync and child_process.execFile invoke commands in different ways given the same arguments.

Additional information

This bug reports follows from a discussion in #29466 - in particular the discussion starting with this comment of mine. This bug report is based on @bnoordhuis' comment in that thread.

I tested and was able to reproduce this bug on Node v16.15.0 and v18.3.0.

[bnoordhuis]

Create an ESM file

For the record: also happens with CJS.

[zhmushan]

This is caused by normalizeSpawnArguments being called repeatedly. We should avoid using the normalize function for spawn in execFile, even though it works fine most of the time.

This issue can be assigned to me.

[F3n67u]

Could you help me understand why the behavior of execFileSync is expected?

If echo foo bar is interpreted to bash -c 'echo foo bar' when shell is /bin/bash, then foo bar should be printed, right?

$ bash -c "echo foo bar"
foo bar

[zhmushan]

@F3n67u Your understanding is correct, but now execFileSync interprets echo foo bar as /bin/bash -c "/bin/bash -c echo foo bar"

[RaisinTen]

This issue can be assigned to me.

No need to wait for someone to assign you to the issue, feel free to send the PR when your patch is ready. :)

[ericcornelissen]

Could you help me understand why the behavior of execFileSync is expected?

That's based on @bnoordhuis' comments in #29466.

If echo foo bar is interpreted to bash -c 'echo foo bar' when shell is /bin/bash, then foo bar should be printed, right?

That is actually the topic of the discussion I brought up over at #29466 as well.