Bug 32357 - SVG test case crashes WebKit (invalid font URL)
Summary: SVG test case crashes WebKit (invalid font URL)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.5
: P1 Major
Assignee: Nobody
URL: http://dev.w3.org/SVG/profiles/1.2T/t...
Keywords:
: 32712 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-12-09 18:00 PST by Michael[tm] Smith
Modified: 2009-12-18 08:28 PST (History)
6 users (show)

See Also:


Attachments
Proposed patch (737 bytes, patch)
2009-12-14 11:59 PST, Justin Schuh
no flags Details | Formatted Diff | Diff
Minimized test case (559 bytes, image/svg+xml)
2009-12-14 12:00 PST, Justin Schuh
no flags Details
Patch with layout tests and changelog (4.27 KB, patch)
2009-12-17 08:23 PST, Justin Schuh
mitz: review+
Details | Formatted Diff | Diff
Patch with layout tests (4.01 KB, patch)
2009-12-17 12:53 PST, Justin Schuh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael[tm] Smith 2009-12-09 18:00:36 PST
Running  r51881. Tried to open http://dev.w3.org/SVG/profiles/1.2T/test/svgHarness/animate-elem-227-t.svg but crashes.
Comment 1 Alexey Proskuryakov 2009-12-10 14:27:15 PST
SVGFontFaceUriElement::loadFont() tries to call setSVGFont on a null m_cachedFont. And m_cachedFont is null because we're trying to resolve "../images/SVGFreeSans.svg#ascii" in an about:blank document, and of course fail.

It's definitely a bug loadFont() that it crashes when font URL is invalid. It may be a bug elsewhere that the document base URL is about:blank.
Comment 2 Justin Schuh 2009-12-14 11:59:21 PST
Created attachment 44810 [details]
Proposed patch

Checks for NULL m_cachedFont before calling setSVGFont().
Comment 3 Justin Schuh 2009-12-14 12:00:24 PST
Created attachment 44811 [details]
Minimized test case
Comment 4 Justin Schuh 2009-12-14 12:02:24 PST
I ran into the same thing last week <http://crbug.com/29890>. Here's a short patch and a minimized test case.
Comment 5 Alexey Proskuryakov 2009-12-14 12:21:57 PST
Would you be willing to submit a patch for review, as described in <http://webkit.org/coding/contributing.html>?
Comment 6 Justin Schuh 2009-12-16 09:18:03 PST
(In reply to comment #5)
> Would you be willing to submit a patch for review, as described in
> <http://webkit.org/coding/contributing.html>?

Yep. I had to set up a proper WebKit build environment, but I'll be submitting a patch today.
Comment 7 Justin Schuh 2009-12-17 08:23:12 PST
Created attachment 45072 [details]
Patch with layout tests and changelog

This patch just checks for a NULL m_cachedFont before continuing. It follows the submission guidelines and should be ready for review.
Comment 8 WebKit Review Bot 2009-12-17 08:25:42 PST
style-queue ran check-webkit-style on attachment 45072 [details] without any errors.
Comment 9 mitz 2009-12-17 08:35:48 PST
Comment on attachment 45072 [details]
Patch with layout tests and changelog

> +    This test is to ensure that we do not crash when loading a SVG image without an invalid font-face-uri

Did you mean “*with* an invalid”? I think this kind of test can be done entirely in SVG.
Comment 10 Justin Schuh 2009-12-17 09:03:02 PST
(In reply to comment #9)
> Did you mean “*with* an invalid”? 

Yes I did. I'll fix that before resubmitting.

> I think this kind of test can be done entirely in SVG.

I don't know, but probably. This is my first crack at submitting a patch. So, I copied format and style from the text-font-invalid.html test, which looked similar to this case.
Comment 11 Justin Schuh 2009-12-17 12:53:52 PST
Created attachment 45104 [details]
Patch with layout tests

I fixed the typo and condensed SVG and HTML into a single file. There's still an expected output file, because that appears to be how other crash tests were done.
Comment 12 WebKit Review Bot 2009-12-17 12:56:51 PST
style-queue ran check-webkit-style on attachment 45104 [details] without any errors.
Comment 13 WebKit Commit Bot 2009-12-17 22:12:43 PST
Comment on attachment 45104 [details]
Patch with layout tests

Clearing flags on attachment: 45104

Committed r52300: <http://trac.webkit.org/changeset/52300>
Comment 14 WebKit Commit Bot 2009-12-17 22:12:48 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 mitz 2009-12-18 08:28:41 PST
*** Bug 32712 has been marked as a duplicate of this bug. ***