[PATCH v4 0/6] MR5342: Draft: shell32: fix some divergence with Windows in FindExecutable, ShellExecute, and PathResolve
The motivator is `ShellExecute`, currently it searches for executables first relative to the current executable, then relative to the current working directory; whereas it should only search in the current working directory. While trying to fix that, I found some related bugs in `FindExecutable` and `PathResolve`. -- v4: shell32: PathResolve should remove trailing dot. https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> FindExecutable should look in the current working directory, not relative to the current executable image (which is what SearchPathW(NULL, ...) does). --- dlls/shell32/tests/shlexec.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c index 11332484800..ba28bbc83d2 100644 --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -2009,9 +2009,11 @@ static void test_urls(void) static void test_find_executable(void) { + char curdir[MAX_PATH]; char notepad_path[MAX_PATH]; char filename[MAX_PATH + 17]; char command[MAX_PATH]; + char *basename = strrchr(argv0, '\\') + 1; const filename_tests_t* test; INT_PTR rc; @@ -2052,6 +2054,18 @@ static void test_find_executable(void) ok(rc == SE_ERR_NOASSOC /* >= win2000 */ || rc > 32 /* win98, nt4 */, "FindExecutable(NULL) returned %Id\n", rc); ok(strcmp(command, "your word") != 0, "FindExecutable(NULL) returned command=[%s]\n", command); + /* Search for the current executabe itself */ + strcpy(command, "your word"); + rc=(INT_PTR)FindExecutableA(argv0, NULL, command); + ok(rc > 32, "FindExecutable(%s) returned %Id\n", argv0, rc); + + /* Make sure FindExecutable uses the correct current directory */ + GetCurrentDirectoryA(MAX_PATH, curdir); + SetCurrentDirectoryA(tmpdir); + rc=(INT_PTR)FindExecutableA(basename, NULL, command); + todo_wine ok(rc == SE_ERR_FNF, "FindExecutable(%s) returned %Id\n", basename, rc); + SetCurrentDirectoryA(curdir); + sprintf(filename, "%s\\test file.sfe", tmpdir); rc=(INT_PTR)FindExecutableA(filename, NULL, command); ok(rc > 32, "FindExecutable(%s) returned %Id\n", filename, rc); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/shell32/tests/shlexec.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c index ba28bbc83d2..c9e23c7062e 100644 --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -2266,6 +2266,8 @@ static void test_exes(void) { char filename[2 * MAX_PATH + 17]; char params[1024]; + char curdir[MAX_PATH]; + char *basename = strrchr(argv0, '\\') + 1; INT_PTR rc; sprintf(params, "shlexec \"%s\" Exec", child_file); @@ -2353,6 +2355,13 @@ static void test_exes(void) "notaverb", argv0, NULL, NULL, NULL); todo_wine okShell(rc == SE_ERR_NOASSOC, "returned %Iu\n", rc); + /* Check the correct search path is used */ + GetCurrentDirectoryA(MAX_PATH, curdir); + SetCurrentDirectoryA(tmpdir); + rc = shell_execute(NULL, basename, params, NULL); + todo_wine okShell(rc == SE_ERR_FNF, "returned %Iu\n", rc); + SetCurrentDirectoryA(curdir); + if (!skip_shlexec_tests) { /* A class overrides the normal handling of executables too */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> Unless it's explicitly specified. --- dlls/shell32/shellpath.c | 4 ++-- dlls/shell32/tests/shellpath.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c index 015d7cdd4e2..e7d4e181682 100644 --- a/dlls/shell32/shellpath.c +++ b/dlls/shell32/shellpath.c @@ -695,7 +695,7 @@ static BOOL PathResolveA(char *path, const char **dirs, DWORD flags) { if (PathFindOnPathExA(path, dirs, dwWhich)) return TRUE; - if (PathFileExistsDefExtA(path, dwWhich)) + if (!is_file_spec && PathFileExistsDefExtA(path, dwWhich)) return TRUE; if (!is_file_spec) GetFullPathNameA(path, MAX_PATH, path, NULL); SetLastError(ERROR_FILE_NOT_FOUND); @@ -724,7 +724,7 @@ static BOOL PathResolveW(WCHAR *path, const WCHAR **dirs, DWORD flags) { if (PathFindOnPathExW(path, dirs, dwWhich)) return TRUE; - if (PathFileExistsDefExtW(path, dwWhich)) + if (!is_file_spec && PathFileExistsDefExtW(path, dwWhich)) return TRUE; if (!is_file_spec) GetFullPathNameW(path, MAX_PATH, path, NULL); SetLastError(ERROR_FILE_NOT_FOUND); diff --git a/dlls/shell32/tests/shellpath.c b/dlls/shell32/tests/shellpath.c index 29b47ac8706..c493f6882ee 100644 --- a/dlls/shell32/tests/shellpath.c +++ b/dlls/shell32/tests/shellpath.c @@ -2942,7 +2942,8 @@ if (0) { /* crashes on native */ static void test_PathResolve(void) { WCHAR testfile[MAX_PATH], testfile_lnk[MAX_PATH], regedit_in_testdir[MAX_PATH], regedit_cmd[MAX_PATH]; - WCHAR tempdir[MAX_PATH], path[MAX_PATH]; + WCHAR tempdir[MAX_PATH], path[MAX_PATH], curdir[MAX_PATH]; + WCHAR argv0_dir[MAX_PATH] = {0}, argv0_base[MAX_PATH] = {0}, *argv0_basep = NULL; const WCHAR *dirs[2] = { tempdir, NULL }; HANDLE file, file2; BOOL ret; @@ -3013,6 +3014,15 @@ static void test_PathResolve(void) return; } + ret = GetModuleFileNameW(NULL, argv0_dir, sizeof(argv0_dir)); + ok(ret != 0 && ret < sizeof(argv0_dir), "GetModuleFileName failed\n"); + if (ret != 0 && ret < sizeof(argv0_dir)) + { + argv0_basep = wcsrchr(argv0_dir, '\\'); + *argv0_basep = 0; + argv0_basep++; + } + GetTempPathW(MAX_PATH, tempdir); lstrcpyW(testfile, tempdir); @@ -3038,6 +3048,28 @@ static void test_PathResolve(void) ok(!lstrcmpiW(path, L"C:\\windows\\regedit.exe") || !lstrcmpiW(path, L"C:\\windows\\system32\\regedit.exe"), "unexpected path %s\n", wine_dbgstr_w(path)); + /* show that PathResolve doesn't check current directory */ + if (argv0_basep) + { + WCHAR *ext; + lstrcpyW(argv0_base, argv0_basep); + GetCurrentDirectoryW(MAX_PATH, curdir); + SetCurrentDirectoryW(argv0_dir); + ret = pPathResolve(argv0_base, NULL, PRF_VERIFYEXISTS | PRF_TRYPROGRAMEXTENSIONS); + ok(!ret, "resolving argv0 succeeded unexpectedly, result: %s\n", wine_dbgstr_w(argv0_base)); + + lstrcpyW(argv0_base, argv0_basep); + if ((ext = wcsrchr(argv0_base, '.'))) + { + *ext = 0; + ret = pPathResolve(argv0_base, NULL, PRF_VERIFYEXISTS | PRF_TRYPROGRAMEXTENSIONS); + ok(!ret, "resolving argv0 without extension succeeded unexpectedly, result: %s\n", wine_dbgstr_w(argv0_base)); + } + SetCurrentDirectoryW(curdir); + } + else + win_skip("couldn't get module filename\n"); + for (i = 0; i < ARRAY_SIZE(tests); i++) { winetest_push_context("test %d", i); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> Previously looking for file that does exist in current directory will fail because of the early `!PathFileExists(path)` check, even when the current directory is specified in `dirs`. --- dlls/shell32/shellpath.c | 4 ++-- dlls/shell32/tests/shellpath.c | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c index e7d4e181682..9b527b35ac4 100644 --- a/dlls/shell32/shellpath.c +++ b/dlls/shell32/shellpath.c @@ -691,7 +691,7 @@ static BOOL PathResolveA(char *path, const char **dirs, DWORD flags) TRACE("(%s,%p,0x%08lx)\n", debugstr_a(path), dirs, flags); - if (flags & PRF_VERIFYEXISTS && !PathFileExistsA(path)) + if (flags & PRF_VERIFYEXISTS) { if (PathFindOnPathExA(path, dirs, dwWhich)) return TRUE; @@ -720,7 +720,7 @@ static BOOL PathResolveW(WCHAR *path, const WCHAR **dirs, DWORD flags) TRACE("(%s,%p,0x%08lx)\n", debugstr_w(path), dirs, flags); - if (flags & PRF_VERIFYEXISTS && !PathFileExistsW(path)) + if (flags & PRF_VERIFYEXISTS) { if (PathFindOnPathExW(path, dirs, dwWhich)) return TRUE; diff --git a/dlls/shell32/tests/shellpath.c b/dlls/shell32/tests/shellpath.c index c493f6882ee..5ed25136003 100644 --- a/dlls/shell32/tests/shellpath.c +++ b/dlls/shell32/tests/shellpath.c @@ -3048,10 +3048,14 @@ static void test_PathResolve(void) ok(!lstrcmpiW(path, L"C:\\windows\\regedit.exe") || !lstrcmpiW(path, L"C:\\windows\\system32\\regedit.exe"), "unexpected path %s\n", wine_dbgstr_w(path)); - /* show that PathResolve doesn't check current directory */ if (argv0_basep) { WCHAR *ext; + const WCHAR *search_path[] = { + argv0_dir, + NULL + }; + /* show that PathResolve doesn't check current directory */ lstrcpyW(argv0_base, argv0_basep); GetCurrentDirectoryW(MAX_PATH, curdir); SetCurrentDirectoryW(argv0_dir); @@ -3065,6 +3069,16 @@ static void test_PathResolve(void) ret = pPathResolve(argv0_base, NULL, PRF_VERIFYEXISTS | PRF_TRYPROGRAMEXTENSIONS); ok(!ret, "resolving argv0 without extension succeeded unexpectedly, result: %s\n", wine_dbgstr_w(argv0_base)); } + + /* show that PathResolve will check specified search path, even if it's the current directory */ + lstrcpyW(argv0_base, argv0_basep); + if ((ext = wcsrchr(argv0_base, '.'))) + { + *ext = 0; + ret = pPathResolve(argv0_base, search_path, PRF_VERIFYEXISTS | PRF_TRYPROGRAMEXTENSIONS); + ok(ret, "resolving argv0 without extension with search path succeeded unexpectedly, result: %s\n", wine_dbgstr_w(argv0_base)); + } + SetCurrentDirectoryW(curdir); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> Setting dwWhich to 0xff forces extensions to be appended, even when the file name already includes an extension. This causes PathResolve to fail in some cases where the file does exist. --- dlls/shell32/shellpath.c | 4 ++-- dlls/shell32/tests/shellpath.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c index 9b527b35ac4..5d7f9d48ebd 100644 --- a/dlls/shell32/shellpath.c +++ b/dlls/shell32/shellpath.c @@ -687,7 +687,7 @@ BOOL WINAPI PathFileExistsDefExtW(LPWSTR,DWORD); static BOOL PathResolveA(char *path, const char **dirs, DWORD flags) { BOOL is_file_spec = PathIsFileSpecA(path); - DWORD dwWhich = flags & PRF_DONTFINDLNK ? 0xf : 0xff; + DWORD dwWhich = flags & PRF_DONTFINDLNK ? 0xf : 0xbf; TRACE("(%s,%p,0x%08lx)\n", debugstr_a(path), dirs, flags); @@ -716,7 +716,7 @@ static BOOL PathResolveA(char *path, const char **dirs, DWORD flags) static BOOL PathResolveW(WCHAR *path, const WCHAR **dirs, DWORD flags) { BOOL is_file_spec = PathIsFileSpecW(path); - DWORD dwWhich = flags & PRF_DONTFINDLNK ? 0xf : 0xff; + DWORD dwWhich = flags & PRF_DONTFINDLNK ? 0xf : 0xbf; TRACE("(%s,%p,0x%08lx)\n", debugstr_w(path), dirs, flags); diff --git a/dlls/shell32/tests/shellpath.c b/dlls/shell32/tests/shellpath.c index 5ed25136003..5778dcc2fc8 100644 --- a/dlls/shell32/tests/shellpath.c +++ b/dlls/shell32/tests/shellpath.c @@ -2962,6 +2962,7 @@ static void test_PathResolve(void) /* PRF_VERIFYEXISTS */ { L"shellpath", PRF_VERIFYEXISTS, TRUE, testfile_lnk }, + { L"shellpath.lnk", PRF_VERIFYEXISTS, TRUE, testfile_lnk }, { L"C:\\shellpath", PRF_VERIFYEXISTS, FALSE, L"C:\\shellpath" }, /* common extensions are tried even if PRF_TRYPROGRAMEXTENSIONS isn't passed */ /* directories in dirs parameter are always searched first even if PRF_FIRSTDIRDEF isn't passed */ @@ -3071,6 +3072,10 @@ static void test_PathResolve(void) } /* show that PathResolve will check specified search path, even if it's the current directory */ + lstrcpyW(argv0_base, argv0_basep); + ret = pPathResolve(argv0_base, search_path, PRF_VERIFYEXISTS | PRF_TRYPROGRAMEXTENSIONS); + ok(ret, "resolving argv0 with search path failed unexpectedly, result: %s\n", wine_dbgstr_w(argv0_base)); + lstrcpyW(argv0_base, argv0_basep); if ((ext = wcsrchr(argv0_base, '.'))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/shell32/shellpath.c | 24 ++++++++++++++++++------ dlls/shell32/tests/shellpath.c | 1 + 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c index 5d7f9d48ebd..e32bf5a50dc 100644 --- a/dlls/shell32/shellpath.c +++ b/dlls/shell32/shellpath.c @@ -694,10 +694,16 @@ static BOOL PathResolveA(char *path, const char **dirs, DWORD flags) if (flags & PRF_VERIFYEXISTS) { if (PathFindOnPathExA(path, dirs, dwWhich)) + { + if (!PathIsFileSpecA(path)) GetFullPathNameA(path, MAX_PATH, path, NULL); return TRUE; - if (!is_file_spec && PathFileExistsDefExtA(path, dwWhich)) - return TRUE; - if (!is_file_spec) GetFullPathNameA(path, MAX_PATH, path, NULL); + } + if (!is_file_spec) + { + GetFullPathNameA(path, MAX_PATH, path, NULL); + if (PathFileExistsDefExtA(path, dwWhich)) + return TRUE; + } SetLastError(ERROR_FILE_NOT_FOUND); return FALSE; } @@ -723,10 +729,16 @@ static BOOL PathResolveW(WCHAR *path, const WCHAR **dirs, DWORD flags) if (flags & PRF_VERIFYEXISTS) { if (PathFindOnPathExW(path, dirs, dwWhich)) + { + if (!PathIsFileSpecW(path)) GetFullPathNameW(path, MAX_PATH, path, NULL); return TRUE; - if (!is_file_spec && PathFileExistsDefExtW(path, dwWhich)) - return TRUE; - if (!is_file_spec) GetFullPathNameW(path, MAX_PATH, path, NULL); + } + if (!is_file_spec) + { + GetFullPathNameW(path, MAX_PATH, path, NULL); + if (PathFileExistsDefExtW(path, dwWhich)) + return TRUE; + } SetLastError(ERROR_FILE_NOT_FOUND); return FALSE; } diff --git a/dlls/shell32/tests/shellpath.c b/dlls/shell32/tests/shellpath.c index 5778dcc2fc8..fcf25c702ab 100644 --- a/dlls/shell32/tests/shellpath.c +++ b/dlls/shell32/tests/shellpath.c @@ -2963,6 +2963,7 @@ static void test_PathResolve(void) /* PRF_VERIFYEXISTS */ { L"shellpath", PRF_VERIFYEXISTS, TRUE, testfile_lnk }, { L"shellpath.lnk", PRF_VERIFYEXISTS, TRUE, testfile_lnk }, + { L"shellpath.lnk.", PRF_VERIFYEXISTS, TRUE, testfile_lnk }, { L"C:\\shellpath", PRF_VERIFYEXISTS, FALSE, L"C:\\shellpath" }, /* common extensions are tried even if PRF_TRYPROGRAMEXTENSIONS isn't passed */ /* directories in dirs parameter are always searched first even if PRF_FIRSTDIRDEF isn't passed */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5342
participants (2)
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)