[PATCH v3 0/2] MR10008: start: Fix arg parsing for quoted paths ending with backslash.
When using the `start` command with a `/d` switch specifying a quoted path that ends with a backslash (e.g., `start /d "c:\path\" app.exe`), subsequent arguments were incorrectly concatenated into the path string due to parsing ambiguity. The error I encountered was with `start /d "c:\temp\" test.exe`, which was parsed into the arguments `/d` and `"c:\temp\" test.exe`. The path and the executable were merged into a single argument as a new path, preventing the child process from launching. In my first patch, I attempted to identify and split this combined argument, but this caused test failures in `kernel32`. I then retested on Windows and found that Windows also parses it as `/d` and `"c:\temp\" test.exe`—Wine's parsing is consistent with Windows. However, `start` works correctly on Windows because it is a built-in command, whereas in Wine, `start` is an external executable, leading to different behavior. Signed-off-by: YeshunYe yeyeshun@uniontech.com -- v3: start: Re-parsing the command line. https://gitlab.winehq.org/wine/wine/-/merge_requests/10008
From: YeshunYe <yeyeshun@uniontech.com> Added a test case to verify the scenario where the path following `/d` ends with a backslash (`\`). Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- programs/cmd/tests/test_builtins.bat | 1 + programs/cmd/tests/test_builtins.bat.exp | 1 + programs/cmd/tests/test_builtins.cmd | 1 + programs/cmd/tests/test_builtins.cmd.exp | 1 + 4 files changed, 4 insertions(+) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 4c3d0e7107e..19cf4f202aa 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -96,6 +96,7 @@ mkdir foo & cd foo set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) +call :setError 666 & (start /B /WAIT /d "%FOO_PATH%\" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 36e14d5be91..20dda0c9ca7 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -63,6 +63,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +@todo_wine@0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index f65a301d914..91af319b686 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -679,6 +679,7 @@ mkdir foo & cd foo set "FOO_PATH=%cd%" > NUL cd .. call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) +call :setError 666 & (start /B /WAIT /d "%FOO_PATH%\" cmd /s /c "if /I \"%%cd%%\"==\"%FOO_PATH%\" (exit 0) else (exit 1)" >nul &&echo !errorlevel!) rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f6d9bdb494..f5358709f83 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -582,6 +582,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 +@todo_wine@0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10008
From: YeshunYe <yeyeshun@uniontech.com> When using the `start` command with a `/d` switch specifying a quoted path that ends with a backslash (e.g., `start /d "c:\path\" app.exe`), subsequent arguments were incorrectly concatenated into the path string due to parsing ambiguity. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- programs/cmd/tests/test_builtins.bat.exp | 2 +- programs/cmd/tests/test_builtins.cmd.exp | 2 +- programs/start/start.c | 175 +++++++++++++++++++++++ 3 files changed, 177 insertions(+), 2 deletions(-) diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 20dda0c9ca7..383c237b684 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -63,7 +63,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 -@todo_wine@0 +0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index f5358709f83..c88dee42c16 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -582,7 +582,7 @@ foo@space@ SUCCESS 1024 @todo_wine@SUCCESS 666 0 -@todo_wine@0 +0 --- success/failure for TYPE command FAILURE 1 SUCCESS 0 diff --git a/programs/start/start.c b/programs/start/start.c index 38aafe870a9..d85b2381fed 100644 --- a/programs/start/start.c +++ b/programs/start/start.c @@ -561,10 +561,185 @@ static void parse_command_line( int argc, WCHAR *argv[] ) } } +static WCHAR *make_argv_pretty(const WCHAR *src) +{ + WCHAR *result = wcsdup(src); + WCHAR *dst = result; + int in_quotes = 0, bcount = 0; + + while (*src) + { + if ((*src == ' ' || *src == '\t') && !in_quotes) + { + /* skip the remaining spaces */ + while (*src == ' ' || *src == '\t') src++; + if (!*src) break; + /* close the argument and copy it */ + *dst++ = 0; + break; + } + else if (*src == '\\') + { + *dst++ = *src++; + bcount++; + } + else if (*src == '"') + { + if ((bcount & 1) == 0) + { + /* Preceded by an even number of '\', this is half that + * number of '\', plus a '"' which we discard. + */ + dst -= bcount / 2; + src++; + if (in_quotes && *src == '"') *dst++ = *src++; + else in_quotes = !in_quotes; + } + else + { + /* Preceded by an odd number of '\', this is half that + * number of '\' followed by a '"' + */ + dst -= bcount / 2 + 1; + *dst++ = *src++; + } + bcount = 0; + } + else /* a regular character */ + { + *dst++ = *src++; + bcount = 0; + } + } + *dst = 0; + return result; +} + +/******************************************************************* + * WCMD_parameter_with_delims + * + * Extracts a delimited parameter from an input string, providing + * the delimiters characters to use + * + * PARAMS + * s [I] input string, non NULL + * n [I] # of the parameter to return, counted from 0 + * start [O] Optional. Pointer to the first char of param n in s + * raw [I] TRUE to return the parameter in raw format (quotes maintained) + * FALSE to return the parameter with quotes stripped (including internal ones) + * wholecmdline [I] TRUE to indicate this routine is being used to parse the + * command line, and special logic for arg0->1 transition + * needs to be applied. + * delims[I] The delimiter characters to use + * + * RETURNS + * Success: The nth delimited parameter found in s + * if start != NULL, *start points to the start of the param (quotes maintained) + * Failure: An empty string if the param is not found. + * *start == NULL + * + * NOTES + * Return value is stored in static storage (i.e. overwritten after each call). + * By default, the parameter is returned with quotes removed, ready for use with + * other API calls, e.g. c:\"a b"\c is returned as c:\a b\c. However, some commands + * need to preserve the exact syntax (echo, for, etc) hence the raw option. + */ +WCHAR *WCMD_parameter_with_delims (WCHAR *s, int n, WCHAR **start, + BOOL raw, BOOL wholecmdline, const WCHAR *delims) +{ + int curParamNb = 0; + static WCHAR param[MAXSTRING]; + WCHAR *p = s, *begin; + + if (start != NULL) *start = NULL; + param[0] = '\0'; + + while (TRUE) { + + /* Absorb repeated word delimiters until we get to the next token (or the end!) */ + while (*p && (wcschr(delims, *p) != NULL)) + p++; + if (*p == '\0') return param; + + /* If we have reached the token number we want, remember the beginning of it */ + if (start != NULL && curParamNb == n) *start = p; + + /* Return the whole word up to the next delimiter, handling quotes in the middle + of it, e.g. a"\b c\"d is a single parameter. */ + begin = p; + + /* Loop character by character, but just need to special case quotes */ + while (*p) { + /* Once we have found a delimiter, break */ + if (wcschr(delims, *p) != NULL) break; + + /* Very odd special case - Seems as if a ( acts as a delimiter which is + not swallowed but is effective only when it comes between the program + name and the parameters. Need to avoid this triggering when used + to walk parameters generally. */ + if (wholecmdline && curParamNb == 0 && *p=='(') break; + + /* If we find a quote, copy until we get the end quote */ + if (*p == '"') { + p++; + while (*p && *p != '"') p++; + } + + /* Now skip the character / quote */ + if (*p) p++; + } + + if (curParamNb == n) { + /* Return the parameter in static storage either as-is (raw) or + suitable for use with other win32 api calls (quotes stripped) */ + if (raw) { + memcpy(param, begin, (p - begin) * sizeof(WCHAR)); + param[p-begin] = '\0'; + } else { + int i=0; + while (begin < p) { + if (*begin != '"') param[i++] = *begin; + begin++; + } + param[i] = '\0'; + } + return param; + } + curParamNb++; + } +} + +/******************************************************************* + * WCMD_parameter + * + * Extracts a delimited parameter from an input string, using a + * default set of delimiter characters. For parameters, see the main + * function above. + */ +WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **start, BOOL raw, BOOL wholecmdline) +{ + return WCMD_parameter_with_delims (s, n, start, raw, wholecmdline, L" \t,=;"); +} int __cdecl wmain (int argc, WCHAR *argv[]) { DWORD binary_type; + int argno; + WCHAR *args = wcsdup(GetCommandLineW()); + + for (argno = 0; ; argno++) + { + WCHAR *arg = WCMD_parameter(args, argno, NULL, TRUE, FALSE); + if (!*arg) break; + } + argc = argno; + argv = HeapAlloc( GetProcessHeap(), 0, (argc+1) * sizeof(*argv) ); + for (argno = 0; argno < argc; argno++) + { + WCHAR *arg = WCMD_parameter(args, argno, NULL, TRUE, FALSE); + argv[argno] = make_argv_pretty(arg); + } + argv[argno] = 0; parse_command_line( argc, argv ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10008
@epo Is there anything else that needs improvement? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10008#note_130803
yes a lot \<g\> as already stated: * WCMD_parameter is bogus * the default arg parsing is slightly better, still broken (not at the same places as WCMD_parameter) * both are badly designed * and are used depending on context, age of the developer and other random parameters, * start (as internal command) is broken wrt parsing and sending arguments to external start * your MR adds another (also broken, see above) arg parsing routine for external start So I'm not in favor of adding more bogus code, but rather rationalize and fix the existing one before anything else I've started fixing WCMD_parameter (and evoluting it into a cleaner API), but it's not finished yet I'm not sure yet of the fate of start implementation... (I want to play a bit with what would it take to move it partially into cmd.exe) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10008#note_130954
On Tue Mar 3 00:39:28 2026 +0000, eric pouech wrote:
yes a lot \<g\> as already stated: * WCMD_parameter is bogus * the default arg parsing is slightly better, still broken (not at the same places as WCMD_parameter) * both are badly designed * and are used depending on context, age of the developer and other random parameters, * start (as internal command) is broken wrt parsing and sending arguments to external start * your MR adds another (also broken, see above) arg parsing routine for external start So I'm not in favor of adding more bogus code, but rather rationalize and fix the existing one before anything else I've started fixing WCMD_parameter (and evoluting it into a cleaner API), but it's not finished yet I'm not sure yet of the fate of start implementation... (I want to play a bit with what would it take to move it partially into cmd.exe) Okay, then I will pause my work and wait for your progress first, because currently I have no idea how to improve my patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10008#note_131039
participants (3)
-
eric pouech (＠epo) -
Yeshun Ye (＠yeyeshun)
-
YeshunYe