[PATCH v3] cmd.exe: Change parsing of the if command to avoid parsing the operators.

[Bernhard Übelacker]

Found while trying to look into #44236.
A batch script is executed containing a line like this:
  if (%1)==(p) start /W  " "  "%SFDIR%WSFplot" wr2300.t35 3

This returns an error like this:
  Syntax error
  Can't recognize 'p' as an internal or external command, or batch script.

It looks like native does handle the brackets differently when contained
inside the condition part of the if command.
Therefore this patch tries to skip the parsing in WCMD_ReadAndParseLine
of that part. Therefore it moves most of the content of WCMD_if into a
helper and calls that to estimate how long the condition part is.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44338
Signed-off-by: Bernhard Übelacker <bernhardu(a)mailbox.org>
---
History:
v0:
  Some comments in bug tracker by Vijay Kamuju.

v1:
  https://www.winehq.org/pipermail/wine-devel/2018-January/121764.html
  (Received no comments.)

v2:
* Rebased to wine 3.13.
* Removed --- in commit message making patchwatcher ignoring Signed-off-by line.

v3:
* Rebased to wine 4.14.
---
 programs/cmd/builtins.c                  | 79 ++++++++++++++----------
 programs/cmd/tests/test_builtins.cmd     | 20 ++++++
 programs/cmd/tests/test_builtins.cmd.exp |  4 ++
 programs/cmd/wcmd.h                      |  2 +
 programs/cmd/wcmdmain.c                  | 30 +++++++--
 5 files changed, 98 insertions(+), 37 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index cd5f744a27..a1952e8024 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -2837,25 +2837,9 @@ static int evaluate_if_comparison(const WCHAR *leftOperand, const WCHAR *operato
     return -1;
 }
 
-/****************************************************************************
- * WCMD_if
- *
- * Batch file conditional.
- *
- * On entry, cmdlist will point to command containing the IF, and optionally
- *   the first command to execute (if brackets not found)
- *   If &&'s were found, this may be followed by a record flagged as isAmpersand
- *   If ('s were found, execute all within that bracket
- *   Command may optionally be followed by an ELSE - need to skip instructions
- *   in the else using the same logic
- *
- * FIXME: Much more syntax checking needed!
- */
-void WCMD_if (WCHAR *p, CMD_LIST **cmdList)
+int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
 {
-  int negate; /* Negate condition */
-  int test;   /* Condition evaluation result */
-  WCHAR condition[MAX_PATH], *command;
+  WCHAR condition[MAX_PATH];
   static const WCHAR notW[]    = {'n','o','t','\0'};
   static const WCHAR errlvlW[] = {'e','r','r','o','r','l','e','v','e','l','\0'};
   static const WCHAR existW[]  = {'e','x','i','s','t','\0'};
@@ -2863,43 +2847,43 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList)
   static const WCHAR parmI[]   = {'/','I','\0'};
   int caseInsensitive = (wcsstr(quals, parmI) != NULL);
 
-  negate = !lstrcmpiW(param1,notW);
-  lstrcpyW(condition, (negate ? param2 : param1));
+  *negate = !lstrcmpiW(param1,notW);
+  lstrcpyW(condition, (*negate ? param2 : param1));
   WINE_TRACE("Condition: %s\n", wine_dbgstr_w(condition));
 
   if (!lstrcmpiW (condition, errlvlW)) {
-    WCHAR *param = WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE);
+    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
     WCHAR *endptr;
     long int param_int = wcstol(param, &endptr, 10);
     if (*endptr) goto syntax_err;
-    test = ((long int)errorlevel >= param_int);
-    WCMD_parameter(p, 2+negate, &command, FALSE, FALSE);
+    *test = ((long int)errorlevel >= param_int);
+    WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else if (!lstrcmpiW (condition, existW)) {
     WIN32_FIND_DATAW fd;
     HANDLE hff;
-    WCHAR *param = WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE);
+    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
     int    len = lstrlenW(param);
 
     /* FindFirstFile does not like a directory path ending in '\', append a '.' */
     if (len && param[len-1] == '\\') lstrcatW(param, dotW);
 
     hff = FindFirstFileW(param, &fd);
-    test = (hff != INVALID_HANDLE_VALUE );
-    if (test) FindClose(hff);
+    *test = (hff != INVALID_HANDLE_VALUE );
+    if (*test) FindClose(hff);
 
-    WCMD_parameter(p, 2+negate, &command, FALSE, FALSE);
+    WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else if (!lstrcmpiW (condition, defdW)) {
-    test = (GetEnvironmentVariableW(WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE),
+    *test = (GetEnvironmentVariableW(WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE),
                                     NULL, 0) > 0);
-    WCMD_parameter(p, 2+negate, &command, FALSE, FALSE);
+    WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else { /* comparison operation */
     WCHAR leftOperand[MAXSTRING], rightOperand[MAXSTRING], operator[MAXSTRING];
     WCHAR *paramStart;
 
-    lstrcpyW(leftOperand, WCMD_parameter(p, negate+caseInsensitive, ¶mStart, TRUE, FALSE));
+    lstrcpyW(leftOperand, WCMD_parameter(p, (*negate)+caseInsensitive, ¶mStart, TRUE, FALSE));
     if (!*leftOperand)
       goto syntax_err;
 
@@ -2920,14 +2904,43 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList)
     if (!*rightOperand)
       goto syntax_err;
 
-    test = evaluate_if_comparison(leftOperand, operator, rightOperand, caseInsensitive);
-    if (test == -1)
+    *test = evaluate_if_comparison(leftOperand, operator, rightOperand, caseInsensitive);
+    if (*test == -1)
       goto syntax_err;
 
     p = paramStart + lstrlenW(rightOperand);
-    WCMD_parameter(p, 0, &command, FALSE, FALSE);
+    WCMD_parameter(p, 0, command, FALSE, FALSE);
   }
 
