[PATCH v3 0/2] MR352: cmd: Don't loop infinitely on NUL.

[Arek Hiler (＠ivyl)]

--
  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

[PATCH v3 1/2] cmd: Use CRT file I/O function inside 'for /F' handling.

[Eric Pouech]

From: Eric Pouech

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 
Signed-off-by: Arkadiusz Hiler 
---
 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 */

[PATCH v3 2/2] cmd/tests: Make sure for /f doesn't loop infinitely on NULs.

[Arkadiusz Hiler]

From: Arkadiusz Hiler

Signed-off-by: Arkadiusz Hiler 
---
 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@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