[PATCH v2 0/2] MR8874: cmd: Fix TYPE behavior.
These changes fix two TYPE issues: - TYPE output should terminate at a Ctrl-Z EOF character, if output is not redirected. - TYPE redirection should function as a rudimentary file copy (bug #56381). Tests run: TYPE text_file_containing_embedded_ctrl_z.txt (Output terminates at Ctrl-Z character.) TYPE c:\windows\winhelp.exe >foo (Issue raised in bug #56381. Foo is identical copy of winhelp.exe and not truncated at first NUL character. Test for this case was added to the automated tests.) CHCP 65001 TYPE text_file_containing_unicode_characters.txt (Unicode characters were output to the console.) -- v2: cmd/tests: Add test for TYPE redirection. cmd: Fix TYPE behavior. https://gitlab.winehq.org/wine/wine/-/merge_requests/8874
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/builtins.c | 64 +++++++++++++++++++++++++++++++++++++---- programs/cmd/wcmdmain.c | 4 +++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index b5ad78ab9c7..7e538dea745 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -36,6 +36,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(cmd); extern int defaultColor; extern BOOL echo_mode; +extern HANDLE stdout_orig; struct env_stack *pushd_directories; const WCHAR inbuilt[][10] = { @@ -3392,12 +3393,33 @@ RETURN_CODE WCMD_title(const WCHAR *args) return NO_ERROR; } +/**************************************************************************** + * WCMD_convert_text + * + * Converts bytes to wide characters in the specified code page. + */ +static int WCMD_convert_text(UINT codepage, char *buffer, DWORD count, LPWSTR *outBuf) +{ + int numChars; + + numChars = MultiByteToWideChar(codepage, 0, buffer, count, NULL, 0); + if (numChars) { + *outBuf = xalloc(numChars * sizeof(WCHAR)); + if (*outBuf) { + numChars = MultiByteToWideChar(codepage, 0, buffer, count, *outBuf, numChars); + } else { + numChars = 0; + } + } + + return numChars; +} + /**************************************************************************** * WCMD_type * * Copy a file to standard output. */ - RETURN_CODE WCMD_type(WCHAR *args) { RETURN_CODE return_code; @@ -3417,9 +3439,12 @@ RETURN_CODE WCMD_type(WCHAR *args) while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); - HANDLE h; - WCHAR buffer[512]; + HANDLE h, hOut; + char buffer[1024]; DWORD count; + UINT output_CP; + BOOL input_is_console, output_is_console, output_redirected; + char *eof = NULL; if (!argN) break; @@ -3434,10 +3459,39 @@ RETURN_CODE WCMD_type(WCHAR *args) if (writeHeaders) { WCMD_output_stderr(L"\n%1\n\n\n", thisArg); } - while (WCMD_ReadFile(h, buffer, ARRAY_SIZE(buffer) - 1, &count)) { + + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + output_redirected = (hOut != stdout_orig); + input_is_console = VerifyConsoleIoHandle(h); + output_is_console = VerifyConsoleIoHandle(hOut); + output_CP = GetConsoleOutputCP(); + + while (ReadFile(h, buffer, ARRAY_SIZE(buffer) - 1, &count, NULL)) { if (count == 0) break; /* ReadFile reports success on EOF! */ buffer[count] = 0; - WCMD_output_asis (buffer); + /* If input is console (i.e. TYPE con > foo) or output is not redirected + then TYPE terminates at the EOF character. */ + if (input_is_console || !output_redirected) { + eof = (char *)memchr((void *)buffer, '\x1A', count); + if (eof) { + count = (DWORD)(eof - buffer); + } + } + if (output_is_console && output_CP) { + /* Convert text for current output code page and write to the console. + We don't use WriteFile here because that writes raw bytes, not characters. */ + WCHAR *string; + count = WCMD_convert_text(output_CP, buffer, count, &string); + if (count) { + WriteConsoleW(hOut, string, count, &count, NULL); + free(string); + } + } else { + WriteFile(hOut, buffer, count, &count, NULL); + } + if (eof) { + break; + } } CloseHandle (h); } diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 26c2be2e1c3..0218c60e7ee 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -61,6 +61,7 @@ BOOL delayedsubst = FALSE; /* The current delayed substitution setting */ int defaultColor = 7; BOOL echo_mode = TRUE; +HANDLE stdout_orig; WCHAR anykey[100], version_string[100]; @@ -4302,6 +4303,9 @@ int __cdecl wmain (int argc, WCHAR *argvW[]) osv.dwOSVersionInfoSize = sizeof(osv); RtlGetVersion(&osv); + /* Remember the original STDOUT handle. */ + stdout_orig = GetStdHandle(STD_OUTPUT_HANDLE); + /* Pre initialize some messages */ lstrcpyW(anykey, WCMD_LoadMessage(WCMD_ANYKEY)); sprintf(osver, "%ld.%ld.%ld", osv.dwMajorVersion, osv.dwMinorVersion, osv.dwBuildNumber); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8874
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/tests/batch.c | 24 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd | 12 ++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 4 ++++ 3 files changed, 40 insertions(+) diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 03a05f34215..8f3d4daf22b 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -531,6 +531,28 @@ void create_nul_test_file(void) CloseHandle(file); } +void create_binary_test_file(void) +{ + HANDLE file; + DWORD size; + BOOL bres; + unsigned char contents[256]; + + for (int x=0; x<sizeof(contents); x++) { + contents[x] = x; + } + + file = CreateFileA("binary_test_file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed\n"); + if(file == INVALID_HANDLE_VALUE) + return; + + bres = WriteFile(file, contents, ARRAYSIZE(contents), &size, NULL); + ok(bres, "Could not write to file: %lu\n", GetLastError()); + CloseHandle(file); +} + START_TEST(batch) { int argc; @@ -556,6 +578,7 @@ START_TEST(batch) shortpath_len = GetShortPathNameA(path, shortpath, ARRAY_SIZE(shortpath)); create_nul_test_file(); + create_binary_test_file(); argc = winetest_get_mainargs(&argv); if(argc > 2) @@ -564,4 +587,5 @@ START_TEST(batch) EnumResourceNamesA(NULL, "TESTCMD", test_enum_proc, 0); DeleteFileA("nul_test_file"); + DeleteFileA("binary_test_file"); } diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 9f144bb4e0a..fd9f6bd6152 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1405,6 +1405,14 @@ echo ---6 type foobaw echo ---7 del foobaz foobay foobax foobaw +echo ---8 +type c:\windows\winhelp.exe >foo +call :CompareFileSizes c:\windows\winhelp.exe foo +del foo +echo ---9 +type binary_test_file >foo +call :CompareFileSizes binary_test_file foo +del foo echo ------------ Testing NUL ------------ md foobar & cd foobar @@ -3513,6 +3521,10 @@ shift if not "%1"=="" goto :CheckFileSize goto :eof +:CompareFileSizes +if "%~z1"=="%~z2" (echo passed) else (echo failed) +goto :eof + :testcopy rem ----------------------- diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 6ec30eba3a7..f9bec6813d0 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -994,6 +994,10 @@ foobay ---7 +---8 +passed +---9 +passed ------------ Testing NUL ------------ bar bar -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8874
On Wed Sep 3 00:21:53 2025 +0000, eric pouech wrote:
we'd better create our own test files... see chcp related tests in test_builtin.cmd to insert any hex character Perhaps it's something with my environment but I wasn't able to insert hex characters into the file using the lines in the chcp tests. Ended up writing literal "@\xE3@", etc. Added a function in batch.c to write a binary file, similar to the create_nul_test_file() function in that same file.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114612
On Wed Sep 3 00:22:48 2025 +0000, eric pouech wrote:
1) it's overkill to test if files exist 2) why not using only (untested): ``` if "%~z1"=="%~z2" (echo passed) else (echo failed) goto :eof ``` (and there's no need to print the filenames) Done. Previous version was based on the CheckFileSize routine just above this, which did check for file existence and displayed file names, etc.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114613
On Wed Sep 3 00:23:19 2025 +0000, eric pouech wrote:
from a global view: my first thought was that the behavior should be: - if output handle has a console code page (GetConsoleOutputCP()), then input file has to be converted to that code page (and it seems also that native cmd.exe supports some BOM from input file, can be done in a later patch). and likely treat input file as text (to handle ctrl-z). And input CP should come from BOM presence or not. - otherwise, a simple binary copy would do but, things are not that simple :-( `type foo > con:` does output the whole file (no ctrl-z handling) in binary mode to the console (still using current CP for conversion) so CP conversion (if any) comes from current output handle CP (GetConsoleOutputCP()) text vs binary mode seems to be driven by redirection or not (best solution is likely to store as global variable the output handle at cmd.exe startup and compare if current stdout handle if the initial one or not) Did you try to reuse the COPY command helpers (at least for the binary copy)? (note also that `TYPE CON: > foo `works as `COPY CON: foo`) Would like to have some tests for TYPE in various CP. But couldn't find a simple way to do it as default stdout in tests is not bound to a console, and if we force output to console we'll loose the output) (would require reading back console's screenbuffer) I believe that I made all of the changes that you asked for here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114614
eric pouech (@epo) commented about programs/cmd/builtins.c:
return NO_ERROR; }
+/**************************************************************************** + * WCMD_convert_text + * + * Converts bytes to wide characters in the specified code page. + */ +static int WCMD_convert_text(UINT codepage, char *buffer, DWORD count, LPWSTR *outBuf) +{ + int numChars; + + numChars = MultiByteToWideChar(codepage, 0, buffer, count, NULL, 0); + if (numChars) { + *outBuf = xalloc(numChars * sizeof(WCHAR)); + if (*outBuf) {
xalloc terminates program in OOM conditions so outBuf is always non NULL please use the style for new code (4 spaces indent, curly braces on next line...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114671
eric pouech (@epo) commented about programs/cmd/builtins.c:
return NO_ERROR; }
+/**************************************************************************** + * WCMD_convert_text + * + * Converts bytes to wide characters in the specified code page. + */ +static int WCMD_convert_text(UINT codepage, char *buffer, DWORD count, LPWSTR *outBuf) +{ + int numChars; + + numChars = MultiByteToWideChar(codepage, 0, buffer, count, NULL, 0);
you're doing things in reverse what's the cp 'buffer' is encoded in? it's not encoded in 'codepage' as it's the expected *output* code page this means you didn't even compare the output between native and (modified) builtin with small examples (which give different output when using different codepages) please start by doing this and submit this MR when these manual tests pass (it would be better if you we could integrate that to test harness, but that's not doable for now, so you have to do it by hand) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114672
eric pouech (@epo) commented about programs/cmd/builtins.c:
> + }
> + if (output_is_console && output_CP) {
> + /* Convert text for current output code page and write to the console.
> + We don't use WriteFile here because that writes raw bytes, not characters. */
> + WCHAR *string;
> + count = WCMD_convert_text(output_CP, buffer, count, &string);
> + if (count) {
> + WriteConsoleW(hOut, string, count, &count, NULL);
> + free(string);
> + }
> + } else {
> + WriteFile(hOut, buffer, count, &count, NULL);
> + }
> + if (eof) {
> + break;
> + }
this still looks utterly ugly to me...
* VerifyConsoleIoHandle (in native) doesn't detect all kinds of (new) conhost handles, use GetConsoleMode instead
* I'll restart commenting this code when the MR is updated according to comments above (as it shall quite evolve)
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8874#note_114673
participants (3)
-
eric pouech (＠epo) -
Joe Souza
-
Joe Souza (＠JoeS209)