Summary: | Chrome/Safari Crashes on SVG Image | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | W. James MacLean <wjmaclean> | ||||||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED WORKSFORME | ||||||||||
Severity: | Normal | CC: | eric, inferno, jschuh, krit, levin, rwlbuis, simon.fraser, zimmermann | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
W. James MacLean
2010-06-18 08:33:21 PDT
Created attachment 59127 [details]
Proposed patch
This patch is marked for review to discuss the approach to fixing the bug, and does not (yet) include a simplified test and changelog comments.
The bug appears to occur in
SVGRootInlineBox::layoutInlineBoxes(InlineFlowBox* start, Vector<SVGChar>::iterator& it, int& lowX, int& highX, int& lowY, int& highY)
when a non-text InlineFlowBox is encountered that has no children. In this case the default values
int minX = INT_MAX;
int minY = INT_MAX;
int maxX = INT_MIN;
int maxY = INT_MIN;
are retained and lead to the current InlineBox being assigned unrealistically large (x,y) coordinates.
These coordinates then lead to unrealistically large bounding rects, causing the platform-level renderers to fail.
The proposed patch exits, leaving the default size for the InlineBox, which appears to work (performance may be slow, but this appears to be a separate issue with the platform-level filters).
Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ... (In reply to comment #2) > Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ... Hm, it works fine for me w/o any performance and/or crash issues? Using Safari4 on 10.5.8. (In reply to comment #3) > (In reply to comment #2) > > Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ... > > Hm, it works fine for me w/o any performance and/or crash issues? Using Safari4 on 10.5.8. Can't reproduce it either. Neither in WebKitGtk nor in Chromium-linux. Comment on attachment 59127 [details] Proposed patch Thanks for the patch, but we'll need a ChangeLog describing why you're making this change (and linking to this bug) as well as a test to make sure we don't screw this up in the future. You can find more information about submitting patch here: http://webkit.org/coding/contributing.html W. James, you might want to use http://upload.wikimedia.org/wikipedia/commons/d/d9/Unix_history-simple.en.svg (from security bug https://bugs.webkit.org/show_bug.cgi?id=39253) as your testcase since it reliably crashes every time i use it :). Dirk is already taking a look at 39253. Also have a quick question, do you see any security issue like memory corruption, etc inside debugger. when we were testing bug 39253, we could also reproduce a null ptr exception arising from a imagebuffer can't being allocated from large size. If that is the case, we can unhide/remove security flags from 39253 as well. Otherwise, we need to mark this bug as security and hide it. (In reply to comment #6) > W. James, you might want to use http://upload.wikimedia.org/wikipedia/commons/d/d9/Unix_history-simple.en.svg Thanks. This was the image I intended to use, but I must have gotten the wrong link. I've been find the latest pull from the repository doesn't seem to have the crashing problem anymore, although there is still a large performance hit (at least on Chrome-Linux). I'll compare the two images to see if there's a difference that points us towards the problem. I guess there is the possibility of a null pointer exception in release builds - the debug builds die on an assert (at least in older versions of the code that had the problem). Created attachment 59516 [details]
SVG that (may) crash and/or cause performance issues.
Revised SVG image that (may) cause crash/performance issues.
Seems fine in a recent nightly. Checked again, works fine nowadays. |