[PATCH v5 0/2] MR10797: cmd: Add a second set of illegal characters for the quotes strip
Prior to XP Professional x64 Edition, logic was that special characters such as: `&,<>,()@^` were forbidden in quotes. Quotes must be stripped if any of these characters appear in a parameter. This logic is documented in the cmd.exe documentation at MSDN. However, after XP Professional x64 Edition, Microsoft added the `Program Files (x86)` directory, and it seems like they removed parentheses from the list of forbidden characters in quotes. The documentation doesn't mention it. I have checked that XP Professional SP 2 x32 bit strips quotes when it finds parentheses in a parameter passed with /C, and XP Professional x64 Edition and all versions after it, whether 32 or 64 bit, preserve quotation marks when parentheses are in a parameter. Also, I checked that XP Professional x64 Edition is based already on NT 5.2, same with Windows Server 2003. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=37789 -- v5: cmd: Remove parentheses from list of illegal characters for quote strip cmd: Update tests for parentheses in quoted parameter https://gitlab.winehq.org/wine/wine/-/merge_requests/10797
From: Mykola Mykhno <klimmihno5+wine+gitlab@gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=37789 --- programs/cmd/tests/test_cmdline.cmd | 8 ++++++++ programs/cmd/tests/test_cmdline.cmd.exp | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/programs/cmd/tests/test_cmdline.cmd b/programs/cmd/tests/test_cmdline.cmd index 8f3143d0700..a31c248012d 100644 --- a/programs/cmd/tests/test_cmdline.cmd +++ b/programs/cmd/tests/test_cmdline.cmd @@ -101,6 +101,8 @@ echo @echo 2 > "saytwo.bat" echo @echo 3 > "say (3).bat" echo @echo 4 > "say .bat" echo @echo 5 > "bazbaz(5).bat" +mkdir "foo (1)" +echo @echo 6 > "foo (1)\saysix.bat" echo ------ Testing invocation of batch files ---------- call say one @@ -145,6 +147,9 @@ cmd /c s"aytwo if errorlevel 2 echo error %ErrorLevel% cmd /c say (3) call :setError 0 +cmd /c "say (3)" +if errorlevel 2 echo error %ErrorLevel% +call :setError 0 cmd /c say" (3)" if errorlevel 2 echo error %ErrorLevel% call :setError 0 @@ -155,6 +160,7 @@ rem Deliberately invoking a fully qualified batch name containing a bracket rem should fail, as a bracket is a command delimiter. cmd /c "bazbaz(5).bat" if errorlevel 1 echo Passed +cmd /c "foo (1)\saysix.bat" echo ---------- Testing CMD /C quoting ----------------- cmd /c @echo "hi" @@ -284,6 +290,8 @@ call tell(12(34) call tell(12;34) echo --------- Finished -------------- del tell.bat say*.* bazbaz*.bat +del "foo (1)\saysix.bat" +rmdir "foo (1)" exit :setError exit /B %1 diff --git a/programs/cmd/tests/test_cmdline.cmd.exp b/programs/cmd/tests/test_cmdline.cmd.exp index cb9d306b487..9de3e60b389 100644 --- a/programs/cmd/tests/test_cmdline.cmd.exp +++ b/programs/cmd/tests/test_cmdline.cmd.exp @@ -71,8 +71,10 @@ var=33@space@ 2@space@ 0@space@ 3@space@ +3@space@ 4@space@ Passed +6@space@ ---------- Testing CMD /C quoting ----------------- "hi" 1@space@ @@ -82,7 +84,7 @@ Passed 1@space@ 0@space@ 0@space@ -0@space@@or_broken@3@space@ +3@space@@or_broken@0@space@ 3@space@ 2@space@ 2@space@ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10797
From: Mykola Mykhno <klimmihno5+wine+gitlab@gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=37789 --- programs/cmd/wcmdmain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 4aec2e882e9..467209a053b 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -4849,7 +4849,7 @@ static void parse_command_line_parameters(struct cmd_parameters *parameters) /* opt_s left unflagged if the command starts with and contains exactly * one quoted string (exactly two quote characters). The quoted string * must be an executable name that has whitespace and must not have the - * following characters: &<>()@^| + * following characters: &<>@^| */ /* 1. Confirm there is at least one quote */ @@ -4867,7 +4867,7 @@ static void parse_command_line_parameters(struct cmd_parameters *parameters) opt_s = TRUE; for (p = q1; p != q2; p++) { - if (wcschr(L"&<>()@^'", *p)) + if (wcschr(L"&<>@^'", *p)) { opt_s = TRUE; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10797
Thank you for the review! I understand the general part better now. For some reason, after reading the Wine developer documentation, I thought tests should be in a separate commit only for significant changes. I just read the documentation on winecfg, and now I understand why version-dependent paths should be avoided. Yes, simpler solution would be better in this case. I updated the merge request, so it should be better now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10797#note_138561
On Tue May 5 11:32:27 2026 +0000, Mykola Mykhno wrote:
Thank you for the review! I understand the general part better now. For some reason, after reading the Wine developer documentation, I thought tests should be in a separate commit only for significant changes. I just read the documentation on winecfg, and now I understand why version-dependent paths should be avoided. Yes, simpler solution would be better in this case. I updated the merge request, so it should be better now. we want every commit to pass the non regression tests (so that, eg, bisecting later on will not fail)
so you need to add @todo_wine@ in the cmd.exp file for the failing tests in first commit, and remove them (for the tests that now pass) in the second commit -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10797#note_138864
eric pouech (@epo) commented about programs/cmd/tests/test_cmdline.cmd.exp:
1@space@ 0@space@ 0@space@ -0@space@@or_broken@3@space@ +3@space@@or_broken@0@space@ it seems to me the 0 case is for the case where ( or ) is not supported
tested on window 8/10/11: removing the @or_broken@0@space@ now passes as all the other tests don't have the or_broken clause, I'd suggest doing some house keeping and aligning all the tests to a window 7+ behavior -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10797#note_138865
participants (3)
-
eric pouech (＠epo) -
Mykola Mykhno
-
Mykola Mykhno (＠MykolaMykhno)