[PATCH] user32: LoadImage should search image in PE directory, not the working directory

[Fabian Maurer]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46186
Signed-off-by: Fabian Maurer 
---
 dlls/user32/cursoricon.c       | 15 ++++++++-
 dlls/user32/tests/cursoricon.c | 56 ++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/dlls/user32/cursoricon.c b/dlls/user32/cursoricon.c
index f11f5e45ed..4d803e7818 100644
--- a/dlls/user32/cursoricon.c
+++ b/dlls/user32/cursoricon.c
@@ -3039,11 +3039,24 @@ HANDLE WINAPI LoadImageW( HINSTANCE hinst, LPCWSTR name, UINT type,
                 INT desiredx, INT desiredy, UINT loadflags )
 {
     int depth;
+    WCHAR path[MAX_PATH];

TRACE_(resource)("(%p,%s,%d,%d,%d,0x%08x)\n",
                      hinst,debugstr_w(name),type,desiredx,desiredy,loadflags);

-    if (loadflags & LR_LOADFROMFILE) loadflags &= ~LR_SHARED;
+    if (loadflags & LR_LOADFROMFILE) {
+        loadflags &= ~LR_SHARED;
+
+        /* Relative paths are relative to the PE directory - not the current working directory */
+        if (name != NULL && lstrlenW(name) > 2 && name[0] != '\' && name[1] != ':') {
+            WCHAR *part_filename;
+            GetModuleFileNameW(hinst, path, ARRAY_SIZE(path));
+            part_filename = strrchrW(path, '\') + 1;
+            *part_filename = '\0';
+            strcatW(path, name);
+            name = path;
+        }
+    }
     switch (type) {
     case IMAGE_BITMAP:
         return BITMAP_Load( hinst, name, desiredx, desiredy, loadflags );
diff --git a/dlls/user32/tests/cursoricon.c b/dlls/user32/tests/cursoricon.c
index 8404ee71ab..a4af988720 100644
--- a/dlls/user32/tests/cursoricon.c
+++ b/dlls/user32/tests/cursoricon.c
@@ -1203,6 +1203,59 @@ static void create_ico_file(const char *filename, const test_icon_entries_t *tes
     HeapFree(GetProcessHeap(), 0, buf);
 }

+static void test_LoadImageWorkingDirectory(void)
+{
+    char working_dir[MAX_PATH];
+    char temp_dir[MAX_PATH];
+    DWORD bytes_written;
+    HANDLE handle;
+    BOOL ret;
+    static const test_icon_entries_t icon_desc = {32, 32};
+
+    GetCurrentDirectoryA(ARRAY_SIZE(working_dir), working_dir);
+    GetTempPathA(ARRAY_SIZE(temp_dir), temp_dir);
+
+    strcat(temp_dir, "wine-test-dir");
+
+    CreateDirectoryA(temp_dir, NULL);
+
+    /* Create Files */
+    create_ico_file("icon.ico", &icon_desc, 1);
+
+    handle = CreateFileA("test.bmp", GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
+    ok(handle != INVALID_HANDLE_VALUE, "CreateFileA failed. %u\n", GetLastError());
+    ret = WriteFile(handle, bmpimage, sizeof(bmpimage), &bytes_written, NULL);
+    ok(ret && bytes_written == sizeof(bmpimage), "Test file created improperly.\n");
+    CloseHandle(handle);
+
+    SetCurrentDirectoryA(temp_dir);
+
+    /* Test cursor */
+    handle = LoadImageA(NULL, "icon.ico", IMAGE_CURSOR, 0, 0, LR_LOADFROMFILE);
+    ok(handle != NULL, "LoadImage() failed.\n");
+
+    ret = DestroyCursor(handle);
+    ok(ret, "DestroyCusor failed: %d\n", GetLastError());
+
+    /* Test image */
+    handle = LoadImageA(NULL, "test.bmp", IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
+    ok(handle != NULL, "LoadImageA failed.\n");
+
+    ret = DeleteObject(handle);
+    ok(ret, "DeleteObject failed: %d\n", GetLastError());
+
+    /* Cleanup */
+    SetCurrentDirectoryA(working_dir);
+
+    ret = DeleteFileA("test.bmp");
+    ok(ret, "DeleteFileA failed: %d\n", GetLastError());
+    ret = DeleteFileA("icon.ico");
+    ok(ret, "RemoveDirectoryA failed: %d\n", GetLastError());
+
+    ret = RemoveDirectoryA(temp_dir);
+    ok(ret, "RemoveDirectoryA failed: %d\n", GetLastError());
+}
+
 static void test_LoadImage(void)
 {
     HANDLE handle;
@@ -1326,6 +1379,9 @@ static void test_LoadImage(void)
     bitmap_header->biSize = sizeof(BITMAPINFOHEADER);

test_LoadImageFile("Cursor (invalid dwDIBOffset)", invalid_dwDIBOffset, sizeof(invalid_dwDIBOffset), "cur", 0);
+
+    /* Test if images are properly loaded - from executable directory instead of current working directory */
+    test_LoadImageWorkingDirectory();
 }

static void test_CreateIconFromResource(void)

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=44925

Your paranoid android.

=== debian9 (32 bit WoW report) ===

user32:
input.c:2054: Test failed: expected WM_NCHITTEST message

[Alexandre Julliard]

Fabian Maurer  writes:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46186
> Signed-off-by: Fabian Maurer
>
> dlls/user32/cursoricon.c       | 15 ++++++++-
>  dlls/user32/tests/cursoricon.c | 56 ++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 1 deletion(-)

This would need more tests, but it's pretty unlikely that it would
search only the exe directory. Probably it should be using something
like SearchPathW.

[Fabian Maurer]

> This would need more tests, but it's pretty unlikely that it would
> search only the exe directory. Probably it should be using something
> like SearchPathW.

Thanks, sounds sensible.

Apart from current working directory and PE directory, which paths 
should I best write tests for?

Regards,
Fabian Maurer

[Dmitry Timoshkov]

Fabian Maurer  wrote:
> > This would need more tests, but it's pretty unlikely that it would
> > search only the exe directory. Probably it should be using something
> > like SearchPathW.
>
> Thanks, sounds sensible.
>
> Apart from current working directory and PE directory, which paths 
> should I best write tests for?

Create temp path with an image, add it to %PATH%, call LoadImage, and
see what happens?

[Erich E. Hoover]

On Mon, Nov 26, 2018 at 1:15 PM Dmitry Timoshkov  wrote:
> Fabian Maurer  wrote:
> > > This would need more tests, but it's pretty unlikely that it would
> > > search only the exe directory. Probably it should be using something
> > > like SearchPathW.
> >
> > Thanks, sounds sensible.
> >
> > Apart from current working directory and PE directory, which paths
> > should I best write tests for?
>
> Create temp path with an image, add it to %PATH%, call LoadImage, and
> see what happens?

The behavior may also be dependent upon the setting of
NeedCurrentDirectoryForExePath (
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684269(v=vs.85).a...
).  I have a staging patch for CreateProcess that checks this, but
it's possible that the behavior is implemented at a lower level.

Best,
Erich