From: Bernhard Übelacker Subject: [PATCH] cmd: Change parsing of the if command to avoid parsing the operators. Message-Id: <20180128135129.29161-1-bernhardu@mailbox.org> Date: Sun, 28 Jan 2018 14:51:29 +0100 https://bugs.winehq.org/show_bug.cgi?id=44338 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. Signed-off-by: Bernhard Übelacker --- programs/cmd/builtins.c | 75 +++++++++++++++++++------------- 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, 96 insertions(+), 35 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 14961d7922..519fd435da 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2765,25 +2765,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'}; @@ -2791,33 +2775,33 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList) static const WCHAR parmI[] = {'/','I','\0'}; int caseInsensitive = (strstrW(quals, parmI) != NULL); - negate = !lstrcmpiW(param1,notW); - strcpyW(condition, (negate ? param2 : param1)); + *negate = !lstrcmpiW(param1,notW); + strcpyW(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 = strtolW(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)) { - test = (GetFileAttributesW(WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE)) + *test = (GetFileAttributesW(WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE)) != INVALID_FILE_ATTRIBUTES); - 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; - strcpyW(leftOperand, WCMD_parameter(p, negate+caseInsensitive, ¶mStart, TRUE, FALSE)); + strcpyW(leftOperand, WCMD_parameter(p, (*negate)+caseInsensitive, ¶mStart, TRUE, FALSE)); if (!*leftOperand) goto syntax_err; @@ -2838,14 +2822,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 + strlenW(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 62334b17b6..6060c15fa7 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -907,6 +907,26 @@ if %elseIF% == 1 ( ) else ( echo else if seems 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 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 796550e57e..7654f20694 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -657,6 +657,10 @@ if seems not to detect /c as parameter else if seems to work else if seems to work else if seems 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 +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 d4d97a0067..4937d16f00 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -154,6 +154,8 @@ static inline BOOL ends_with_backslash( const WCHAR *path ) return path[0] && path[strlenW(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 827ddd2121..62b6c5832f 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1916,13 +1916,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, sizeof(ifCmd)/sizeof(ifCmd[0]), 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+(sizeof(ifCmd)/sizeof(*ifCmd)); + while (*p == ' ' || *p == '\t') { + p++; + if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), notW) == 0) + p += strlenW(notW); + if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), parmI) == 0) + p += strlenW(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, sizeof(ifElse)/sizeof(ifElse[0]), curPos)) { const int keyw_len = sizeof(ifElse)/sizeof(ifElse[0]) + 1; inElse = TRUE; -- 2.15.1