Bug 14714

Summary: REGRESSION: ASSERTION FAILED: i < size() in Vector.h:401 on negative -webkit-column-width
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: DOMAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, darin, hyatt, joost, mitz
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Test case
none
Patch v1
darin: review-
Patch in render tree darin: review+

David Kilzer (:ddkilzer)
Reported 2007-07-22 11:32:59 PDT
* SUMMARY A reproducible assertion failure occurs with some mangled HTML on debug builds of ToT WebKit. * STEPS TO REPRODUCE 1. Launch Safari/WebKit with a debug build of WebKit. 2. Open attached test case. * RESULTS Safari crashes with an assertion failure. * REGRESSION Does NOT crash Safari 3.0.2 (522.12) public beta with its own WebKit on Mac OS X 10.4.10 (8R218). DOES crash Safari 3.0.2 (522.12) with a local debug build of WebKit r24513 on Mac OS X 10.4.10 (8R218).
Attachments
Test case (45 bytes, text/html)
2007-07-22 11:33 PDT, David Kilzer (:ddkilzer) 		no flags
Details
Patch v1 (3.21 KB, patch)
2007-07-22 14:43 PDT, David Kilzer (:ddkilzer) 		darin: review-
DetailsFormatted DiffDiff
Patch in render tree (7.62 KB, patch)
2007-07-24 11:59 PDT, Beth Dakin 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2007-07-22 11:33:40 PDT
Created attachment 15625 [details] Test case
David Kilzer (:ddkilzer)
Comment 2 2007-07-22 11:36:50 PDT
(In reply to comment #0) > * REGRESSION > Does NOT crash Safari 3.0.2 (522.12) public beta with its own WebKit on Mac OS > X 10.4.10 (8R218). > > DOES crash Safari 3.0.2 (522.12) with a local debug build of WebKit r24513 on > Mac OS X 10.4.10 (8R218). DOES crash WebKit nightly r24513 with Safari 3.0 (522.12) on Mac OS X 10.4.10 (8R218). Version: r24513 (24513) PID: 3638 Thread: 0 Exception: EXC_BAD_ACCESS (0x0001) Codes: KERN_INVALID_ADDRESS (0x0001) at 0xfffffff0 Thread 0 Crashed: 0 com.apple.WebCore 0x011437dc WebCore::RenderBlock::rightmostPosition(bool, bool) const + 508 1 com.apple.WebCore 0x011a7aec WebCore::RenderTableSection::rightmostPosition(bool, bool) const + 188 2 com.apple.WebCore 0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312 3 com.apple.WebCore 0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40 4 com.apple.WebCore 0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312 5 com.apple.WebCore 0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40 6 com.apple.WebCore 0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312 7 com.apple.WebCore 0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40 8 com.apple.WebCore 0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312 9 com.apple.WebCore 0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40 10 com.apple.WebCore 0x011546b4 WebCore::RenderView::docWidth() const + 84 11 com.apple.WebCore 0x011552a0 WebCore::RenderView::layout() + 288 12 com.apple.WebCore 0x010e9344 WebCore::FrameView::layout(bool) + 1364 13 com.apple.WebCore 0x010f3584 WebCore::Document::implicitClose() + 788 14 com.apple.WebCore 0x0142714c WebCore::FrameLoader::checkCallImplicitClose() + 348 15 com.apple.WebCore 0x014363d4 WebCore::FrameLoader::checkCompleted() + 228 16 com.apple.WebCore 0x014378e8 WebCore::FrameLoader::finishedParsing() + 104 17 com.apple.WebCore 0x01021d7c WebCore::HTMLTokenizer::end() + 188 18 com.apple.WebCore 0x01022228 WebCore::HTMLTokenizer::finish() + 1160 19 com.apple.WebCore 0x0143943c WebCore::FrameLoader::endIfNotLoadingMainResource() + 124 20 com.apple.WebCore 0x0143165c WebCore::FrameLoader::finishedLoading() + 92 21 com.apple.WebCore 0x01443448 WebCore::MainResourceLoader::didFinishLoading() + 56 22 com.apple.WebCore 0x01415454 -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 84 23 com.apple.Foundation 0x92c1489c -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback] + 188 24 com.apple.Foundation 0x92c12b08 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 556 25 com.apple.Foundation 0x92c12860 _sendCallbacks + 156 26 com.apple.CoreFoundation 0x907de4fc __CFRunLoopDoSources0 + 384 27 com.apple.CoreFoundation 0x907dda2c __CFRunLoopRun + 452 28 com.apple.CoreFoundation 0x907dd4ac CFRunLoopRunSpecific + 268 29 com.apple.HIToolbox 0x9329ab20 RunCurrentEventLoopInMode + 264 30 com.apple.HIToolbox 0x9329a1b4 ReceiveNextEventCommon + 380 31 com.apple.HIToolbox 0x9329a020 BlockUntilNextEventMatchingListInMode + 96 32 com.apple.AppKit 0x937a0ae4 _DPSNextEvent + 384 33 com.apple.AppKit 0x937a07a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 34 com.apple.Safari 0x00006740 0x1000 + 22336 35 com.apple.AppKit 0x9379ccec -[NSApplication run] + 472 36 com.apple.AppKit 0x9388d87c NSApplicationMain + 452 37 com.apple.Safari 0x0005c77c 0x1000 + 374652 38 com.apple.Safari 0x0005c624 0x1000 + 374308
David Kilzer (:ddkilzer)
Comment 3 2007-07-22 11:37:38 PDT
<rdar://problem/5352884>
David Kilzer (:ddkilzer)
Comment 4 2007-07-22 11:38:35 PDT
Regression per Comment #2.
David Kilzer (:ddkilzer)
Comment 5 2007-07-22 12:29:23 PDT
I have a fix for this. Basically, -webkit-column-width should not allow negative widths.
David Kilzer (:ddkilzer)
Comment 6 2007-07-22 14:43:51 PDT
Created attachment 15628 [details] Patch v1 Proposed patch. Don't allow negative values for -webkit-column-width. NOTE TO REVIEWER: I did not trace through the render code to determine why negative values failed. I simply added the FNonNeg flag when parsing -webkit-column-width values to prevent non-negative numbers from being recognized as valid.
David Kilzer (:ddkilzer)
Comment 7 2007-07-22 15:29:16 PDT
(In reply to comment #6) > Created an attachment (id=15628) [edit] > Patch v1 > > Proposed patch. Don't allow negative values for -webkit-column-width. > > NOTE TO REVIEWER: I did not trace through the render code to determine why > negative values failed. I simply added the FNonNeg flag when parsing > -webkit-column-width values to prevent non-negative numbers from being > recognized as valid. I should also note that all layout tests passed.
David Kilzer (:ddkilzer)
Comment 8 2007-07-22 15:37:50 PDT
Note that this test case was derived from the original iExploder test case in Bug 14715.
David Kilzer (:ddkilzer)
Comment 9 2007-07-22 20:12:44 PDT
See also Bug 14718.
Darin Adler
Comment 10 2007-07-24 08:46:06 PDT
Comment on attachment 15628 [details] Patch v1 The CSS specification doesn't say anything about column widths needing to be positive, and I also think a column width of 0 could be a problem. I think the fix for this should be in RenderBlock::calcColumnWidth. Setting review- to keep the ball rolling on this. Hyatt should really take a look.
Beth Dakin
Comment 11 2007-07-24 11:13:32 PDT
Hyatt has mentioned that he would really like to pull our current implementation of column layout before Leopard ships. I wonder if that is how he will want to fix this bug. Looking into it in the meantime anyway.
Beth Dakin
Comment 12 2007-07-24 11:33:26 PDT
If we decide not to fix this by yanking column layout entirely, this will be easy to fix, but we still need to decide on a behavior change. As Darin mentioned, the CSS spec doesn't seem to address the issue, but a negative column width doesn't make any sense to me. I tend to think we should treat all column widths that are less than 1 as 1.
Beth Dakin
Comment 13 2007-07-24 11:46:58 PDT
I have a patch. Just putting it together with the layout tests.
Beth Dakin
Comment 14 2007-07-24 11:59:43 PDT
Created attachment 15668 [details] Patch in render tree
Darin Adler
Comment 15 2007-07-24 12:03:46 PDT
Comment on attachment 15668 [details] Patch in render tree r=me
Beth Dakin
Comment 16 2007-07-24 12:06:28 PDT
Fixed with r24599.
Beth Dakin
Comment 17 2007-07-24 12:07:00 PDT
*** Bug 14718 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.