[PATCH v5 0/4] MR6004: comctl32: A NULL string fix.
-- v5: user32/edit: Add some WM_ASKCBFORMATNAME tests. comctl32/user32: Add tests for NULL strings with edit control GETTEXT message. user32: Correctly handle NULL string in GETTEXT/CBFORMAT AtoW wrapper. comctl32/user32: Fail on NULL string for edit control WM_GETTEXT message. 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 +- dlls/user32/edit.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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); 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 +++++++++++++ dlls/user32/tests/edit.c | 14 ++++++++++++++ 2 files changed, 27 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 diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..8045ec7d9cb 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); + + 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); 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 8045ec7d9cb..33a15b4577a 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_cbformatname(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_cbformatname(); test_EM_GETLINE(); test_wordbreak_proc(); test_dbcs_WM_CHAR(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6004
On Wed Jul 10 19:46:55 2024 +0000, Zhiyi Zhang wrote:
Can't it be something like the following? ``` if (!str) ptr = NULL; else if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) break;
``` This should be solved now :frog:
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_75813
Please add tests for user32/edit as well
I copied the comctl32/edit stuff back to user32/edit (so that should be solved)
Also, test WM_ASKCBFORMATNAME.
I tried to add some tests in a recent push (but something may be wrong there because I always get a 0 length string) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_75814
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/edit.c:
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);
Please separate this commit to user32/tests and comctl32/tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76063
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
*/ static INT EDIT_WM_GetText(const EDITSTATE *es, INT count, LPWSTR dst, BOOL unicode) Please separate this commit into two, one for comctl32 and one for user32.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76064
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
DestroyWindow(hMultilineEdit); }
+static void test_cbformatname(void)
Let's rename this to test_WM_ASKCBFORMATNAME(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76065
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
DestroyWindow(hMultilineEdit); }
+static void test_cbformatname(void) +{ + char buf[MAXLEN]; + HWND hWnd;
a coding style nitpick. Let's use "hwnd" instead so that the style is consistent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6004#note_76066
participants (2)
-
Aida Jonikienė -
Zhiyi Zhang (＠zhiyi)