-- v9: 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. user32: Fail on NULL string for edit control WM_GETTEXT message. comctl32: Fail on NULL string for edit control WM_GETTEXT message.
From: Aida Jonikienė aidas957@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);
From: Aida Jonikienė aidas957@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) {
From: Aida Jonikienė aidas957@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)
From: Aida Jonikienė aidas957@gmail.com
--- dlls/comctl32/tests/edit.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..e88a42aebb0 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -1470,6 +1470,21 @@ 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); + ret = SendMessageW(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, 1, (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); + } + buf[0] = 0; ret = SendMessageA(hWnd, WM_DESTROY, 0, 0); todo_wine
From: Aida Jonikienė aidas957@gmail.com
--- dlls/user32/tests/edit.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..859c5b2c6a3 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -1394,6 +1394,22 @@ 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 = SendMessageW(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 = SendMessageA(hWnd, WM_GETTEXT, 0, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + + if (0) /* Crashes on Windows. */ + { + ret = SendMessageA(hWnd, WM_GETTEXT, 1, (LPARAM)NULL); + ok(ret == 0, "Expected 0, got len %ld\n", ret); + ret = SendMessageA(hWnd, WM_GETTEXT, MAXLEN, (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);
From: Aida Jonikienė aidas957@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 859c5b2c6a3..306e71589f1 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3300,6 +3300,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]; @@ -3477,6 +3514,7 @@ START_TEST(edit) test_contextmenu(); test_EM_GETHANDLE(); test_paste(); + test_WM_ASKCBFORMATNAME(); test_EM_GETLINE(); test_wordbreak_proc(); test_dbcs_WM_CHAR();
On Thu Jul 18 09:28:19 2024 +0000, Zhiyi Zhang wrote:
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.
I updated the MR with more accurate test results :frog:
In this MR version, the build-clang failure is in odbc32 and test-linux-32 (actually the only full Linux test) failure is in secur32 (so both are unrelated to this MR) :frog:
This merge request was approved by Zhiyi Zhang.
I don't think I follow how tests results were translated to code changes. If it crashes on NULL buffer on Windows, which it does, why are we not doing the same? Having 0 length and NULL buffer is fine, having non-zero length and NULL buffer crashes, so why are we checking output pointer?
On Wed Jul 31 08:42:15 2024 +0000, Nikolay Sivov wrote:
I don't think I follow how tests results were translated to code changes. If it crashes on NULL buffer on Windows, which it does, why are we not doing the same? Having 0 length and NULL buffer is fine, having non-zero length and NULL buffer crashes, so why are we checking output pointer?
This MR was intended to be a quick fix for the Zadig program but it snowballed into these 6 patches because of test requirements and other unrelated code changes
I might try to make the test behavior closer to Windows but isn't better memory safety a good thing?
On Wed Jul 31 08:42:15 2024 +0000, Aida Jonikienė wrote:
This MR was intended to be a quick fix for the Zadig program but it snowballed into these 6 patches because of test requirements and other unrelated code changes I might try to make the test behavior closer to Windows but isn't better memory safety a good thing?
It isn't better if it doesn't work this way.
-
Aida Jonikienė -
Aida Jonikienė (＠DodoGTA)
-
Nikolay Sivov (＠nsivov)
-
Zhiyi Zhang (＠zhiyi)