[PATCH v3 0/2] MR352: cmd: Don't loop infinitely on NUL.
-- v3: cmd/tests: Make sure for /f doesn't loop infinitely on NULs. cmd: Use CRT file I/O function inside 'for /F' handling. https://gitlab.winehq.org/wine/wine/-/merge_requests/352
From: Eric Pouech <eric.pouech(a)gmail.com> this allows: - to mimic native behavior when a stray '\0' is present in file (in 'FOR /F' execution, it's interpreted as an EOF marker) (prevents an infinite loop) - supports (with /USEBACKQ) unicode output (if BOM is present) Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53386 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- programs/cmd/builtins.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index dd3ae5b509b..82008c3ec7a 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2048,9 +2048,9 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, } /************************************************************************** - * WCMD_forf_getinputhandle + * WCMD_forf_getinput * - * Return a file handle which can be used for reading the input lines, + * Return a FILE* which can be used for reading the input lines, * either to a specific file (which may be quote delimited as we have to * read the parameters in raw mode) or to a command which we need to * execute. The command being executed runs in its own shell and stores @@ -2064,12 +2064,12 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, * * Returns a file handle which can be used to read the input lines from. */ -static HANDLE WCMD_forf_getinputhandle(BOOL usebackq, WCHAR *itemstr, BOOL iscmd) { +static FILE* WCMD_forf_getinput(BOOL usebackq, WCHAR *itemstr, BOOL iscmd) { WCHAR temp_str[MAX_PATH]; WCHAR temp_file[MAX_PATH]; WCHAR temp_cmd[MAXSTRING]; WCHAR *trimmed = NULL; - HANDLE hinput = INVALID_HANDLE_VALUE; + FILE* ret; /* Remove leading and trailing character (but there may be trailing whitespace too) */ if ((iscmd && (itemstr[0] == '`' && usebackq)) || @@ -2097,17 +2097,14 @@ static HANDLE WCMD_forf_getinputhandle(BOOL usebackq, WCHAR *itemstr, BOOL iscmd WCMD_execute (temp_cmd, temp_str, NULL, FALSE); /* Open the file, read line by line and process */ - hinput = CreateFileW(temp_file, GENERIC_READ, FILE_SHARE_READ, - NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL); - + ret = _wfopen(temp_file, L"rt,ccs=unicode"); } else { /* Open the file, read line by line and process */ WINE_TRACE("Reading input to parse from '%s'\n", wine_dbgstr_w(itemstr)); - hinput = CreateFileW(itemstr, GENERIC_READ, FILE_SHARE_READ, - NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + ret = _wfopen(itemstr, L"rt,ccs=unicode"); } heap_free(trimmed); - return hinput; + return ret; } /************************************************************************** @@ -2389,7 +2386,7 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { } else if (doFileset && ((!forf_usebackq && *itemStart != '"') || (forf_usebackq && *itemStart != '\''))) { - HANDLE input; + FILE *input; WCHAR *itemparm; WINE_TRACE("Processing for filespec from item %d '%s'\n", itemNum, @@ -2407,10 +2404,10 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { /* Use item because the file to process is just the first item in the set */ itemparm = item; } - input = WCMD_forf_getinputhandle(forf_usebackq, itemparm, (itemparm==itemStart)); + input = WCMD_forf_getinput(forf_usebackq, itemparm, (itemparm==itemStart)); /* Process the input file */ - if (input == INVALID_HANDLE_VALUE) { + if (!input) { WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), item); errorlevel = 1; @@ -2419,12 +2416,20 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { } else { /* Read line by line until end of file */ - while (WCMD_fgets(buffer, ARRAY_SIZE(buffer), input)) { + while (fgetws(buffer, ARRAY_SIZE(buffer), input)) { + size_t len = wcslen(buffer); + /* Either our buffer isn't large enough to fit a full line, or there's a stray + * '\0' in the buffer. + */ + if (!feof(input) && (len == 0 || (buffer[len - 1] != '\n' && buffer[len - 1] != '\r'))) + break; + while (len && (buffer[len - 1] == '\n' || buffer[len - 1] == '\r')) + buffer[--len] = L'\0'; WCMD_parse_line(cmdStart, firstCmd, &cmdEnd, variable[1], buffer, &doExecuted, &forf_skip, forf_eol, forf_delims, forf_tokens); buffer[0] = 0; } - CloseHandle (input); + fclose (input); } /* When we have processed the item as a whole command, abort future set processing */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/352
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- programs/cmd/tests/batch.c | 22 ++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd | 3 +++ programs/cmd/tests/test_builtins.cmd.exp | 2 ++ 3 files changed, 27 insertions(+) diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 3846693ef2e..ade6587be81 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -455,6 +455,24 @@ static int cmd_available(void) return FALSE; } +void create_nul_test_file(void) +{ + HANDLE file; + DWORD size; + BOOL bres; + char contents[] = "a b c\nd e\0f\ng h i"; + + file = CreateFileA("nul_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; @@ -479,9 +497,13 @@ START_TEST(batch) } shortpath_len = GetShortPathNameA(path, shortpath, ARRAY_SIZE(shortpath)); + create_nul_test_file(); + argc = winetest_get_mainargs(&argv); if(argc > 2) run_from_file(argv[2]); else EnumResourceNamesA(NULL, "TESTCMD", test_enum_proc, 0); + + DeleteFileA("nul_test_file"); } diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 3f410e55166..3829be50f1a 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1957,6 +1957,9 @@ FOR /F "delims=. tokens=1*" %%A IN (testfile) DO @echo 5:%%A,%%B FOR /F "delims=. tokens=2*" %%A IN (testfile) DO @echo 6:%%A,%%B FOR /F "delims=. tokens=3*" %%A IN (testfile) DO @echo 7:%%A,%%B del testfile +rem file contains NUL, created by the .exe +for /f %%A in (nul_test_file) DO echo %%A +for /f "tokens=*" %%A in (nul_test_file) DO echo %%A echo ------------ Testing del ------------ echo abc > file diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 8b6e0914112..437ded18000 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1315,6 +1315,8 @@ h=%h i=b j=c k= l= m=%m n=%n o=%o(a)or_broken@h=%h i=b j=c k= l= m= n=%n o=%o 4:3.14,%B 5:3,14 6:14, +a +a b c ------------ Testing del ------------ deleting 'file' errorlevel is 0, good -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/352
participants (3)
-
Arek Hiler (＠ivyl) -
Arkadiusz Hiler
-
Eric Pouech