[PATCH 0/2] MR3461: gdiplus: Reject zero-width/zero-height rectangles
Works around the issue manifesting here : https://bugs.winehq.org/show_bug.cgi?id=55351 The MR also contains a patch that refactors the related tests a bit. Signed-off-by: David Kahurani k.kahurani(a)gmail.com -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3461
From: David Kahurani <k.kahurani(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55351 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/graphicspath.c | 20 ++++++++++---------- dlls/gdiplus/tests/graphicspath.c | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 545aaa8441e..a142596c4c8 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -283,10 +283,10 @@ static GpStatus extend_current_figure(GpPath *path, GDIPCONST PointF *points, IN * * PARAMS * path [I/O] Path that the arc is appended to - * x1 [I] X coordinate of the boundary box - * y1 [I] Y coordinate of the boundary box - * x2 [I] Width of the boundary box - * y2 [I] Height of the boundary box + * x1 [I] X coordinate of the boundary rectangle + * y1 [I] Y coordinate of the boundary rectangle + * width [I] Width of the boundary rectangle + * height [I] Height of the boundary rectangle * startAngle [I] Starting angle of the arc, clockwise * sweepAngle [I] Angle of the arc, clockwise * @@ -302,20 +302,20 @@ static GpStatus extend_current_figure(GpPath *path, GDIPCONST PointF *points, IN * In both cases, the value of newfigure of the given path is FALSE * afterwards. */ -GpStatus WINGDIPAPI GdipAddPathArc(GpPath *path, REAL x1, REAL y1, REAL x2, - REAL y2, REAL startAngle, REAL sweepAngle) +GpStatus WINGDIPAPI GdipAddPathArc(GpPath *path, REAL x1, REAL y1, REAL width, + REAL height, REAL startAngle, REAL sweepAngle) { GpPointF *points; GpStatus status; INT count; TRACE("(%p, %.2f, %.2f, %.2f, %.2f, %.2f, %.2f)\n", - path, x1, y1, x2, y2, startAngle, sweepAngle); + path, x1, y1, width, height, startAngle, sweepAngle); - if(!path) + if(!path || !width || !height) return InvalidParameter; - count = arc2polybezier(NULL, x1, y1, x2, y2, startAngle, sweepAngle); + count = arc2polybezier(NULL, x1, y1, width, height, startAngle, sweepAngle); if(count == 0) return Ok; @@ -323,7 +323,7 @@ GpStatus WINGDIPAPI GdipAddPathArc(GpPath *path, REAL x1, REAL y1, REAL x2, if(!points) return OutOfMemory; - arc2polybezier(points, x1, y1, x2, y2, startAngle, sweepAngle); + arc2polybezier(points, x1, y1, width, height, startAngle, sweepAngle); status = extend_current_figure(path, points, count, PathPointTypeBezier); diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c index 558eedc147f..4bbddfbe5b8 100644 --- a/dlls/gdiplus/tests/graphicspath.c +++ b/dlls/gdiplus/tests/graphicspath.c @@ -1266,7 +1266,7 @@ static void test_flatten2(void) /* path seen in the wild that caused a stack overflow */ status = GdipAddPathArc(path, -136.33, 20.00, 786.00, 786.00, -105.00, 30.00); expect(Ok, status); - status = GdipAddPathArc(path, 256.67, 413.00, 0.00, 0.00, -75.00, -30.00); + status = GdipAddPathArc(path, 256.67, 413.00, 1.00, 1.00, -75.00, -30.00); expect(Ok, status); status = GdipClosePathFigure(path); expect(Ok, status); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3461
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/tests/graphicspath.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c index 4bbddfbe5b8..7904ca9533a 100644 --- a/dlls/gdiplus/tests/graphicspath.c +++ b/dlls/gdiplus/tests/graphicspath.c @@ -1249,16 +1249,7 @@ static void test_flatten(void) expect(Ok, status); ok_path(path, flattenquater_path, ARRAY_SIZE(flattenquater_path), FALSE); - GdipDeleteMatrix(m); - GdipDeletePath(path); -} - -static void test_flatten2(void) -{ - GpStatus status; - GpPath *path; - - status = GdipCreatePath(0, &path); + status = GdipResetPath(path); expect(Ok, status); status = GdipStartPathFigure(path); expect(Ok, status); @@ -1285,7 +1276,9 @@ static void test_flatten2(void) status = GdipClosePathFigure(path); expect(Ok, status); status = GdipFlattenPath(path, NULL, 1.0); + expect(Ok, status); + GdipDeleteMatrix(m); GdipDeletePath(path); } @@ -1971,7 +1964,6 @@ START_TEST(graphicspath) test_widen_cap(); test_isvisible(); test_empty_rect(); - test_flatten2(); GdiplusShutdown(gdiplusToken); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3461
Jeffrey Smith (@whydoubt) commented about dlls/gdiplus/graphicspath.c:
* * PARAMS * path [I/O] Path that the arc is appended to - * x1 [I] X coordinate of the boundary box - * y1 [I] Y coordinate of the boundary box - * x2 [I] Width of the boundary box - * y2 [I] Height of the boundary box + * x1 [I] X coordinate of the boundary rectangle + * y1 [I] Y coordinate of the boundary rectangle With renaming `x2` and `y2` to `width` and `height`, `x1` and `x2` should be renamed `x` and `y`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3461#note_40760
Jeffrey Smith (@whydoubt) commented about dlls/gdiplus/tests/graphicspath.c:
/* path seen in the wild that caused a stack overflow */ status = GdipAddPathArc(path, -136.33, 20.00, 786.00, 786.00, -105.00, 30.00); expect(Ok, status); - status = GdipAddPathArc(path, 256.67, 413.00, 0.00, 0.00, -75.00, -30.00); + status = GdipAddPathArc(path, 256.67, 413.00, 1.00, 1.00, -75.00, -30.00);
Unless it is causing a fault on Windows, I think there should still be a test for the zero-width/zero-height case. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3461#note_40763
On Mon Jul 31 11:54:46 2023 +0000, Jeffrey Smith wrote:
With renaming `x2` and `y2` to `width` and `height`, `x1` and `x2` should be renamed `x` and `y`. True
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3461#note_40813
On Mon Jul 31 12:05:36 2023 +0000, Jeffrey Smith wrote:
Unless it is causing a fault on Windows, I think there should still be a test for the zero-width/zero-height case. Alright, I'll add one. I reckon we need to change the expected return type to return failure(InvalidParameter).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3461#note_40814
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Jeffrey Smith (＠whydoubt)