| Summary: | SVG test case crashes WebKit (invalid font URL) | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | sideshowbarker <mike> | ||||||||||
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Major | CC: | ap, commit-queue, cshu, jschuh, krit, webkit.review.bot | ||||||||||
| Priority: | P1 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Mac (Intel) | ||||||||||||
| OS: | OS X 10.5 | ||||||||||||
| URL: | http://dev.w3.org/SVG/profiles/1.2T/test/svgHarness/animate-elem-227-t.svg | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
sideshowbarker
2009-12-09 18:00:36 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. Created attachment 44810 [details]
Proposed patch
Checks for NULL m_cachedFont before calling setSVGFont().
Created attachment 44811 [details]
Minimized test case
I ran into the same thing last week <http://crbug.com/29890>. Here's a short patch and a minimized test case. Would you be willing to submit a patch for review, as described in <http://webkit.org/coding/contributing.html>? (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. 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.
style-queue ran check-webkit-style on attachment 45072 [details] without any errors.
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. (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. 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.
style-queue ran check-webkit-style on attachment 45104 [details] without any errors.
Comment on attachment 45104 [details] Patch with layout tests Clearing flags on attachment: 45104 Committed r52300: <http://trac.webkit.org/changeset/52300> All reviewed patches have been landed. Closing bug. *** Bug 32712 has been marked as a duplicate of this bug. *** |
