[PATCH v3 0/2] MR11094: cmd: Add wildcard expansion for type command.
Fixes: https://bugs.winehq.org/show_bug.cgi?id=59816 Signed-off-by: Barath Kannan barathrk11@gmail.com -- v3: cmd: Add wildcard expansion for the type command's arguments. cmd: Add tests for the expansion of wildcards in the type command's arguments. https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
From: Barath Kannan <barathrk11@gmail.com> Signed-off-by: Barath Kannan <barathrk11@gmail.com> --- programs/cmd/tests/test_builtins.bat | 15 +++++++++++---- programs/cmd/tests/test_builtins.bat.exp | 11 +++++++++-- programs/cmd/tests/test_builtins.cmd | 15 +++++++++++---- programs/cmd/tests/test_builtins.cmd.exp | 11 +++++++++-- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 4c3d0e7107e..2b9f2eb5fb1 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -99,16 +99,23 @@ call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\" rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo +mkdir bar +mkdir spam echo a > fileA echo b > fileB call :setError 666 & (type &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- -call :setError 666 & (type i\dont\exist\at\all.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -cd .. && rd /q /s foo - +call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type file* idontexistatall.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type idontexistatall.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type i\dont\exist\at\all.txt file*) +echo FAILURE !errorlevel! +cd .. +del foo\file* +call :setError 666 & (type foo\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +rd /q /s foo echo --- success/failure for COPY command mkdir foo & cd foo echo a > fileA diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 36e14d5be91..0738a87f6b8 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -67,11 +67,18 @@ SUCCESS 1024 FAILURE 1 SUCCESS 0 FAILURE 1 +--- @todo_wine@a@space@ @todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@--- FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index f65a301d914..9b659c8c82a 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -682,16 +682,23 @@ call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\" rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo +mkdir bar +mkdir spam echo a > fileA echo b > fileB call :setError 666 & (type &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- -call :setError 666 & (type i\dont\exist\at\all.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -cd .. && rd /q /s foo - +call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type file* idontexistatall.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type idontexistatall.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type i\dont\exist\at\all.txt file*) +echo FAILURE !errorlevel! +cd .. +del foo\file* +call :setError 666 & (type foo\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +rd /q /s foo echo --- success/failure for COPY command mkdir foo & cd foo echo a > fileA diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f6d9bdb494..79defed5cdb 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -586,11 +586,18 @@ SUCCESS 1024 FAILURE 1 SUCCESS 0 FAILURE 1 +--- @todo_wine@a@space@ @todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@--- FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
From: Barath Kannan <barathrk11@gmail.com> Signed-off-by: Barath Kannan <barathrk11@gmail.com> --- programs/cmd/builtins.c | 127 ++++++++++++++++++++--- programs/cmd/tests/test_builtins.bat.exp | 20 ++-- programs/cmd/tests/test_builtins.cmd.exp | 20 ++-- 3 files changed, 135 insertions(+), 32 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 1c9074b75d3..566576be234 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -30,6 +30,7 @@ #include "wcmd.h" #include <shellapi.h> +#include <shlwapi.h> #include "winternl.h" #include "winioctl.h" #include "ddk/ntifs.h" @@ -3458,22 +3459,124 @@ RETURN_CODE WCMD_type(WCHAR *args) if (!argN) break; WINE_TRACE("type: Processing arg '%s'\n", wine_dbgstr_w(thisArg)); - hIn = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, NULL); - if (hIn == INVALID_HANDLE_VALUE) { - WCMD_print_error (); - WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); - return errorlevel = ERROR_INVALID_FUNCTION; - } - hOut = GetStdHandle(STD_OUTPUT_HANDLE); - if (writeHeaders) { - WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + if (wcspbrk(thisArg, L"*?")) + { + BOOL foundOnlyDirectories = TRUE; + WIN32_FIND_DATAW fd; + HANDLE hff = INVALID_HANDLE_VALUE; + WCHAR *fileNamePart; + DWORD till_file_name_part_len; + DWORD file_name_len; + + hff = FindFirstFileW(thisArg, &fd); + + if (hff == INVALID_HANDLE_VALUE) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + continue; + } + + writeHeaders = TRUE; + + do { + WCHAR srcpath[MAX_PATH]; + + if(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + WINE_TRACE("Skipping directories\n"); + continue; + } + + foundOnlyDirectories = FALSE; + lstrcpyW(srcpath, thisArg); + + fileNamePart = PathFindFileNameW(srcpath); + + /* Calculate length till fileNamePart */ + till_file_name_part_len = wcslen(thisArg) - wcslen(fileNamePart); + file_name_len = wcslen(fd.cFileName); + + if ((till_file_name_part_len + file_name_len) > MAX_PATH) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + + FindClose(hff); + continue; + } + + lstrcpyW(fileNamePart, fd.cFileName); + + hIn = CreateFileW(srcpath, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (hIn == INVALID_HANDLE_VALUE) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), srcpath); + + FindClose(hff); + + /* Invalid directory path; Return immediately */ + if (wcschr(thisArg, L'\\')) { + return errorlevel = ERROR_INVALID_FUNCTION; + } + + continue; + } + + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", srcpath); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + + CloseHandle (hIn); + + foundOnlyDirectories = TRUE; + } while (FindNextFileW(hff, &fd) != 0); + + FindClose (hff); + + if (foundOnlyDirectories) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + + errorlevel = ERROR_INVALID_FUNCTION; + } } + else + { + hIn = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); - WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + if (hIn == INVALID_HANDLE_VALUE) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + + /* Invalid directory path; Return immediately */ + if (wcschr(thisArg, L'\\')) { + return errorlevel = ERROR_INVALID_FUNCTION; + } - CloseHandle (hIn); + continue; + } + + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + + CloseHandle (hIn); + } } return errorlevel = return_code; diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 0738a87f6b8..77ca71c8778 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -68,17 +68,17 @@ FAILURE 1 SUCCESS 0 FAILURE 1 --- -@todo_wine@a@space@ -@todo_wine@b@space@ +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +FAILURE 1 FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 79defed5cdb..dc6420e1432 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -587,17 +587,17 @@ FAILURE 1 SUCCESS 0 FAILURE 1 --- -@todo_wine@a@space@ -@todo_wine@b@space@ +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +FAILURE 1 FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
Some notes on the added test cases:
call :setError 666 & (type file\* idontexistatall.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
call :setError 666 & (type idontexistatall.txt file\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
Different behaviour is shown in Windows for paths with directories that don't exist, as tested by the `type i\dont\exist\at\all.txt file*` and similar, and paths without directories, that don't exist.
cd ..
del foo\\file\*
call :setError 666 & (type foo\\\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
Check for failure if upon wilcard expansion only directories are yielded. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143513
eric pouech (@epo) commented about programs/cmd/builtins.c:
+ WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + continue; + } + + writeHeaders = TRUE; + + do { + WCHAR srcpath[MAX_PATH]; + + if(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + WINE_TRACE("Skipping directories\n"); + continue; + } + + foundOnlyDirectories = FALSE; + lstrcpyW(srcpath, thisArg); this has to be moved after the checks about buffer length (thisArg can be up to MAXSTRING)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143625
eric pouech (@epo) commented about programs/cmd/builtins.c:
+ continue; + } + + lstrcpyW(fileNamePart, fd.cFileName); + + hIn = CreateFileW(srcpath, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (hIn == INVALID_HANDLE_VALUE) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), srcpath); + + FindClose(hff); + + /* Invalid directory path; Return immediately */ I'm not sure this is fully correct
from your explanations, you want to distinguish from directory that doesn't exist from error when opening the file (in an existing directory) testing for '\\' doesn't seem fully correct could you discriminate based on GetLastError() value? it may return ERROR_PATH_NOT_FOUND when directory doesn't exist (and some other errors FILE_NOT_FOUND is file doesn't exist whilst directory exists, and other errors (access denied...)) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143626
eric pouech (@epo) commented about programs/cmd/builtins.c:
+ } + + continue; + } + + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", srcpath); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + + CloseHandle (hIn); + + foundOnlyDirectories = TRUE; is this needed (doesn't make sense to me)?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143627
eric pouech (@epo) commented about programs/cmd/builtins.c:
+ errorlevel = ERROR_INVALID_FUNCTION; + } } + else + { + hIn = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL);
- WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + if (hIn == INVALID_HANDLE_VALUE) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + + /* Invalid directory path; Return immediately */ + if (wcschr(thisArg, L'\\')) { same remark as above...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143628
On Fri Jun 19 13:25:56 2026 +0000, eric pouech wrote:
is this needed (doesn't make sense to me)? True, it isn't needed. Even if one file is found that is not a directory then `foundOnlyDirectories` is `TRUE` from that point onwards.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143634
thisArg can be up to MAXSTRING
Wasn't aware of this. I've moved the copying of thisArg to srcpath after the checking. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_143635
participants (3)
-
Barath Kannan -
Barath Kannan (＠barath_kannan)
-
eric pouech (＠epo)