[PATCH v2 0/4] MR5400: shell32: Tying up some loose ends
Follow up to !5342. This adds the last missing piece for fixing how `ShellExecute` finds files, and restore the ability of running native unix programs with `ShellExecute` -- v2: shell32: Accept ".so" as a valid program extension. https://gitlab.winehq.org/wine/wine/-/merge_requests/5400
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/shell32/shlexec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index f09b6d4daf3..804aa38a904 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -637,7 +637,7 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, lstrcpyW(xlpFile, lpFile); if (PathResolveAW(xlpFile, (const void **)search_paths, PRF_TRYPROGRAMEXTENSIONS | PRF_VERIFYEXISTS)) { - TRACE("SearchPathW returned non-zero\n"); + TRACE("PathResolveAW returned non-zero\n"); lpFile = xlpFile; /* The file was found in lpPath or one of the directories in the system-wide search path */ } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5400
From: Yuxuan Shui <yshui(a)codeweavers.com> So that the path returned by SHELL_FindExecutable would be fully qualified, otherwise CreateProcess will do its own path resolution which is not what we want. --- dlls/shell32/shlexec.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index 804aa38a904..9704f9d8f06 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -1797,10 +1797,10 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) if (*sei_tmp.lpDirectory) { + LPWSTR buf; len = ExpandEnvironmentStringsW(sei_tmp.lpDirectory, NULL, 0); if (len > 0) { - LPWSTR buf; len++; buf = malloc(len * sizeof(WCHAR)); ExpandEnvironmentStringsW(sei_tmp.lpDirectory, buf, len); @@ -1809,6 +1809,18 @@ static BOOL SHELL_execute( LPSHELLEXECUTEINFOW sei, SHELL_ExecuteW32 execfunc ) wszDir = buf; sei_tmp.lpDirectory = wszDir; } + + len = GetFullPathNameW(sei_tmp.lpDirectory, 0, NULL, NULL); + if (len > 0) + { + len++; + buf = malloc(len * sizeof(WCHAR)); + GetFullPathNameW(sei_tmp.lpDirectory, len, buf, NULL); + if (wszDir != dirBuffer) + free(wszDir); + wszDir = buf; + sei_tmp.lpDirectory = wszDir; + } } /* Else, try to execute the filename */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5400
From: Yuxuan Shui <yshui(a)codeweavers.com> When ShellExecute is called with the expressed intention to run a file without an extension (i.e. by putting a trailing dot in the file name), honor that and try running the file as a program. --- dlls/shell32/shlexec.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index 9704f9d8f06..b437f98e6e6 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -601,6 +601,7 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, WCHAR xlpFile[256]; /* result of SearchPath */ DWORD attribs; /* file attributes */ WCHAR curdir[MAX_PATH]; + BOOL has_trailing_dot; const WCHAR *search_paths[3] = {0}; TRACE("%s\n", debugstr_w(lpFile)); @@ -626,6 +627,7 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, return 33; } + has_trailing_dot = lpFile[lstrlenW(lpFile) - 1] == '.'; if (lpPath && *lpPath) { search_paths[0] = lpPath; @@ -663,10 +665,18 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, /* .\FILE.EXE :( */ TRACE("xlpFile=%s,extension=%s\n", debugstr_w(xlpFile), debugstr_w(extension)); - if (extension == NULL || extension[1]==0) + if (extension == NULL) { - WARN("Returning SE_ERR_NOASSOC\n"); - return SE_ERR_NOASSOC; + if (!has_trailing_dot) + { + WARN("Returning SE_ERR_NOASSOC\n"); + return SE_ERR_NOASSOC; + } + + /* Special wine extension to support running a native unix program, + * if the file name has an trailing dot. */ + lstrcpyW(lpResult, xlpFile); + return 33; } /* Three places to check: */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5400
From: Yuxuan Shui <yshui(a)codeweavers.com> So ShellExecute can run ".exe.so" winelib programs. --- dlls/shell32/shlexec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index b437f98e6e6..51b390aed6d 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -690,7 +690,7 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, /* See if it's a program - if GetProfileString fails, we skip this * section. Actually, if GetProfileString fails, we've probably * got a lot more to worry about than running a program... */ - if (GetProfileStringW(L"windows", L"programs", L"exe pif bat cmd com", wBuffer, ARRAY_SIZE(wBuffer)) > 0) + if (GetProfileStringW(L"windows", L"programs", L"exe pif bat cmd com so", wBuffer, ARRAY_SIZE(wBuffer)) > 0) { CharLowerW(wBuffer); tok = wBuffer; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5400
Some applications ship excutables with custom extensions like .bin, will ShellExecute() work in that case? Adding support for a random extension like .so seems to be a fragile solution. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66503
Yes, for Unix integration it's necessary to support any extension, or no extension at all, even if that's not fully compatible with Windows. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66506
On Sat Mar 30 19:43:16 2024 +0000, Alexandre Julliard wrote:
Yes, for Unix integration it's necessary to support any extension, or no extension at all, even if that's not fully compatible with Windows. OK... All options feel bad here.
How does this interact with extension associations? Like if `.bin` has association and is an Unix program, what happens? Do we find out if something is an Unix program by looking at unix file permissions? Is there an existing wine internal API for that? I think previously we just try to run it and see if it works... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66532
On Sat Mar 30 19:43:16 2024 +0000, Yuxuan Shui wrote:
OK... All options feel bad here. How does this interact with extension associations? Like if `.bin` has association and is an Unix program, what happens? Do we find out if something is an Unix program by looking at unix file permissions? Is there an existing wine internal API for that? I think previously we just try to run it and see if it works... Windows configuration such as associations should take priority. Essentially the normal Windows processing should take place first, but if we find that there's nothing we can do at the Windows level, instead of giving up we should simply try to run the file through CreateProcess and see what happens.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66533
On Sat Mar 30 19:56:02 2024 +0000, Alexandre Julliard wrote:
Windows configuration such as associations should take priority. Essentially the normal Windows processing should take place first, but if we find that there's nothing we can do at the Windows level, instead of giving up we should simply try to run the file through CreateProcess and see what happens. What worries me is that this means `FindExecutable` will be inconsistent with `ShellExecute`, because `FindExecutable` can't try running the program. That's why I think instead we should check the file permissions.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66687
On Tue Apr 2 16:07:18 2024 +0000, Yuxuan Shui wrote:
What worries me is that this means `FindExecutable` will be inconsistent with `ShellExecute`, because `FindExecutable` can't try running the program. That's why I think instead we should check the file permissions. It's OK for `FindExecutable` to return failure in that case. If it turns out to be a problem in practice we can look for ways to address it, but returning Unix binaries to the app is not necessarily going to give better results.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66688
On Tue Apr 2 16:12:51 2024 +0000, Alexandre Julliard wrote:
It's OK for `FindExecutable` to return failure in that case. If it turns out to be a problem in practice we can look for ways to address it, but returning Unix binaries to the app is not necessarily going to give better results. I found one more corner case. Say we try to run `file.noassoc`, assuming this file exists and have no Windows associations. We should try to run it in this case, because it could be a Unix program, so far so good.
However, `file.noassoc` could actually be a valid PE file, and running it would succeed. Is this a desirable outcome? This is in one of our test cases: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/shell32/tests/shlexec... `test file.noassoc` is a valid PE file. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5400#note_66693
participants (4)
-
Alexandre Julliard (＠julliard) -
Dmitry Timoshkov (＠dmitry)
-
Yuxuan Shui
-
Yuxuan Shui (＠yshui)