[PATCH v2] user32: Add exception handler to GetWindowText.

[Dmitry Timoshkov]

v2: Fix the spaces.

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
 dlls/user32/tests/win.c | 29 ++++++++++++++++----
 dlls/user32/win.c       | 59 ++++++++++++++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index 6b13d93be14..dc18d054496 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -7923,17 +7923,36 @@ static void test_gettext(void)
     if (0)
     {
     r = SendMessageA( hwnd, WM_GETTEXT, 0x10, 0x1000);
-    ok( r == 0, "settext should return zero\n");
+    ok( r == 0, "WM_GETTEXT should return zero (%ld)\n", r );
 
     r = SendMessageA( hwnd, WM_GETTEXT, 0x10000, 0);
-    ok( r == 0, "settext should return zero (%ld)\n", r);
+    ok( r == 0, "WM_GETTEXT should return zero (%ld)\n", r );
 
     r = SendMessageA( hwnd, WM_GETTEXT, 0xff000000, 0x1000);
-    ok( r == 0, "settext should return zero (%ld)\n", r);
+    ok( r == 0, "WM_GETTEXT should return zero (%ld)\n", r );
 
     r = SendMessageA( hwnd, WM_GETTEXT, 0x1000, 0xff000000);
-    ok( r == 0, "settext should return zero (%ld)\n", r);
-    }
+    ok( r == 0, "WM_GETTEXT should return zero (%ld)\n", r );
+    }
+
+    /* GetWindowText doesn't crash */
+    r = GetWindowTextA( hwnd, (LPSTR)0x10, 0x1000 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextA( hwnd, (LPSTR)0x10000, 0 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextA( hwnd, (LPSTR)0xff000000, 0x1000 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextA( hwnd, (LPSTR)0x1000, 0xff000000 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+
+    r = GetWindowTextW( hwnd, (LPWSTR)0x10, 0x1000 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextW( hwnd, (LPWSTR)0x10000, 0 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextW( hwnd, (LPWSTR)0xff000000, 0x1000 );
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
+    r = GetWindowTextW( hwnd, (LPWSTR)0x1000, 0xff000000);
+    ok( r == 0, "GetWindowText should return zero (%ld)\n", r );
 
     DestroyWindow(hwnd);
 }
diff --git a/dlls/user32/win.c b/dlls/user32/win.c
index 5e89f4c2c97..e57f291be35 100644
--- a/dlls/user32/win.c
+++ b/dlls/user32/win.c
@@ -35,6 +35,7 @@
 #include "controls.h"
 #include "winerror.h"
 #include "wine/gdi_driver.h"
+#include "wine/exception.h"
 #include "wine/debug.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(win);
@@ -3001,22 +3002,34 @@ LONG WINAPI DECLSPEC_HOTPATCH SetWindowLongW(
 INT WINAPI GetWindowTextA( HWND hwnd, LPSTR lpString, INT nMaxCount )
 {
     WCHAR *buffer;
+    int ret = 0;
 
     if (!lpString || nMaxCount <= 0) return 0;
 
-    if (WIN_IsCurrentProcess( hwnd ))
+    __TRY
     {
-        lpString[0] = 0;
-        return (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString );
+        if (WIN_IsCurrentProcess( hwnd ))
+        {
+            lpString[0] = 0;
+            ret = (INT)SendMessageA( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString );
+        }
+        else if ((buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) )))
+        {
+            /* when window belongs to other process, don't send a message */
+            get_server_window_text( hwnd, buffer, nMaxCount );
+            if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL ))
+                lpString[nMaxCount-1] = 0;
+            HeapFree( GetProcessHeap(), 0, buffer );
+            ret = strlen(lpString);
+        }
+    }
+    __EXCEPT_ALL
+    {
+        ret = 0;
     }
+    __ENDTRY
 
-    /* when window belongs to other process, don't send a message */
-    if (!(buffer = HeapAlloc( GetProcessHeap(), 0, nMaxCount * sizeof(WCHAR) ))) return 0;
-    get_server_window_text( hwnd, buffer, nMaxCount );
-    if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpString, nMaxCount, NULL, NULL ))
-        lpString[nMaxCount-1] = 0;
-    HeapFree( GetProcessHeap(), 0, buffer );
-    return strlen(lpString);
+    return ret;
 }
 
 
@@ -3049,17 +3062,31 @@ INT WINAPI InternalGetWindowText(HWND hwnd,LPWSTR lpString,INT nMaxCount )
  */
 INT WINAPI GetWindowTextW( HWND hwnd, LPWSTR lpString, INT nMaxCount )
 {
+    int ret;
+
     if (!lpString || nMaxCount <= 0) return 0;
 
-    if (WIN_IsCurrentProcess( hwnd ))
+    __TRY
     {
-        lpString[0] = 0;
-        return (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString );
+        if (WIN_IsCurrentProcess( hwnd ))
+        {
+            lpString[0] = 0;
+            ret = (INT)SendMessageW( hwnd, WM_GETTEXT, nMaxCount, (LPARAM)lpString );
+        }
+        else
+        {
+            /* when window belongs to other process, don't send a message */
+            get_server_window_text( hwnd, lpString, nMaxCount );
+            ret = lstrlenW(lpString);
+        }
     }
+    __EXCEPT_ALL
+    {
+        ret = 0;
+    }
+    __ENDTRY
 
-    /* when window belongs to other process, don't send a message */
-    get_server_window_text( hwnd, lpString, nMaxCount );
-    return lstrlenW(lpString);
+    return ret;
 }
 
 
-- 
2.31.1

[Marvin]

Hi,

While running your changed tests, 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=99951

Your paranoid android.


=== debiant2 (32 bit WoW report) ===

user32:
win.c:10433: Test failed: Expected foreground window 0, got 00AD00EA
win.c:10439: Test failed: Expected foreground window 00120126, got 00AD00EA

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>

Seems like it is swallowing exceptions raised from the window proc indeed.
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

[Dmitry Timoshkov]

Hello,

Rémi Bernon signed off on this patch, is there anything else that should be
done for the patch to be accepted?

-- 
Dmitry.

[Rémi Bernon]

On 11/12/21 15:08, Dmitry Timoshkov wrote:
> Hello,
>
> Rémi Bernon signed off on this patch, is there anything else that should be
> done for the patch to be accepted?

As there's some win32u refactoring in progress, it's maybe delayed to 
prevent conflicts.

Then if it ends up behind a syscall interface maybe the exceptions will 
or should be trapped there.
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

[Dmitry Timoshkov]

Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> On 11/12/21 15:08, Dmitry Timoshkov wrote:
> > Hello,
> >
> > Rémi Bernon signed off on this patch, is there anything else that should be
> > done for the patch to be accepted?
>
> As there's some win32u refactoring in progress, it's maybe delayed to 
> prevent conflicts.
>
> Then if it ends up behind a syscall interface maybe the exceptions will 
> or should be trapped there.

It's pretty unlikely that GetWindowText() functionality should be moved
to win32u.

-- 
Dmitry.