[PATCH v8 0/6] MR6004: comctl32: A NULL string fix.
-- v8: user32/edit: Add some WM_ASKCBFORMATNAME tests. user32/tests: Add tests for NULL strings with edit control GETTEXT message. comctl32/tests: Add tests for NULL strings with edit control GETTEXT message. user32: Correctly handle NULL string in GETTEXT/CBFORMAT AtoW wrapper. https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> Zadig passes a fixed string length to WM_GETTEXT in one place regardless of the string: https://github.com/pbatard/libwdi/blob/9b23b82a2dd1cbffc16d46c212f92c6bf8c0c... (which causes a segfault with NULL strings on Wine but not on Windows). This fixes an unrelated issue in https://bugs.winehq.org/show_bug.cgi?id=54750 (but not the main one). --- dlls/comctl32/edit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 4b950412617..b7fd50a75e9 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -3183,7 +3183,7 @@ static void EDIT_WM_ContextMenu(EDITSTATE *es, INT x, INT y) */ static INT EDIT_WM_GetText(const EDITSTATE *es, INT count, LPWSTR dst) { - if (!count) + if (!count || !dst) return 0; lstrcpynW(dst, es->text, count); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> --- dlls/user32/edit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 39f429813d6..696acea16cd 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -3340,7 +3340,7 @@ static void EDIT_WM_ContextMenu(EDITSTATE *es, INT x, INT y) */ static INT EDIT_WM_GetText(const EDITSTATE *es, INT count, LPWSTR dst, BOOL unicode) { - if(!count) return 0; + if(!count || !dst) return 0; if(unicode) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> Spotted in WM_GETTEXT NULL string tests. --- dlls/user32/winproc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c index 5678e75b4e9..1ce5c87f954 100644 --- a/dlls/user32/winproc.c +++ b/dlls/user32/winproc.c @@ -274,9 +274,13 @@ LRESULT WINPROC_CallProcAtoW( winproc_callback_t callback, HWND hwnd, UINT msg, LPSTR str = (LPSTR)lParam; DWORD len = wParam * sizeof(WCHAR); - if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) break; + if (!str) + ptr = NULL; + else if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) + break; + ret = callback( hwnd, msg, wParam, (LPARAM)ptr, result, arg ); - if (wParam) + if (wParam && str) { len = 0; if (*result) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> --- dlls/comctl32/tests/edit.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..92017c046b2 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -1470,6 +1470,19 @@ static void test_edit_control_6(void) ok(ret == strlen(str), "Expected %s, got len %ld\n", str, ret); ok(!strcmp(buf, str), "Expected %s, got %s\n", str, buf); + ret = SendMessageA(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + ret = SendMessageA(hWnd, WM_GETTEXT, 0, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + + if (0) /* Crashes on Windows. */ + { + ret = SendMessageW(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + ret = SendMessageW(hWnd, WM_GETTEXT, 0, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + } + buf[0] = 0; ret = SendMessageA(hWnd, WM_DESTROY, 0, 0); todo_wine -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> Most of these crash on Windows. --- dlls/user32/tests/edit.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..99725b27eba 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -1394,6 +1394,20 @@ static void test_edit_control_6(void) ret = SendMessageA(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)buf); ok(ret == strlen(str), "Expected %s, got len %ld\n", str, ret); ok(!strcmp(buf, str), "Expected %s, got %s\n", str, buf); + + if (0) /* Crashes on Windows. */ + { + ret = SendMessageA(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + ret = SendMessageA(hWnd, WM_GETTEXT, 0, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + + ret = SendMessageW(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + ret = SendMessageW(hWnd, WM_GETTEXT, 0, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + } + buf[0] = 0; ret = SendMessageA(hWnd, WM_DESTROY, 0, 0); ok(ret == 0, "Expected 0, got %ld\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
From: Aida Jonikienė <aidas957(a)gmail.com> --- dlls/user32/tests/edit.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index 99725b27eba..9bc8d776fa5 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3298,6 +3298,43 @@ static void test_paste(void) DestroyWindow(hMultilineEdit); } +static void test_WM_ASKCBFORMATNAME(void) +{ + char buf[MAXLEN]; + HWND hwnd; + LONG ret; + HANDLE ret2; + + hwnd = CreateWindowExA(0, "EDIT", "Test", 0, 10, 10, 1, 1, NULL, NULL, hinst, NULL); + ok(hwnd != NULL, "Failed to create edit control.\n"); + + ret = open_clipboard(hwnd); + ok(ret == TRUE, "Expected %d, got %ld (error %lu)", TRUE, ret, GetLastError()); + ret = EmptyClipboard(); + ok(ret == TRUE, "Expected %d, got %ld\n", TRUE, ret); + + SetClipboardViewer(hwnd); + ret2 = SetClipboardData(CF_OWNERDISPLAY, NULL); + ok(ret2 == NULL, "Expected NULL, got %p\n", ret2); + + ret = SendMessageA(hwnd, WM_ASKCBFORMATNAME, 0, (LPARAM)NULL); + ok(ret == FALSE, "Expected %d, got %ld\n", FALSE, ret); + ret = SendMessageA(hwnd, WM_ASKCBFORMATNAME, MAXLEN, (LPARAM)NULL); + ok(ret == FALSE, "Expected %d, got %ld\n", FALSE, ret); + + memset(buf, 0, sizeof(buf)); + ret = SendMessageA(hwnd, WM_ASKCBFORMATNAME, 0, (LPARAM)buf); + ok(ret == FALSE, "Expected %d, got %ld\n", FALSE, ret); + ok(strcmp(buf, "") == 0, "Expected NULL, got %s\n", buf); + memset(buf, 0, sizeof(buf)); + ret = SendMessageA(hwnd, WM_ASKCBFORMATNAME, MAXLEN, (LPARAM)buf); + ok(ret == FALSE, "Expected %d, got %ld\n", FALSE, ret); + ok(strcmp(buf, "") == 0, "Expected NULL, got %s\n", buf); + + CloseClipboard(); + DestroyWindow(hwnd); +} + static void test_EM_GETLINE(void) { HWND hwnd[2]; @@ -3475,6 +3512,7 @@ START_TEST(edit) test_contextmenu(); test_EM_GETHANDLE(); test_paste(); + test_WM_ASKCBFORMATNAME(); test_EM_GETLINE(); test_wordbreak_proc(); test_dbcs_WM_CHAR(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
ret = SendMessageA(hWnd, WM_GETTEXT, MAXLEN, (LPARAM)buf); ok(ret == strlen(str), "Expected %s, got len %ld\n", str, ret); ok(!strcmp(buf, str), "Expected %s, got %s\n", str, buf); + + if (0) /* Crashes on Windows. */ Most of these crash on Windows.
Most or all? If they all crash on Windows, we should just let it crash. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76500
On Thu Jul 18 06:34:14 2024 +0000, Zhiyi Zhang wrote:
Most of these crash on Windows. Most or all? If they all crash on Windows, we should just let it crash. Crashing tests would likely stop the rest of the tests in that section (user32:edit) from running and introduce a red checkbox for the Windows test jobs (which Julliard wouldn't be happy about)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76507
On Thu Jul 18 09:11:00 2024 +0000, Aida Jonikienė wrote:
Crashing tests would likely stop the rest of the tests in that section (user32:edit) from running and introduce a red checkbox for the Windows test jobs (which Julliard wouldn't be happy about) I meant that if user32 edit is supposed to crash on all Windows versions when handling WM_GETTEXT with a NULL destination buffer. Then according to Wine's bug-for-bug compatibility guide line, we should just let it crash in such a case. Then you also don't need to add such tests to user32/tests.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76510
participants (2)
-
Aida Jonikienė -
Zhiyi Zhang (＠zhiyi)