Bug 143903 - SVGFitToViewBox::viewBoxToViewTransform() has to count for zero physical width and height before calling SVGPreserveAspectRatio::getCTM()
Summary: SVGFitToViewBox::viewBoxToViewTransform() has to count for zero physical widt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-17 16:34 PDT by Said Abou-Hallawa
Modified: 2015-04-20 13:46 PDT (History)
3 users (show)

See Also:


Attachments
Test case (will crash on Mac) (244 bytes, text/html)
2015-04-17 16:34 PDT, Said Abou-Hallawa
no flags Details
Patch (4.66 KB, patch)
2015-04-17 17:13 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.66 KB, patch)
2015-04-17 18:57 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (5.82 KB, patch)
2015-04-20 11:19 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (5.74 KB, patch)
2015-04-20 11:53 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (5.74 KB, patch)
2015-04-20 12:48 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2015-04-17 16:34:19 PDT
Created attachment 251068 [details]
Test case (will crash on Mac)

If SVGPreserveAspectRatio::getCTM() receives zero width or height, the returned transformation might have a=0, d=0 or e=f=NaN. This makes the transformation is non-invertible. CG path functions assert and might crash, if they encounter such transformation.
Comment 1 Said Abou-Hallawa 2015-04-17 17:13:54 PDT
Created attachment 251075 [details]
Patch
Comment 2 Said Abou-Hallawa 2015-04-17 18:57:09 PDT
Created attachment 251078 [details]
Patch
Comment 3 Said Abou-Hallawa 2015-04-20 11:19:36 PDT
Created attachment 251168 [details]
Patch
Comment 4 Said Abou-Hallawa 2015-04-20 11:33:00 PDT
* If an SVG has the following root element:

<svg viewBox="0 0 10000000000000000000000000000000000000 1" width="1" height="1">

where the width = 1e+38. The viewBoxToView transformation will still have an invertible matrix without this patch. The scaling will be 1e-38 which is a valid float value.

* If we increase the width by making it 1e+39 like the following: 

<svg viewBox="0 0 100000000000000000000000000000000000000 1" width="1" height="1">

The viewBox will be all [0 0 0 0]. The reason is SVGFitToViewBox::parseViewBox() calls parseNumber() which calls genericParseNumber(). The later function bails out early if the number overflows. So this case is also fine without this patch.

* If we explicitly say the width is zero like the following: 

<svg viewBox="0 0 100 1" width="0" height="1">

the viewBoxToView transform was not be invertible but the good thing is this SVG will be omitted from display because the width is zero. So without this patch, we were not going to crash even though the calculation was wrong. With this patch, the viewBoxToView transform will be the identity but this has no effect on the display since the SVG will not be displayed anyway.
Comment 5 Daniel Bates 2015-04-20 11:44:54 PDT
Comment on attachment 251168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251168&action=review

> Source/WebCore/ChangeLog:3
> +        SVGFitToViewBox::viewBoxToViewTransform() has to count for zero physical width and height before calling SVGPreserveAspectRatio::getCTM().

Minor: Remove the period at the end of this line to match the bug title.

> Source/WebCore/ChangeLog:10
> +        invertible matrix

Missing period at the end of this sentence.
Comment 6 Said Abou-Hallawa 2015-04-20 11:53:44 PDT
Created attachment 251178 [details]
Patch
Comment 7 WebKit Commit Bot 2015-04-20 12:43:44 PDT
Comment on attachment 251178 [details]
Patch

Rejecting attachment 251178 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 251178, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/5079594059169792
Comment 8 Said Abou-Hallawa 2015-04-20 12:48:35 PDT
Created attachment 251183 [details]
Patch
Comment 9 WebKit Commit Bot 2015-04-20 13:44:20 PDT
Comment on attachment 251183 [details]
Patch

Clearing flags on attachment: 251183

Committed r183026: <http://trac.webkit.org/changeset/183026>
Comment 10 WebKit Commit Bot 2015-04-20 13:44:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Said Abou-Hallawa 2015-04-20 13:46:30 PDT
<rdar://problem/19005137>