Allow commands/arrgs in spawnSync() to contain whitespace #821
Conversation
| wizer, | ||
| `"${wizer}"`, | ||
| [ | ||
| "--inherit-env=true", | ||
| "--allow-wasi", | ||
| "--dir=.", | ||
| ...starlingMonkey ? [`--dir=${dirname(wizerInput)}`] : [], | ||
| `--wasm-bulk-memory=true`, | ||
| "-r _start=wizer.resume", | ||
| `-o=${output}`, | ||
| wasmEngine, | ||
| `-o="${output}"`, | ||
| `"${wasmEngine}"`, |
There was a problem hiding this comment.
Normally spawnSync doesn't require escaping of arguments as it automatically handles this (and this is also why windowsVerbatimArguments is an option).
Therefore unless we can prove there is a test case where this fails, I'm pretty sure this code path should support spaces okay in the command and arguments.
There was a problem hiding this comment.
Unfortunately it still needs the double quotes around the input on L#120 and around the args on L#128-L#129
|
I've posted #822 with just the check changes, which I believe may be sufficient, but please let me know if I'm mistaken as well. |
|
Hi @guybedford! I have tried fixing only the check in containsSyntaxErrors #L9 but then it broke in the other It still broken when including the fix for the input command for And including the fix for the arg And finally by including the fix for the I hope this clear things out 😃 |
|
Okay, thank you for confirming all the usages are required. Yeah it seems this is because of the Regardless will merge this for the next release, thank you!. |
This PR fixes whitespaces in commands and arrgs when calling
spawnSync()A simple fix is to always wrap commands and arrgs with double quotes to avoid similar issues.
Issue: #820