Fix Windows argument quoting for backslash runs before quotes - #101
Open
ServeurpersoCom wants to merge 2 commits into
Open
Fix Windows argument quoting for backslash runs before quotes#101ServeurpersoCom wants to merge 2 commits into
ServeurpersoCom wants to merge 2 commits into
Conversation
The command line builder doubles a backslash only when the next source character is a quote. Per the CRT parsing rules, every backslash in a run preceding a double quote must be doubled, and this includes a run that ends up before the generated closing quote of a quoted argument. As a result, a quoted argument ending in a backslash swallows its closing quote and merges with every following argument, a run of two or more backslashes before an embedded quote is under-escaped, and a quoted argument ending in several backslashes loses half of them. Count backslash runs in the length and emit passes and double the whole run before an escaped quote or before the closing quote. Extend the special argv test with the three cases plus a merge canary.
4 tasks
Rework the two command line builder passes without changing the emitted bytes. The length pass becomes a linear scan that carries the current backslash run, and the emit pass folds the duplicated write loops into the explicit 2n + 1 and 2n CRT formulas. Extend the special argv test with a quoted argument combining a space and a backslash run before an embedded quote.
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
1 participant
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.
As explained in #100, we hit this in llama.cpp where subprocess.h launches MCP servers on Windows: an argument like C:\my dir\ reaches the child merged with everything after it, because the trailing backslash escapes the closing quote the builder adds.
Proposed fix: count backslash runs and double the whole run when it lands before a quote, including the generated closing quote, as the CRT parsing rules expect.
The extended special argv test fails on Windows without the fix and passes with it; on POSIX argv is passed as-is. Tested on a dedicated physical Windows 11 machine (before/after screenshot in #100).
Fixes #100