[BUG] spawnWithShell is very, very broken on Windows

[rotu]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

There are multiple significant issues with spawnWithShell on Windows:

1. It finds the executable by looking at the string and naively scanning for quotes. This doesn't respect ^-escaped spaces.

2. This quote-scanning doesn't match types of quotes. For instance, "'my double quoted path'" will open quotes at the first " character and then say the quotes have ended at the ' character.

3. Finally, the path is passed to which.sync with the quotes intact. So it's looking for an executable with those quotes in its path.

4. The isCmd regex searches for \ but / is a legal alternative path separator on windows.

5. Shell can be passed in. It's not unlikely that, if the user is on Windows, they're using PowerShell. In that case, I don't know what's supposed to happen.

Originally reported at npm/cli#6716 (comment)

[Voltra]

Related: npm/cli#4225

[Voltra]

The escape functions aren't correct either, working on a PR I found this:

promiseSpawn('node', ['C:\\R&D\\node.js'], {
    shell: 'cmd.exe',
})

Spawns cmd.exe /d /s /c node C:\R^&D\node.js which isn't properly interpreted by node (the script name is truncated right after the R).

However, after fiddling within cmd I found out that cmd.exe /d /s /c node "C:\R&D\node.js" is valid and does what it should.

Thus, C:\R&D\node.js should be escaped as "C:\R&D\node.js" instead of C:\R^&D\node.js

[Voltra]

I suggest taking inspiration from Symfony's Process::prepareWindowsCommandLine to escape arguments.

It does the right thing when you pass an array to the constructor (e.g. new Process(['node', 'D:\\R&D\\node.js'])->run()).