+  return 1;
+
+syntax_err:
+  return -1;
+}
+
+/****************************************************************************
+ * WCMD_if
+ *
+ * Batch file conditional.
+ *
+ * On entry, cmdlist will point to command containing the IF, and optionally
+ *   the first command to execute (if brackets not found)
+ *   If &&'s were found, this may be followed by a record flagged as isAmpersand
+ *   If ('s were found, execute all within that bracket
+ *   Command may optionally be followed by an ELSE - need to skip instructions
+ *   in the else using the same logic
+ *
+ * FIXME: Much more syntax checking needed!
+ */
+void WCMD_if (WCHAR *p, CMD_LIST **cmdList)
+{
+  int negate; /* Negate condition */
+  int test;   /* Condition evaluation result */
+  WCHAR *command;
+
+  if (evaluate_if_condition(p, &command, &test, &negate) == -1)
+      goto syntax_err;
+
   /* Process rest of IF statement which is on the same line
      Note: This may process all or some of the cmdList (eg a GOTO) */
   WCMD_part_execute(cmdList, command, TRUE, (test != negate));
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index a992cda480..4bf37a35bb 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -993,6 +993,26 @@ if "x" == "a" (
   if "y" == "b" echo broken2
   echo expected post-embedded if
 )
+if ()==() (
+  echo comparison operators surrounded by brackets seem to work
+) else (
+  echo comparison operators surrounded by brackets seem to be broken
+)
+if 1(==1( (
+  echo comparison operators surrounded by brackets seem to work
+) else (
+  echo comparison operators surrounded by brackets seem to be broken
+)
+if )==) (
+  echo comparison operators surrounded by brackets seem to work
+) else (
+  echo comparison operators surrounded by brackets seem to be broken
+)
+if /i not (a)==(b) (
+  echo comparison operators surrounded by brackets seem to work
+) else (
+  echo comparison operators surrounded by brackets seem to be broken
+)
 echo --- case sensitivity with and without /i option
 if bar==BAR echo if does not default to case sensitivity
 if not bar==BAR echo if seems to default to case sensitivity
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 488c8e6fd8..5bcd29b917 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -695,6 +695,10 @@ else if seems to work
 else if seems to work
 expected1
 expected post-embedded if
+comparison operators surrounded by brackets seem to work
+comparison operators surrounded by brackets seem to work
+comparison operators surrounded by brackets seem to work
+comparison operators surrounded by brackets seem to work
 --- case sensitivity with and without /i option
 if seems to default to case sensitivity
 if /i seems to work
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index 12c878c3c9..3e99a0d203 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -150,6 +150,8 @@ static inline BOOL ends_with_backslash( const WCHAR *path )
     return path[0] && path[lstrlenW(path) - 1] == '\\';
 }
 
+int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate);
+
 /* Data structure to hold context when executing batch files */
 
 typedef struct _BATCH_CONTEXT {
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index d7c0ce13fe..8a68c0c2df 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -1943,13 +1943,35 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE
 
         /* If command starts with 'if ' or 'else ', handle ('s mid line. We should ensure this
            is only true in the command portion of the IF statement, but this
-           should suffice for now
-            FIXME: Silly syntax like "if 1(==1( (
-                                        echo they equal
-                                      )" will be parsed wrong */
+           should suffice for now.
+           To be able to handle ('s in the condition part take as much as evaluate_if_condition
+           would take and skip parsing it here. */
         } else if (WCMD_keyword_ws_found(ifCmd, ARRAY_SIZE(ifCmd), curPos)) {
+          static const WCHAR parmI[]   = {'/','I','\0'};
+          static const WCHAR notW[]    = {'n','o','t','\0'};
+          int negate; /* Negate condition */
+          int test;   /* Condition evaluation result */
+          WCHAR *p, *command;
+
           inIf = TRUE;
 
+          p = curPos+(ARRAY_SIZE(ifCmd));
+          while (*p == ' ' || *p == '\t') {
+            p++;
+            if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), notW) == 0)
+              p += lstrlenW(notW);
+            if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), parmI) == 0)
+              p += lstrlenW(parmI);
+          }
+
+          if (evaluate_if_condition(p, &command, &test, &negate) != -1)
+          {
+              int if_condition_len = command - curPos;
+              memcpy(&curCopyTo[*curLen], curPos, if_condition_len*sizeof(WCHAR));
+              (*curLen)+=if_condition_len;
+              curPos+=if_condition_len;
+          }
+
         } else if (WCMD_keyword_ws_found(ifElse, ARRAY_SIZE(ifElse), curPos)) {
           const int keyw_len = ARRAY_SIZE(ifElse) + 1;
           inElse = TRUE;
-- 
2.20.1