[PATCH v4 0/1] 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 -- v4: cmd: Add a second set of illegal characters for the quotes strip 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 +++- programs/cmd/wcmdmain.c | 20 ++++++++++++++------ 3 files changed, 25 insertions(+), 7 deletions(-) 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@ diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 4aec2e882e9..4f38ebafd31 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -59,6 +59,7 @@ int errorlevel; WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; FOR_CONTEXT *forloopcontext; /* The 'for' loop context */ BOOL delayedsubst = FALSE; /* The current delayed substitution setting */ +RTL_OSVERSIONINFOEXW os_version; WCHAR anykey[100], version_string[100]; @@ -4842,6 +4843,9 @@ static void parse_command_line_parameters(struct cmd_parameters *parameters) if (parameters->opt_c || parameters->opt_k) { WCHAR *q1 = NULL, *q2 = NULL, *p; + BOOL is_version_before_64_bit_support = (os_version.dwMajorVersion < 6) || + (os_version.dwMajorVersion == 5 && os_version.dwMinorVersion <= 1); + WCHAR illegal_characters[9] = L"&<>@^'"; /* Take a copy */ parameters->initial_command = xstrdupW(arg); @@ -4849,8 +4853,13 @@ 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: + * &<>()@^| - prior 5.2 version (only 32-bit systems supported) + * &<>@^| - after 5.2 version (64-bit support added) */ + if (is_version_before_64_bit_support) { + wcscat(illegal_characters, L"()"); + } /* 1. Confirm there is at least one quote */ if (!opt_s && !(q1 = wcschr(arg, L'"'))) opt_s = TRUE; @@ -4867,7 +4876,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(illegal_characters, *p)) { opt_s = TRUE; break; @@ -4902,7 +4911,6 @@ static void parse_command_line_parameters(struct cmd_parameters *parameters) static void WCMD_setup(void) { WCHAR string[MAX_PATH]; - RTL_OSVERSIONINFOEXW osv; char osver[50]; WCHAR *cmd; @@ -4928,12 +4936,12 @@ static void WCMD_setup(void) } /* Get the windows version being emulated */ - osv.dwOSVersionInfoSize = sizeof(osv); - RtlGetVersion(&osv); + os_version.dwOSVersionInfoSize = sizeof(os_version); + RtlGetVersion(&os_version); /* Pre initialize some messages */ lstrcpyW(anykey, WCMD_LoadMessage(WCMD_ANYKEY)); - sprintf(osver, "%ld.%ld.%ld", osv.dwMajorVersion, osv.dwMinorVersion, osv.dwBuildNumber); + sprintf(osver, "%ld.%ld.%ld", os_version.dwMajorVersion, os_version.dwMinorVersion, os_version.dwBuildNumber); cmd = WCMD_format_string(WCMD_LoadMessage(WCMD_VERSION), osver); lstrcpyW(version_string, cmd); LocalFree(cmd); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10797
generally: * we'd prefer when tests are added in a separate commit (in the same MR), marking failing tests with todo_wine so that it's easier to review what's fixed (in the second commit) * we also tend to avoid having code paths depending on a given window version also tests as they are added will fail if run with Wine set with an old version (not that we still test in the CI such an old version) so I wonder if this couldn't be simplified by just removing the parenthesis from the set of characters? (iow, since XP Professional x64 Edition has shipped, eg 21 years ago, all .bat/.cmd files have run on these OSes and subsequent ones have run without the offending ()...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10797#note_138523
participants (3)
-
eric pouech (＠epo) -
Mykola Mykhno
-
Mykola Mykhno (＠MykolaMykhno)