[PATCH v2 0/1] MR6997: shell32: Remove trailing spaces from lpFile in SHELL_execute.

[Louis Lenders (＠xe)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56940

Attempt to fix the regression caused by 0bad544aab9e2c9ee93bbabac0386e02c58a39c0.
Unfortunately the author of that patch doesn't seem to be active anymore, so here's an attempt, with the remark that I don't know why it worked before that patch, and I'm not sure sure if this is the correct way to fix this.

The only thing I know for sure is that on windows one can start an app via ShellExecuteW with a trailing space, and in wine this doesn't work anymore, and it did work before that commit.

--
  v2: shell32: Remove trailing spaces from lpFile in SHELL_execute.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6997

[PATCH v2 1/1] shell32: Remove trailing spaces from lpFile in SHELL_execute.

[Louis Lenders]

From: Louis Lenders <xerox.xerox2000x(a)gmail.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56940

Attempt to fix the regression caused by 0bad544aab9e2c9ee93bbabac0386e02c58a39c0.
Unfortunately the author of that patch doesn't seem to be active anymore, so here's an attempt, with the remark that I don't know why it worked before that patch, and I'm not sure sure if this is the correct way to fix this.

The only thing I know for sure is that on windows one can start an app via ShellExecuteW with a trailing space, and in wine this doesn't work anymore, and it did work before that commit.
---
 dlls/shell32/shlexec.c       | 10 +++++++++-
 dlls/shell32/tests/shlexec.c |  9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c
index 828c7168a34..45f840f2d2d 100644
--- a/dlls/shell32/shlexec.c
+++ b/dlls/shell32/shlexec.c
@@ -1670,10 +1670,18 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc )
             wszApplicationName[len-2] = '\0';
         TRACE("wszApplicationName=%s\n",debugstr_w(wszApplicationName));
     } else {
-        DWORD l = lstrlenW(sei_tmp.lpFile)+1;
+        DWORD l;
+
+        /* remove trailing spaces */
+        WCHAR *buf = wcsdup(sei->lpFile), *end = buf + wcslen( buf ) - 1;
+        while (end >= buf && *end == ' ') *end-- = 0;
+        sei_tmp.lpFile = buf;
+
+        l = lstrlenW(sei_tmp.lpFile)+1;
         if(l > dwApplicationNameLen) dwApplicationNameLen = l+1;
         wszApplicationName = malloc(dwApplicationNameLen * sizeof(WCHAR));
         memcpy(wszApplicationName, sei_tmp.lpFile, l*sizeof(WCHAR));
+        free(buf);
     }
 
     wszParameters = parametersBuffer;
diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
index bd32ece8705..9579e18a531 100644
--- a/dlls/shell32/tests/shlexec.c
+++ b/dlls/shell32/tests/shlexec.c
@@ -1166,6 +1166,15 @@ static void test_lpFile_parsed(void)
                         NULL, "\"%TMPDIR%\\simple.shlexec\"", NULL, NULL, NULL);
     okShell(rc > 32 || broken(rc == SE_ERR_FNF) /* Win95/NT4 */,
             "failed: rc=%Iu\n", rc);
+    /* test lpfile + trailing space */
+    rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI,
+                        NULL, "%TMPDIR%\\simple.shlexec ", NULL, NULL, NULL);
+    okShell(rc > 32, "failed: rc=%Iu\n", rc);
+    /* test lpfile + leading space */
+    rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI,
+                        NULL, " %TMPDIR%\\simple.shlexec", NULL, NULL, NULL);
+    okShell(rc == SE_ERR_FNF, "failed: rc=%Iu\n", rc);
+
 }
 
 typedef struct
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6997

[PATCH v2 1/1] shell32: Remove trailing spaces from lpFile in SHELL_execute.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=150341

Your paranoid android.


=== w10pro64_ar (64 bit report) ===

shell32:
shlexec.c:1730: Test failed: ShellExecute(verb="NoQuotes", file="C:\Users\winetest\AppData\Local\Temp\wtShlexecDir\test file.shlexec") WaitForSingleObject returned 258
shlexec.c:1777: Test failed: ShellExecute(file=""C:\Users\winetest\AppData\Local\Temp\wtShlexecDir\test file.shlexec"") WaitForSingleObject returned 258
shlexec.c:1780: Test failed: ShellExecute(file=""C:\Users\winetest\AppData\Local\Temp\wtShlexecDir\test file.shlexec"") argvA3 expected 'Open', but key not found or empty
shlexec.c:1782: Test failed: ShellExecute(file=""C:\Users\winetest\AppData\Local\Temp\wtShlexecDir\test file.shlexec"") argvA4 expected 'C:\Users\winetest\AppData\Local\Temp\wtShlexecDir\test file.shlexec', but key not found or empty

[Louis Lenders (＠xe)]

Hi, the test for leading space is also in the commit, and there was already a test for outer quotes or wasn't that not what you meant:

    /* quoted lpFile does not work on real win95 and nt4 */
    rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI,
                        NULL, "\"%TMPDIR%\\simple.shlexec\"", NULL, NULL, NULL);
    okShell(rc > 32 || broken(rc == SE_ERR_FNF) /* Win95/NT4 */,

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6997#note_90281

[PATCH v2 0/1] MR6997: shell32: Remove trailing spaces from lpFile in SHELL_execute. - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6997

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/shell32/shlexec.c:
> wszApplicationName[len-2] = '\0';
>          TRACE("wszApplicationName=%s\n",debugstr_w(wszApplicationName));
>      } else {
> -        DWORD l = lstrlenW(sei_tmp.lpFile)+1;
> +        DWORD l;
> +
> +        /* remove trailing spaces */
> +        WCHAR *buf = wcsdup(sei->lpFile), *end = buf + wcslen( buf ) - 1;
> +        while (end >= buf && *end == ' ') *end-- = 0;
> +        sei_tmp.lpFile = buf;
> +
> +        l = lstrlenW(sei_tmp.lpFile)+1;
>          if(l > dwApplicationNameLen) dwApplicationNameLen = l+1;
>          wszApplicationName = malloc(dwApplicationNameLen * sizeof(WCHAR));
>          memcpy(wszApplicationName, sei_tmp.lpFile, l*sizeof(WCHAR));
> +        free(buf);

Actually I meant that you can avoid the extra copy. Like this:
`
    } else {
        /* remove trailing spaces */
        WCHAR *buf = wcsdup(sei->lpFile), *end = buf + wcslen( buf ) - 1;
        while (end >= buf && *end == ' ') *end-- = 0;

        len = lstrlenW( buf ) + 1;
        if (len > dwApplicationNameLen) dwApplicationNameLen = len + 1;
        wszApplicationName = buf;
    }
`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6997#note_90326