[PATCH v5 0/7] 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`.
-- v5: shell32: Fix FindExecutable search path.
From: Yuxuan Shui yshui@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);
From: Yuxuan Shui yshui@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 */
From: Yuxuan Shui yshui@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);
From: Yuxuan Shui yshui@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
From: Yuxuan Shui yshui@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, '.'))) {
From: Yuxuan Shui yshui@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 */
From: Yuxuan Shui yshui@codeweavers.com
It should look in the currect working directory, instead of the directory where the current executable is in. --- dlls/shell32/shlexec.c | 20 ++++++++++++++------ dlls/shell32/tests/shlexec.c | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c index abb519e260d..9aa7963b71c 100644 --- a/dlls/shell32/shlexec.c +++ b/dlls/shell32/shlexec.c @@ -48,6 +48,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(exec);
typedef UINT_PTR (*SHELL_ExecuteW32)(const WCHAR *lpCmd, WCHAR *env, BOOL shWait, const SHELLEXECUTEINFOW *sei, LPSHELLEXECUTEINFOW sei_out); +extern BOOL WINAPI PathResolveAW(void *path, const void **paths, DWORD flags);
static inline BOOL isSpace(WCHAR c) { @@ -599,6 +600,8 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, WCHAR *tok; /* token pointer */ WCHAR xlpFile[256]; /* result of SearchPath */ DWORD attribs; /* file attributes */ + WCHAR curdir[MAX_PATH]; + const WCHAR *search_paths[3] = {0};
TRACE("%s\n", debugstr_w(lpFile));
@@ -623,18 +626,23 @@ static UINT SHELL_FindExecutable(LPCWSTR lpPath, LPCWSTR lpFile, LPCWSTR lpVerb, return 33; }
- if (SearchPathW(lpPath, lpFile, L".exe", ARRAY_SIZE(xlpFile), xlpFile, NULL)) + if (lpPath && *lpPath) { - TRACE("SearchPathW returned non-zero\n"); - lpFile = xlpFile; - /* The file was found in the application-supplied default directory (or the system search path) */ + search_paths[0] = lpPath; + search_paths[1] = curdir; } - else if (lpPath && SearchPathW(NULL, lpFile, L".exe", ARRAY_SIZE(xlpFile), xlpFile, NULL)) + else + search_paths[0] = curdir; + GetCurrentDirectoryW(MAX_PATH, curdir); + lstrcpyW(xlpFile, lpFile); + if (PathResolveAW(xlpFile, (const void **)search_paths, PRF_TRYPROGRAMEXTENSIONS | PRF_VERIFYEXISTS)) { TRACE("SearchPathW returned non-zero\n"); lpFile = xlpFile; - /* The file was found in one of the directories in the system-wide search path */ + /* The file was found in lpPath or one of the directories in the system-wide search path */ } + else + xlpFile[0] = '\0';
attribs = GetFileAttributesW(lpFile); if (attribs!=INVALID_FILE_ATTRIBUTES && (attribs&FILE_ATTRIBUTE_DIRECTORY)) diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c index c9e23c7062e..2f95ff2e15d 100644 --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -2063,7 +2063,7 @@ static void test_find_executable(void) 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); + ok(rc == SE_ERR_FNF, "FindExecutable(%s) returned %Id\n", basename, rc); SetCurrentDirectoryA(curdir);
sprintf(filename, "%s\test file.sfe", tmpdir);
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)