[PATCH 0/2] MR9996: msvcrt: Correct backslash handling in argument parser.
The `cmdline_to_argv` function previously had a bug in handling backslashes preceding double quotes, particularly regarding the escaping rules. \ When processing a case like `"\"c:\windows\\""`, the flawed logic incorrectly consumed the quote character, leading to an error in parsing the number of arguments. Signed-off-by: YeshunYe yeyeshun@uniontech.com -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9996
From: YeshunYe <yeyeshun@uniontech.com> Added a test case to verify the scenario where the path following `/d` ends with a backslash (`\`). Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- programs/cmd/tests/test_builtins.bat | 1 + programs/cmd/tests/test_builtins.bat.exp | 1 + programs/cmd/tests/test_builtins.cmd | 1 + programs/cmd/tests/test_builtins.cmd.exp | 1 + 4 files changed, 4 insertions(+) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 4c3d0e7107e..19cf4f202aa 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -96,6 +96,7 @@ mkdir foo & cd foo set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) +call :setError 666 & (start /B /WAIT /d "%FOO_PATH%\" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 36e14d5be91..20dda0c9ca7 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -63,6 +63,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +@todo_wine@0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index f65a301d914..91af319b686 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -679,6 +679,7 @@ mkdir foo & cd foo set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) +call :setError 666 & (start /B /WAIT /d "%FOO_PATH%\" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f6d9bdb494..f5358709f83 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -582,6 +582,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +@todo_wine@0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9996
From: YeshunYe <yeyeshun@uniontech.com> The `cmdline_to_argv` function previously had a bug in handling backslashes preceding double quotes, particularly regarding the escaping rules. When processing a case like `"\"c:\windows\\""`, the flawed logic incorrectly consumed the quote character, leading to an error in parsing the number of arguments. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/msvcrt/data.c | 6 +++--- programs/cmd/tests/test_builtins.bat.exp | 2 +- programs/cmd/tests/test_builtins.cmd.exp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/msvcrt/data.c b/dlls/msvcrt/data.c index 6e764e52eae..6166f7540b0 100644 --- a/dlls/msvcrt/data.c +++ b/dlls/msvcrt/data.c @@ -119,8 +119,8 @@ static WCHAR **cmdline_to_argv( const WCHAR *src, int *ret_argc ) */ dst -= bcount / 2; src++; - if (in_quotes && *src == '"') *dst++ = *src++; - else in_quotes = !in_quotes; + in_quotes = !in_quotes; + if (bcount) bcount--; } else { @@ -129,8 +129,8 @@ static WCHAR **cmdline_to_argv( const WCHAR *src, int *ret_argc ) */ dst -= bcount / 2 + 1; *dst++ = *src++; + bcount = 0; } - bcount = 0; } else /* a regular character */ { diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 20dda0c9ca7..383c237b684 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -63,7 +63,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 -@todo_wine@0 +0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index f5358709f83..c88dee42c16 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -582,7 +582,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 -@todo_wine@0 +0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9996
This merge request was closed by Yeshun Ye. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9996
The error I encountered was with `start /d "c:\temp\" test.exe`, which was parsed into the arguments `/d` and `"c:\temp\" test.exe`. The path and the executable were merged into a single argument as a new path, preventing the child process from launching. In my first patch, I attempted to identify and split this combined argument, but this caused test failures in `kernelbase`. I then retested on Windows and found that Windows also parses it as `/d` and `"c:\temp\" test.exe`—Wine's parsing is consistent with Windows. However, `start` works correctly on Windows because it is a built-in command, whereas in Wine, `start` is an external executable, leading to different behavior. I will revert the changes to the parsing logic and fix the issue directly within the `start` itself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9996#note_128507
participants (2)
-
Yeshun Ye (＠yeyeshun) -
YeshunYe