SVGFitToViewBox::viewBoxToViewTransform() has to count for zero physical width and height before calling SVGPreserveAspectRatio::getCTM()

• Component: SVG
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

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.

[Said Abou-Hallawa]

Created attachment 251075 [details]
Patch

[Said Abou-Hallawa]

Created attachment 251078 [details]
Patch

[Said Abou-Hallawa]

Created attachment 251168 [details]
Patch

[Said Abou-Hallawa]

* 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.

[Daniel Bates]

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.

[Said Abou-Hallawa]

Created attachment 251178 [details]
Patch

[WebKit Commit Bot]

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

[Said Abou-Hallawa]

Created attachment 251183 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 251183 [details]
Patch

Clearing flags on attachment: 251183

Committed r183026: <http://trac.webkit.org/changeset/183026>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

<rdar://problem/19005137>