Summary: | Invalid CSS code crashes Safari | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Swiecki <robert.swiecki+wkbugs> | ||||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aroben, bdakin, gwilson | ||||||||
Priority: | P2 | Keywords: | HasReduction, InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows XP | ||||||||||
URL: | http://alt.swiecki.net/s-crash6.html | ||||||||||
Attachments: |
|
Description
Robert Swiecki
2008-08-25 09:05:12 PDT
Created attachment 22982 [details] Test reduction for bug 20512 Attached is a possible test reduction of this issue. I believe that this comes from the parsing of a CSS "counter" with invalid content....investigating further. This one line of CSS by itself will crash Safari, but not IE or FF3. Then it might be a duplicate of: https://bugs.webkit.org/show_bug.cgi?id=20396 Created attachment 23507 [details]
Patch to prevent function input to counters
I can imagine a number of different ways to fix this bug, but here is a simple one that comes to mind. It seems like we are crashing because we are trying to convert a function to a string.
Comment on attachment 23507 [details]
Patch to prevent function input to counters
Is it worthwhile to add another testcase that uses a function that WebKit's CSS parser knows about? Like -webkit-gradient or something like that?
r=me
Sure! I'll add one now. Comment on attachment 23507 [details]
Patch to prevent function input to counters
That seems a little backwards. Maybe a better check would be:
if (i->unit != CSSPrimitiveValue::CSS_STRING)
Is there a reason to not do it that way?
Hmmm…good question, Darin! I am running the layout tests with your recommended change now. Created attachment 23512 [details]
Better fix
Here's a better fix based on Darin's comment. Just checking against string is not sufficient since numbers are also okay.
Re-opening bug since I have a new patch to be reviewed. Comment on attachment 23512 [details]
Better fix
r=me
Fixed again with r36567. |