[Webkit-unassigned] [Bug 53081] New: Need to explicitly check non-finite value in input coordinate in Canvas2D::isPointInPath

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53081

           Summary: Need to explicitly check non-finite value in input
                    coordinate in Canvas2D::isPointInPath
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Mac OS X 10.5
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Canvas
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: jnd at chromium.org
                CC: mdelaney at apple.com, klobag at chromium.org


In test canvas/philip/tests/2d.path.isPointInPath.nonfinite.html, we have test code to check the non-finite coordinate as listed below,
ctx.rect(-100, -50, 200, 100);
_assertSame(ctx.isPointInPath(NaN, 0), false, "ctx.isPointInPath(NaN, 0)", "false");
_assertSame(ctx.isPointInPath(0, NaN), false, "ctx.isPointInPath(0, NaN)", "false");
_assertSame(ctx.isPointInPath(NaN, NaN), false, "ctx.isPointInPath(NaN, NaN)", "false");

In all above tests, the input coordinates contain non-finite value, currently we leave them to let the following platform implementation (Path::contains) handle the non-finite value.
However, most of platform implementation don't explicit check the non-finite value.

Let's see the Skia implementation Path::contains (WebCore/platform/graphics/skia/PathSkia.cpp), it calls SkPathContainsPoint to do the real work.
In SkPathContainsPoint (WebCore/platform/graphics/skia/SkiaUtils.cpp), it doesn't check non-finite value, like NaN, then when executing on line 191-192, it converts float value to int, see the following code.
int x = static_cast<int>(floorf(point.x() / scale));
int y = static_cast<int>(floorf(point.y() / scale));

Unfortunately, doing NaN-to-int casting with static_cast<int>(float) returns different results on different platforms, See the following code

int i = static_cast<float>(d);
printf("INT: %08x\n", i)

On Mac, Linux & Windows, i is set to 0x80000000, on Android, i is set to 0.
So the test 2d.path.isPointInPath.nonfinite.html is passed on Mac, Linux & Windows, but failed on Android because point(0,0) is indeed in the rect

We know NaN(QNaN) means uncertain operation. we can not trust and use the result from operating NaN or other non-finite number, we need to explicitly check non-finite value in input coordinate.

We can add the check in  place.
(1) CanvasRenderingContext2D::isPointInPath, it's a root when calling CanvasContext at d.isPointInPath from JS binding.
(2) Path::contains, add the check logic in each Path::contains implementation.
(3) Platfrom Implementation, like SkPathContainsPoint, CGPathContainsPoint, ..., etc. which can cover other calls which are not from JS binding.

My first patch will use method (1) but may be changed according to the review comments.

Thanks

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.