Skip to content

shell option for execFileSync , the parameters cannot be obtained in the script #29466

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

Closed
rambo-panda opened this issue Sep 6, 2019 · 8 comments
Labels
child_process Issues and PRs related to the child_process subsystem. invalid Issues and PRs that are invalid.

Comments

@rambo-panda
Copy link

rambo-panda commented Sep 6, 2019

I'm not sure if it's a bug or whether it's the design intent.

  • Version: v10.15.2
  • Platform: Darwin bogon 18.7.0 Darwin Kernel Version 18.7.0 x86_64
  • Subsystem:

bin/test.sh

#!/bin/bash

echo $1
echo $2

image

here is my debug info:

return child_process.spawnSync(opts);

image

the final exec script

/bin/sh -c ./bin/test.sh arg0 arg1

here is my trying
image
the final exec script

/bin/sh -c "./bin/test.sh arg0 arg1"
@rambo-panda
Copy link
Author

rambo-panda commented Sep 6, 2019

it's work fine in window 10 + git bash

@bnoordhuis
Copy link
Member

bnoordhuis commented Sep 6, 2019

This command:

child_process.execFileSync('./bin/test.sh', ['arg0', 'arg1'], {shell: true})

Ends up executing this (note the quotes):

/bin/sh -c "/bin/sh -c ./bin/test.sh arg0 arg1"

Which then executes (note the lack of quotes):

/bin/sh -c /bin/sh -c ./bin/test.sh arg0 arg1

-c takes exactly one argument so arg0 and arg1 are effectively discarded here. Therefore, what's ultimately executed is this:

./bin/test.sh

Compare sh -c /bin/echo no bueno, that doesn't print anything either.

If you drop the {shell: true} it works like (I think) you expect it to.

I'm going to close this out because it's not a bug, just (perhaps not completely obvious) shell interaction.

@bnoordhuis bnoordhuis added child_process Issues and PRs related to the child_process subsystem. invalid Issues and PRs that are invalid. labels Sep 6, 2019
@rambo-panda
Copy link
Author

@bnoordhuis

thanks

as you said

Ends up executing this (note the quotes):
/bin/sh -c "/bin/sh -c ./bin/test.sh arg0 arg1"

l tried it too

/bin/sh -c "/bin/sh -c \"./bin/webp.sh arg0 arg1\""

it work fine

thank again

@ericcornelissen
Copy link

If you drop the {shell: true} it works like (I think) you expect it to.

[...] it's not a bug, just (perhaps not completely obvious) shell interaction.

That statement make some sense to me for {shell: true}, but what about something like {shell: "/bin/bash"} - i.e. if the intend is to use a specific shell (e.g. because a specific shell's feature set is required)?

The provided explanation suggests to me that it is expected that execFileSync cannot be used with both a custom shell and arguments. is that correct, @bnoordhuis?

On a related note, the described problem does not exist with execFile. That discrepancy is unexpected to me.

For reference, I tested this on Ubuntu 20.04.4 LTS with both Node v16.15.0 and Node v18.3.0 using the script:

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"
  }
);

@bnoordhuis
Copy link
Member

@ericcornelissen

The provided explanation suggests to me that it is expected that execFileSync cannot be used with both a custom shell and arguments. is that correct

I can't really answer that with yes or no because it depends on the shell. For Bourne-like shells: yes.

the described problem does not exist with execFile

I think you've spotted a bug. This is args after normalization:

$ env NODE_DEBUG=child_process node t.js 2>&1 | grep -w args
  args: [ '/bin/bash', '-c', '/bin/bash -c echo foo bar' ],
  args: [ '/bin/bash', '-c', 'echo foo bar' ],

Can you open a new issue for that?

@ericcornelissen
Copy link

I can't really answer that with yes or no because it depends on the shell. For Bourne-like shells: yes.

I see. Rereading everything in this thread following your comment clarified it to me.

Still, I can't help but wonder if that behaviour should be fixed. What if, rather than joining the args if options.shell is truthy:

node/lib/child_process.js

Lines 564 to 565 in b631086

if (options.shell) {
const command = ArrayPrototypeJoin([file, ...args], ' ');

we pass the individual args to spawn. So, oversimplified, do:

function execFileSync(file, args, options) {
  spawnSync('/bin/bash', ['-c', file, ...args], options);
}

Am I missing something or would that make sense?

Can you open a new issue for that?

Done 🙂

@davewichers
Copy link

@ericcornelissen - Can you reference the new issue you opened for this problem? I just ran into this problem today and think this behavior is bizarre and should be fixed. But I don't know what the 'new' issue that is tracking this problem.

@ericcornelissen
Copy link

@davewichers it's #43333 (there's a reference to it just above my previous comment).

The problem has been fixed for me since v18.7.0 - I don't know if the fix got backported to older LTS releases.

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. invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

4 participants