Cannot spawn shell script if path has spaces

[robertpatrick]

• Version: 14.16.0
• Platform: MacOS 10.15.7 (Windows and Linux too)
• Subsystem: child_process

What steps will reproduce the bug?

1. Create a javascript program that spawns a shell script.
2. Ensure that the full path to the shell script has spaces in the directory names.
3. Run the javascript program.

async function executeChildProcess(executable, argList) {
  const child = spawn(executable, argList, {
    stdio: [ 'pipe', 'pipe', 'pipe' ],
    shell: true,
    detached: false,
    windowsHide: true
  });
  await onExit(child)
    .then(() => console.log(`${executable} exited with exit code 0`))
    .catch((err) => console.log(`${executable} failed: ${err.toString()}`));

  return child.exitCode;
}

executeChildProcess('/Applications/My Application.app/Contents/tools/myScript.sh', [ '-v' ]).then();

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

Every single time. Yes, the child_process module should be properly escaping commands that have spaces in them since we cannot control where the user installs the application.

What is the expected behavior?

The shell script is executed properly

What do you see instead?

/bin/sh: /Applications/My: No such file or directory
Applications/My Application.app/Contents/tools/myScript.sh failed: Error: Exit with error code: 127

Additional information

[pd4d10]

Cannot reproduce on my local machine.

Could you please provide more information, for example, what's the implementation of onExit function?

[Ayase-252]

I can reproduce on Node.js v16

const { spawn } = require('child_process');
const path = require('path');

const cp = spawn(
  path.resolve(__dirname, 'with space', 'shell.sh'),
  {
    shell: true,
  });

cp.stderr.on('data', (chunk) => {
  console.log(chunk.toString());
});

It exits with 127 and shows

/bin/sh: /Users/qingyudeng/projects/node/with: No such file or directory

[pd4d10]

I guess it's caused by the shell: true option. Set to false should solve this issue.

Or add quotes if shell is necessarily set to true:

spawn('"path with space.sh"')

[robertpatrick]

Thanks, I had already figured out the workaround with extra quotes. I still consider this a bug that should be fixed since adding quotes to a Javascript string is pretty non-intuitive...

[RaisinTen]

Another way is to escape the space character:

const { spawn } = require('child_process');
const path = require('path');

const cp = spawn(
  path.resolve(__dirname, 'with\\ space', 'shell.sh'),
  {
    shell: true,
  });

cp.stdout.on('data', (chunk) => {
  console.log(chunk.toString());
});

Also, this behaviour seems to be compliant with the way the C library function system works, which also invokes the shell:

#include <stdlib.h>

int main() {
  system("with\\ space/shell.sh");

  return 0;
}

[robertpatrick]

@RaisinTen step back away from the fix and think about the child_process.spawn API. From an API perspective, I am passing the executable argument as a single Javascript string, as specified by the API. I shouldn't have to understand the underlying implementation requires me to "fix up" the argument in certain scenarios. You can make all the arguments you want but from an API user perspective, the implementation should handle this for me since it is a limitation of the implementation.

By the way, I suspect that the argument list elements may suffer from the same problem...

[Trott]

I don't know for certain, but I'm going to guess this is the expected behavior with shell: true and will not be considered a bug. But let's check with @nodejs/child_process.

[schamberg97]

Perhaps the compromise could be specifying the workarounds in the docs?

[Trott]

Perhaps the compromise could be specifying the workarounds in the docs?

Yeah, definitely, if it's not a bug, that should be included in the docs with an example. It is in there already, but kind of hidden in a section that applies only to Windows.

[robertpatrick]

Agreed. While spaces and other special characters are more common on Windows, they are also possible on Unix-based platforms. We are building an Electron-based app and packaging some scripts with the app. Because of how MacOS deals with application names mapping to the .app directory, we end up with spaces in the path because we want our application name to appear in the Launcher in human-readable form rather than Pascal- or camel-case...

[RaisinTen]

@robertpatrick Would you like to send a PR to fix this issue?

[robertpatrick]

I am happy to take a whack at it if I can figure out where the relevant code is and how to build/test it.

[RaisinTen]

@robertpatrick
The docs for child_process.spawn() lies here:
https://github.com/nodejs/node/blob/master/doc/api/child_process.md#child_processspawncommand-args-options
and here is the guide on how you can make the change:
https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md

[koush]

None of these workarounds work for a path with spaces in Windows as far I can tell from testing.

[koush]

Also, this behaviour seems to be compliant with the way the C library function system works, which also invokes the shell:

@RaisinTen

The "system" command in C and the "exec" command in C are different. Exec takes the path to an executable as the first argument. So this is named poorly so as to be misleading, since this behavior happens with child_process.exec/spawn. There is no child_process.system. As far as I can tell, there's no way to call C exec (which would give desirable behavior with spaces with paths).

[RaisinTen]

@koush

None of these workarounds work for a path with spaces in Windows as far I can tell from testing.

Using "s works on CMD for me:

C:\Users\Administrator\Desktop\temp>type "Hello world\test.bat"
echo Hi
C:\Users\Administrator\Desktop\temp>"Hello world\test.bat"

C:\Users\Administrator\Desktop\temp>echo Hi
Hi

C:\Users\Administrator\Desktop\temp>node -p "child_process.execSync('\"Hello world\\test.bat\"').toString()"

C:\Users\Administrator\Desktop\temp>echo Hi
Hi

Also, this behaviour seems to be compliant with the way the C library function system works, which also invokes the shell:

@RaisinTen

The "system" command in C and the "exec" command in C are different. Exec takes the path to an executable as the first argument.

I never said that they are the same.

So this is named poorly so as to be misleading, since this behavior happens with child_process.exec/spawn. There is no child_process.system.

It's unlikely that the name will change anytime soon.

As far as I can tell, there's no way to call C exec (which would give desirable behavior with spaces with paths).

Is this what you're looking for #21664?

[robertpatrick]

By putting an extra set of double quotes, I was able to get spawn to work properly.

const command = ‘“/path with spaces/to/executable”’;
const args = [
    ‘-f’,
    ‘“arg with spaces”’
];
const cp = spawn(command, args, …);

[koush]

I never said that they are the same.

You indicated that this is the expected behavior of "system" which is named after the system C call which is different from the exec C call. Citing that as the reason for their behavior. The confusion is understandable when they're not analogous to behavior of POSIX functions, because they're named after mismatched POSIX counterparts. I expect to have to quote args in C "system", but I do not expect that for "exec".

It's unlikely that the name will change anytime soon.

Yeah, I get that.

Is this what you're looking for #21664?

Not exactly, process replacement isn't the issue. What we need is having a way to launch a process without having to worry about quoting the executable (something that did not work for me, but I'll try again). This is problematic because the execFile/spawn call accepts "args" arguments: why would one need to quote the executable in the "command" parameter to prevent it from being arg split, if there exists an args parameter? I expected that the executable path is the simply the "command" parameter, and the "args" to contain the arguments.

This is a bug farm lurking in the API. Every consumer of the process.exec/spawn API is going to eventually run into this quoting issue when a user of theirs has the program in a path with a space. Then they'll come find this bug, and go back and fix their code.

A non-replacement "exec" exists in the windows APIs for this purpose (as far as I know, exec-replace doesn't exist on windows). https://docs.microsoft.com/en-us/cpp/c-runtime-library/exec-wexec-functions?view=msvc-160