[PATCH 0/2] MR10235: gdiplus: Validate window handle in has_gdi_dc
graphics->owndc being TRUE indicates that graphics is created using GdipCreateFromHWND. In has_gdi_dc, it is necessary to check whether the window has already been destroyed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
From: YeshunYe <yeyeshun@uniontech.com> Add a test case to verify the behavior of GdipMeasureString when the window associated with the graphics object is destroyed before the operation. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/gdiplus/tests/graphics.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 96ef52ce935..bba7338973f 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -35,6 +35,7 @@ static GpStatus (WINAPI *pGdipGraphicsSetAbort)(GpGraphics*,GdiplusAbort*); static const REAL mm_per_inch = 25.4; static const REAL point_per_inch = 72.0; static HWND hwnd; +static HWND hwnd2; static void set_rect_empty(RectF *rc) { @@ -4629,6 +4630,7 @@ static void test_measure_string(void) CharacterRange range; GpRegion *region; GpGraphics *graphics; + GpGraphics *graphics2; GpFontFamily *family; GpFont *font; GpStatus status; @@ -4649,6 +4651,7 @@ static void test_measure_string(void) hdc = CreateCompatibleDC(0); status = GdipCreateFromHDC(hdc, &graphics); + status = GdipCreateFromHWND(hwnd2, &graphics2); status = GdipCreateFont(family, 20, FontStyleRegular, UnitPixel, &font); expect(Ok, status); @@ -5088,6 +5091,21 @@ static void test_measure_string(void) expect(3, glyphs); expect(1, lines); + DestroyWindow(hwnd2); + hwnd2 = NULL; + rect.X = 5.0; + rect.Y = 5.0; + rect.Width = width_M_M; + rect.Height = 32000.0; + status = GdipMeasureString(graphics2, string2, -1, font, &rect, format_no_wrap, &bounds, &glyphs, &lines); + todo_wine expect(Ok, status); + if (status == Ok) + { + expectf_(width_M_M, bounds.Width, 0.1); + expect(3, glyphs); + expect(1, lines); + } + status = GdipDeleteFont(font); expect(Ok, status); @@ -7553,6 +7571,9 @@ START_TEST(graphics) hwnd = CreateWindowA( "gdiplus_test", "graphics test", WS_OVERLAPPEDWINDOW | WS_VISIBLE, CW_USEDEFAULT, CW_USEDEFAULT, 200, 200, 0, 0, GetModuleHandleA(0), 0 ); ok(hwnd != NULL, "Expected window to be created\n"); + hwnd2 = CreateWindowA( "gdiplus_test", "test graphics after window destroy", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + CW_USEDEFAULT, CW_USEDEFAULT, 200, 200, 0, 0, GetModuleHandleA(0), 0 ); + ok(hwnd2 != NULL, "Expected window to be created\n"); gdiplusStartupInput.GdiplusVersion = 1; gdiplusStartupInput.DebugEventCallback = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
From: YeshunYe <yeyeshun@uniontech.com> graphics->owndc being TRUE indicates that graphics is created using `GdipCreateFromHWND`. In `has_gdi_dc`, it is necessary to check whether the window has already been destroyed. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/gdiplus/gdiplus_private.h | 2 +- dlls/gdiplus/tests/graphics.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 9d0a166fdeb..a5e3c09251e 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -699,7 +699,7 @@ static inline void image_unlock(GpImage *image) static inline BOOL has_gdi_dc(GpGraphics *graphics) { - return graphics->hdc != NULL || graphics->owndc; + return graphics->hdc != NULL || (graphics->owndc && IsWindow(graphics->hwnd)); } static inline void set_rect(GpRectF *rect, REAL x, REAL y, REAL width, REAL height) diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index bba7338973f..3195698deaf 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -5098,7 +5098,7 @@ static void test_measure_string(void) rect.Width = width_M_M; rect.Height = 32000.0; status = GdipMeasureString(graphics2, string2, -1, font, &rect, format_no_wrap, &bounds, &glyphs, &lines); - todo_wine expect(Ok, status); + expect(Ok, status); if (status == Ok) { expectf_(width_M_M, bounds.Width, 0.1); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/graphics.c:
hdc = CreateCompatibleDC(0); status = GdipCreateFromHDC(hdc, &graphics); + status = GdipCreateFromHWND(hwnd2, &graphics2);
`graphics2` is never deleted. Also, the `status` value is never tested (which is also an issue with the existing code, but we might as well fix that while we're changing it). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131304
Esme Povirk (@madewokherd) commented about dlls/gdiplus/gdiplus_private.h:
static inline BOOL has_gdi_dc(GpGraphics *graphics) { - return graphics->hdc != NULL || graphics->owndc; + return graphics->hdc != NULL || (graphics->owndc && IsWindow(graphics->hwnd));
This would add a non-obvious corner case. We generally assume that there are only a few valid Graphics object types: Metafile recording, Bitmap, HWND, and HDC, and that has_gdi_dc divides them cleanly (Metafile/Bitmap do not have a DC, and HWND/HDC do). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131305
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/graphics.c:
expect(3, glyphs); expect(1, lines);
+ DestroyWindow(hwnd2); + hwnd2 = NULL; + rect.X = 5.0; + rect.Y = 5.0; + rect.Width = width_M_M; + rect.Height = 32000.0; + status = GdipMeasureString(graphics2, string2, -1, font, &rect, format_no_wrap, &bounds, &glyphs, &lines); + todo_wine expect(Ok, status);
We should also test a drawing function. It's unclear whether native succeeds here because GdipMeasureString doesn't need to draw anything (and therefore in theory doesn't need to call GetDC), or because native is ignoring failures from it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131306
participants (3)
-
Esme Povirk (＠madewokherd) -
Yeshun Ye (＠yeyeshun)
-
YeshunYe