[PATCH v2 0/1] MR9881: cmd: Replace '/' to '\' for path in 'WCMD_pushd'
The parameter of the 'pushd' command may contain the character ‘/’, which is compatible with Windows. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> -- v2: cmd: Allow '/' in quoted 'WCMD_pushd' args. https://gitlab.winehq.org/wine/wine/-/merge_requests/9881
From: YeshunYe <yeyeshun@uniontech.com> The parameter of the 'pushd' command may contain ‘/’, which is compatible with Windows if quoted. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- programs/cmd/builtins.c | 9 ++++++++- 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 + programs/cmd/wcmd.h | 2 +- 6 files changed, 13 insertions(+), 2 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 7d1ad5c1e15..70848378a17 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1774,7 +1774,7 @@ RETURN_CODE WCMD_goto(void) * Push a directory onto the stack */ -RETURN_CODE WCMD_pushd(const WCHAR *args) +RETURN_CODE WCMD_pushd(WCHAR *args) { struct env_stack *curdir; WCHAR *thisdir; @@ -1783,6 +1783,13 @@ RETURN_CODE WCMD_pushd(const WCHAR *args) if (!*args) return errorlevel = NO_ERROR; + if (args[0] == '\"' && args[wcslen(args) - 1] == '\"') { + WCHAR *backslask; + while ((backslask = wcschr(args, '/'))) { + *backslask = '\\'; + } + } + if (wcschr(args, '/') != NULL) { SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 1bd3de55292..518752d5861 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -190,6 +190,7 @@ call :setError 666 & (pushd abc &&echo SUCCESS !errorlevel!||echo FAILURE !error call :setError 666 & (pushd abc &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (popd abc &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (popd &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (pushd "abc/"&&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & popd & echo ERRORLEVEL !errorlevel! cd .. && rd /q /s foo diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 397b720c3bf..1fc6db28e2f 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -123,6 +123,7 @@ SUCCESS 0 FAILURE 1 SUCCESS 666 FAILURE 1 +SUCCESS 0 ERRORLEVEL 666 --- success/failure for DIR command FAILURE 1 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 36b36699772..fc87b8dd83d 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -773,6 +773,7 @@ call :setError 666 & (pushd abc &&echo SUCCESS !errorlevel!||echo FAILURE !error call :setError 666 & (pushd abc &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (popd abc &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (popd &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (pushd "abc/"&&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & popd & echo ERRORLEVEL !errorlevel! cd .. && rd /q /s foo diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index d169066a167..409021ad4a9 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -642,6 +642,7 @@ SUCCESS 0 FAILURE 1 SUCCESS 666 FAILURE 1 +SUCCESS 0 ERRORLEVEL 666 --- success/failure for DIR command FAILURE 1 diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index c8d0d03e9e6..ee11d83c920 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -193,7 +193,7 @@ RETURN_CODE WCMD_output_asis_stderr(const WCHAR *message); RETURN_CODE WCMD_pause(void); RETURN_CODE WCMD_popd(void); void WCMD_print_error (void); -RETURN_CODE WCMD_pushd(const WCHAR *args); +RETURN_CODE WCMD_pushd(WCHAR *args); RETURN_CODE WCMD_remove_dir(WCHAR *command); RETURN_CODE WCMD_rename(void); RETURN_CODE WCMD_setlocal(WCHAR *args); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9881
On Wed Jan 21 06:42:27 2026 +0000, Yeshun Ye wrote:
I've discovered that the behavior of `pushd` on Windows 7 is indeed quite complex. I need to conduct more tests to understand the rules behind how `pushd` handles quotes and `/`. I have written some test cases and run them on Windows 7, but I cannot discern any consistent pattern from the success or failure of these tests. I suspect that the edge cases in the tests may have triggered bugs in Windows 7's `cmd.exe`:smile: Windows 10 behaves the same as Windows 7. I have decided against trying to solve this problem comprehensively at once. This time, I will only fix the issue where arguments wrapped in quotes contain a `/`, ensuring that Wine's behavior aligns with Windows.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9881#note_127485
eric pouech (@epo) commented about programs/cmd/builtins.c:
if (!*args) return errorlevel = NO_ERROR;
+ if (args[0] == '\"' && args[wcslen(args) - 1] == '\"') {
solving this requires touching the arguments parsing, which is currently quite messy there's are several types of helpers (each one with limitations, each one with bugs) before this is worked out, I think the better approach in this case is to use the default parsing (storing values in global variables quals, param1, param2) target is to get rid of there global variables, but the nice part is that they separate parameters from options (qualifiers: starting with /) this patch also ensures that a single arg is passed to pushd this patch should be better ``` diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 7d1ad5c1e15..5272719bf0b 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1783,10 +1783,11 @@ RETURN_CODE WCMD_pushd(const WCHAR *args) if (!*args) return errorlevel = NO_ERROR; - if (wcschr(args, '/') != NULL) { - SetLastError(ERROR_INVALID_PARAMETER); - WCMD_print_error(); - return errorlevel = ERROR_INVALID_FUNCTION; + if (*quals || *param2) + { + SetLastError(ERROR_INVALID_PARAMETER); + WCMD_print_error(); + return errorlevel = ERROR_INVALID_FUNCTION; } curdir = xalloc(sizeof(struct env_stack)); @@ -1796,7 +1797,7 @@ RETURN_CODE WCMD_pushd(const WCHAR *args) lstrcpyW(quals, L"/D"); GetCurrentDirectoryW (1024, thisdir); - return_code = WCMD_setshow_default(args); + return_code = WCMD_setshow_default(param1); if (return_code != NO_ERROR) { free(curdir); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9881#note_127624
On Wed Jan 21 13:42:39 2026 +0000, eric pouech wrote:
solving this requires touching the arguments parsing, which is currently quite messy there's are several types of helpers (each one with limitations, each one with bugs) before this is worked out, I think the better approach in this case is to use the default parsing (storing values in global variables quals, param1, param2) target is to get rid of there global variables, but the nice part is that they separate parameters from options (qualifiers: starting with /) this patch also ensures that a single arg is passed to pushd this patch should be better ``` diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 7d1ad5c1e15..5272719bf0b 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1783,10 +1783,11 @@ RETURN_CODE WCMD_pushd(const WCHAR *args) if (!*args) return errorlevel = NO_ERROR;
- if (wcschr(args, '/') != NULL) { - SetLastError(ERROR_INVALID_PARAMETER); - WCMD_print_error(); - return errorlevel = ERROR_INVALID_FUNCTION; + if (*quals || *param2) + { + SetLastError(ERROR_INVALID_PARAMETER); + WCMD_print_error(); + return errorlevel = ERROR_INVALID_FUNCTION; }
curdir = xalloc(sizeof(struct env_stack)); @@ -1796,7 +1797,7 @@ RETURN_CODE WCMD_pushd(const WCHAR *args) lstrcpyW(quals, L"/D"); GetCurrentDirectoryW (1024, thisdir);
- return_code = WCMD_setshow_default(args); + return_code = WCMD_setshow_default(param1); if (return_code != NO_ERROR) { free(curdir); ``` Your solution doesn't seem very feasible. This is because Windows supports input in this format: `"pushd path with space"`. If a directory named `"path with space"` exists in the current directory, this command will execute successfully and navigate into `"path with space"`. During the argument parsing stage, we cannot treat this as a single parameter because it is not wrapped in quotes. `pushd` currently works well, and I don't think making extensive modifications is a good idea. I lean more towards patching its minor flaws. As mentioned earlier, Windows's handling is very complex when dealing with combinations of quotes, `/`, and spaces. Without fully grasping the underlying rules, we should not attempt to perfectly solve the problem all at once and expect to achieve behavior identical to Windows.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9881#note_127667
On Mon Jan 26 09:11:54 2026 +0000, Yeshun Ye wrote:
Your solution doesn't seem very feasible. This is because Windows supports input in this format: `"pushd path with space"`. If a directory named `"path with space"` exists in the current directory, this command will execute successfully and navigate into `"path with space"`. During the argument parsing stage, we cannot treat this as a single parameter because it is not wrapped in quotes. `pushd` currently works well, and I don't think making extensive modifications is a good idea. I lean more towards patching its minor flaws. As mentioned earlier, Windows's handling is very complex when dealing with combinations of quotes, `/`, and spaces. Without fully grasping the underlying rules, we should not attempt to perfectly solve the problem all at once and expect to achieve behavior identical to Windows. I don't like the idea of idea a specific arg parsing code for every command (but yes, PUSHD and DIR don't have the same)
moreover adding small fixes without grasping the underlying logic generates spaghetti code, that we're trying to remove from cmd.exe perhaps the simplest way forward would be to fail if first char is a '/', then remove enclosing quotes if any, and use the result as directory argument -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9881#note_127991
perhaps the simplest way forward would be to fail if first char is a '/', then remove enclosing quotes if any, and use the result as directory argument
Okay, I will give it a try. Do you mean we should perform this operation within `WCMD_pushd` ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9881#note_128086
participants (3)
-
eric pouech (＠epo) -
Yeshun Ye (＠yeyeshun)
-
YeshunYe