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

Commands with spaces fail with shell: true #77

Closed
imgx64 opened this issue Jul 30, 2017 · 2 comments
Closed

Commands with spaces fail with shell: true #77

imgx64 opened this issue Jul 30, 2017 · 2 comments

Comments

@imgx64
Copy link

imgx64 commented Jul 30, 2017

Commands with spaces fail with shell: true, but work if it's false. node developers say fixing this from their side would break other things: nodejs/node#7367 . Should this be fixed in cross-spawn? What do you think?

> require('cross-spawn').spawn('C:\\Program Files\\nodejs\\node.exe', [], {shell:true})
undefined
> Error: spawn C:\Program Files\nodejs\node.exe ENOENT
    at notFoundError (C:\Users\redacted\node_modules\cross-spawn\lib\enoent.js:11:11)
    at verifyENOENT (C:\Users\redacted\node_modules\cross-spawn\lib\enoent.js:46:16)
    at ChildProcess.cp.emit (C:\Users\redacted\node_modules\cross-spawn\lib\enoent.js:33:19)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)
> require('cross-spawn').spawn('C:\\Program Files\\nodejs\\node.exe', [], {shell:false})
ChildProcess {
...
@imgx64
Copy link
Author

imgx64 commented Jul 31, 2017

This is a regression in 5.0.0. [email protected] handled this correctly.

satazor added a commit that referenced this issue Nov 11, 2017
- Remove NodeJS v0.10 and v0.12 support
- Change escaping on Windows to use `^` instead of quotes:
    - Fix a bug that made it impossible to escape an argument that contained quotes followed by `>` or other special chars, e.g.: `"foo|bar"`, fixes #82
    - Fix a bug were a command containing `%x%` would be replaced with the contents of the `x` environment variable, fixes #51
- Add a work around for a NodeJS bug when spawning a command with spaces when `options.shell` was enabled, fixes #77
- Fix `options` argument being mutated
- Remove support for running `echo` on Windows
satazor added a commit that referenced this issue Nov 11, 2017
- Remove NodeJS v0.10 and v0.12 support
- Change escaping on Windows to use `^` instead of quotes:
    - Fix a bug that made it impossible to escape an argument that contained quotes followed by `>` or other special chars, e.g.: `"foo|bar"`, fixes #82
    - Fix a bug were a command containing `%x%` would be replaced with the contents of the `x` environment variable, fixes #51
- Add a work around for a NodeJS bug when spawning a command with spaces when `options.shell` was enabled, fixes #77
- Fix `options` argument being mutated
- Remove support for running `echo` on Windows
@satazor
Copy link
Contributor

satazor commented Nov 11, 2017

Sorry for the late response.

When you are using shell: true, you are executing code specific to the platform you are targeting. That's why there's no escape being done in the arguments. The same applies to the command itself.

Having that said, this is by design. Basically, when you use shell:true, you are in charge of everything.

@satazor satazor closed this as completed Nov 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants