[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
participants (2)
-
Mykola Mykhno -
Mykola Mykhno (＠MykolaMykhno)