Repository navigation
windows: quote the CLI's argv for cmd.exe, not a POSIX shell - #792
Open
TurtIeSocks wants to merge 3 commits into
Open
TurtIeSocks wants to merge 3 commits into
TurtIeSocks wants to merge 3 commits into
Conversation
In argv mode the CLI joins its arguments with quote(), which writes POSIX single quotes. On Windows the command runs through `cmd.exe /d /s /c`, and srt-win passes everything after `/c` to cmd verbatim. cmd gives single quotes no meaning, so any argument quote() would not leave bare reached the program with literal quotes around it. A backslash is outside quote()'s bare set, so every absolute Windows path was hit: `srt -- node C:\x\f.js` ran node on `'C:\x\f.js'`. quoteForCmd() quotes each argument for the program's own argv parser (the MSVCRT rules: a backslash is literal unless it comes before a `"`), then caret-escapes every cmd metacharacter, `"` included, so cmd never enters its own quote state and passes the line through intact. The program name is only caret-escaped, since cmd finds the executable before any argv parsing. A word made only of letters, digits, `\`, `:` and `_./@+-` stays bare, so a plain path is unchanged. The CLI uses it on win32 and quote() everywhere else. The tests check the strings on every platform, and on Windows round-trip a fixed list and a fast-check property through a real `cmd.exe /d /s /c`, built the way srt-win's build_cmdline builds it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A caret-escaped space does not stop cmd splitting the executable's path, so `C:\Program Files\nodejs\node.exe` ran as `C:\Program`. cmd honours plain quotes when it finds the executable, and a Windows file name cannot hold a `"`, so the program name is wrapped in them instead. The cmd round-trip now runs node rather than the test runner: bun reads some of a script's arguments as its own, and node's default install path has the space this needs to cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The program name is wrapped in plain quotes for cmd, and inside them cmd still expands `%`, ends the line at a line break, and closes the quotes at a `"`. None of those can appear in a Windows file name except `%`, so a name holding one is refused with a pointer to -c rather than handed to cmd to run as something else. The cmd round-trip runs a script file rather than `node -e`: after -e, node still reads an argument such as `- ` as one of its own options and exits 9, which a property run hit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows the CLI's argv mode runs through
cmd.exe /d /s /c, butquote()writes POSIX single quotes, which cmd passes through literally. Any argument with a\got them, so every absolute path broke.quoteForCmd(): MSVCRT quoting per argument, then^before each cmd metacharacter",%or a line break is refused with a pointer to-c-cmode and the library are unchangedEvidence
Windows 11 ARM64, desktop session,
srt --settings s.json -- node argv.js <args>, whereargv.jsprints its argv:Cannot find module 'C:\Windows\System32\'C:\...\argv.js''After: all 8 args come back intact:
C:\Users\me\f.js,C:\Program Files\x\,a b,x & y,don't,%PATH%,q"uote,""The new cmd round-trip tests in
test/utils/shell-quote.test.tspass on Windows, and the property test passed 2,000 runs locally. The existing POSIX round-trip tests fail on a machine wherebash.exeis the WSL stub, which is unrelated to this change.Merge Danger
Door: two-way
Blast Radius: Windows CLI argv mode
Disclaimer: Code was generated by Claude 🤖 - reviewed and tested by a human 🐢