-- v4: comctl32/tests: Add tests for NULL strings with edit control WM_GETTEXT message. user32: Correctly handle NULL string in WM_GETTEXT AtoW wrapper.
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
Spotted in WM_GETTEXT NULL string tests. --- dlls/user32/winproc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c index 5678e75b4e9..4cb3e007de2 100644 --- a/dlls/user32/winproc.c +++ b/dlls/user32/winproc.c @@ -275,8 +275,14 @@ LRESULT WINPROC_CallProcAtoW( winproc_callback_t callback, HWND hwnd, UINT msg, DWORD len = wParam * sizeof(WCHAR);
if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) break; + if (!str) + { + free_buffer( buffer, ptr ); + ptr = NULL; + } + 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 | 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
The only failure is in the windows.media.speech test now (which seems unrelated); I'll re-run the test to see if it still happens :frog:
Zhiyi Zhang (@zhiyi) commented about dlls/user32/winproc.c:
DWORD len = wParam * sizeof(WCHAR); if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) break;
if (!str)
If you check str for NULL before get_buffer() then you don't need to call free_buffer().
Zhiyi Zhang (@zhiyi) commented about dlls/user32/winproc.c:
break; case WM_GETTEXT: case WM_ASKCBFORMATNAME:
Please add tests for WM_ASKCBFORMATNAME as well.
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);
Please add tests for user32/edit as well. Also, test WM_ASKCBFORMATNAME.
On Wed Jul 10 07:48:16 2024 +0000, Zhiyi Zhang wrote:
If you check str for NULL before get_buffer() then you don't need to call free_buffer().
If I move the `if` statement before the `ptr` assignment then NULL lParam won't be passed through (which doesn't match direct winproc call behavior)
On Wed Jul 10 07:49:27 2024 +0000, Zhiyi Zhang wrote:
Please add tests for user32/edit as well. Also, test WM_ASKCBFORMATNAME.
user32/edit and comctl32/edit basically look the same to me (so could those test parts be commonized?)
WM_ASKCBFORMATNAME basically has no meaningful tests though (so that would be a nice thing to work on) but probably nothing relies on that message either (so :shrug:)
user32/edit and comctl32/edit basically look the same to me (so could those test parts be commonized?)
They look the same but they are for different components, one for user32 edit and the another for the edit control of comctl32 version 6.
WM_ASKCBFORMATNAME basically has no meaningful tests though (so that would be a nice thing to work on) but probably nothing relies on that message either (so 🤷)
It's better to have a test.
On Wed Jul 10 11:44:08 2024 +0000, Aida Jonikienė wrote:
If I move the `if` statement before the `ptr` assignment then NULL lParam won't be passed through (which doesn't match direct winproc call behavior)
Can't it be something like the following?
``` if (!str) ptr = NULL; else if (!(ptr = get_buffer( buffer, sizeof(buffer), len ))) break;
```
-
Aida Jonikienė -
Aida Jonikienė (＠DodoGTA)
-
Zhiyi Zhang (＠zhiyi)