Closed
Bug 141818
Opened 22 years ago
Closed 20 years ago
Table with large rowspans and colspans hangs the browser
Categories
(Core :: Layout: Tables, defect, P3)
Tracking
()
VERIFIED
FIXED
Future
People
(Reporter: amar, Assigned: bernd_mozilla)
References
(Blocks 1 open bug, )
Details
(4 keywords, Whiteboard: [sg:dos] have patch)
Attachments
(7 files)
327 bytes,
text/html
|
Details | |
1.06 KB,
patch
|
karnaze
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
4.98 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
147.21 KB,
application/zip
|
Details | |
288 bytes,
text/html
|
Details | |
2.38 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
asa
:
approval1.7.5-
|
Details | Diff | Splinter Review |
2.39 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
dveditz
:
approval-aviary1.0.5+
dveditz
:
approval1.7.9+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/4.79 [en] (Windows NT 5.0; U) BuildID: In the following code the table has large rowspan="999999". When I load this table our browser hangs and utilise 100% memory. <html> <body> <table class="DataBg" border="0" cellpadding="0" cellspacing="1" > <td align="center" class="DataHeader" width="2" rowspan="9999999"> </td> <td align="center" class="DataHeader" colspan="3"><div class="margin3">Allocation Probability (%)</div></td> </tr> </table> </body> </html> Reproducible: Always Steps to Reproduce: 1.Load the testcase in the latest build 2.It hangs the browser. 3. Actual Results: Browser hangs and utilises 100% memory. Expected Results: Should not hang.
Reporter | ||
Comment 1•22 years ago
|
||
Comment 3•22 years ago
|
||
Mozilla 2002062808. Linux/i686. Bumping severity to major. This bug has a potential to make serious problems - mozilla allocates gobs of RAM and can easily trigger OOM killer in Linux, which in turn can take down the X session. Do not visit above URL if you have anything important running!
Comment 4•22 years ago
|
||
Same hang at http://www.ncbi.nlm.nih.gov/cgi-bin/COG/palox?AF2220, which uses rowspan=268580848. Mozilla grabbed about 400 MB before I killed it.
Updated•22 years ago
|
Target Milestone: --- → Future
Comment 6•22 years ago
|
||
*** Bug 182196 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Severity: major → critical
Attachment #110010 -
Flags: superreview?(roc+moz)
Attachment #110010 -
Flags: review?(karnaze)
Looks good, but there is one problem in celldata.h getting the rowspan: return (PRUint32)((mBits & ROW_SPAN_OFFSET) >> ROW_SPAN_SHIFT); mBits is signed and ROW_SPAN_OFFSET includes the sign bit, so the right shift will propagate the sign bit, so this will return the wrong result. I suggest you change the declaration of mBits to be PRUint32 instead of 'long'.
Assignee | ||
Comment 10•22 years ago
|
||
#include <stdio.h> #define foo 0xFFF80000 void main(void) { long l1; l1 = 0x80000000; printf("%ld", ((l1 & foo) >> 19) ); } This code returns on my machine 4096 as expected. I dont think we need to reorganize celldata. You would be right if math operations would be included but here this are pure bit manipulations. Further I would like to avoid to change the type. Currently tables also work on 64 bit wide machines, with your proposed change I would need to test it on such a machine, which I dont have.
Updated•22 years ago
|
Attachment #110010 -
Flags: review?(karnaze) → review+
#include <stdio.h> int main() { int i = 0x80000000; int foo = 0xFFF80000; printf("%d\n", i >> 16); printf("%d\n", (i & foo) >> 16); printf("%d\n", (i & 0xFFF80000) >> 16); } [roca@majesty tmp]$ gcc -o test test.c ; ./test -32768 -32768 32768 Unfortunately I don't have a copy of the C standard with me, but I believe the exact behaviour here is not specified. In this case perhaps 0xFFF80000 is being treated as an unsigned value and gcc decides that (signed)&(unsigned) should be unsigned. (Although that seems wrong to me; I would have expected 0xFFF80000 to be signed and 0xFFF80000U to be the unsigned version.) In that case the particular code you wrote is OK but I'm still not comfortable with it; it's too easy to get wrong by writing the code slightly differently in a way that everyone would believe "should" be equivalent. A different way to get around this would be to do the shift first and then apply the mask. I wouldn't worry about the 64-bit machines; I'm pretty sure PRUint32 will be fine there.
Assignee | ||
Comment 12•22 years ago
|
||
Attachment #111934 -
Flags: superreview?(roc+moz)
Comment on attachment 110010 [details] [diff] [review] patch sr=roc+moz
Attachment #110010 -
Flags: superreview?(roc+moz) → superreview+
Comment on attachment 111934 [details] [diff] [review] changes to celldata.h r+sr=roc+moz
Attachment #111934 -
Flags: superreview?(roc+moz)
Attachment #111934 -
Flags: superreview+
Attachment #111934 -
Flags: review+
Assignee | ||
Comment 15•22 years ago
|
||
fix checked in, it fixes the rowspan issue, but the url in this still locks the viewer due to the combination of a large rowspan and colspan value.
Comment 16•22 years ago
|
||
*** Bug 192229 has been marked as a duplicate of this bug. ***
Comment 17•22 years ago
|
||
This checkin has caused a regression in layout for 64-bit builds (Bug 194726).
Assignee | ||
Comment 18•21 years ago
|
||
*** Bug 203880 has been marked as a duplicate of this bug. ***
Comment 19•21 years ago
|
||
JFYI, don't have any problems neither with testcase nor with url with Mozilla 1.4 on Linux
Assignee | ||
Comment 20•21 years ago
|
||
*** Bug 229445 has been marked as a duplicate of this bug. ***
Comment 21•20 years ago
|
||
*** Bug 265846 has been marked as a duplicate of this bug. ***
Comment 22•20 years ago
|
||
*** Bug 267199 has been marked as a duplicate of this bug. ***
Comment 23•20 years ago
|
||
I have hit for the third time today a site defacement that used this bug to crash every Mozilla/Firefox user that accessed that page. I've also received a spam html mail some time ago which used this same bug (perhaps a revenge since we have a builtin junk filter?). I think it is about time to give some attention to this bug, since it is becoming well-known among lammers and script-kiddies that want to annoy mozilla users.
Flags: blocking1.8a5?
Comment 24•20 years ago
|
||
*** Bug 267634 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Flags: blocking1.8a5? → blocking1.8a5+
Comment 25•20 years ago
|
||
This WFM with the latest Firefox 1.0 build and Mozilla 1.8a5 trunk build on Windows. For those who do crash, please provide a talkback ID. Also, the URL given is no longer valid and the test case provided doesn't crash. So please provide what web sites (exact URL) you're crashing at.
Comment 26•20 years ago
|
||
AFAIK both patches attached to this bug report have been checked into the trunk.
Comment 27•20 years ago
|
||
Glad I didn't do a WFM on this. When using the example URL from Bug #267634 I got high CPU usage and an increasing amount of RAM allocated to the process. Killed the process when it hit 256Meg (I've got 1Gig installed). http://einsteinmg.dyndns.org/cgi-bin/remangle.cgi?=0x70b0e8ce WinXP SP2, Firefox 1.0
Comment 28•20 years ago
|
||
this testcase from the dupe bug 267199 also causes a hang for me (not a crash, so no talkback): https://bugzilla.mozilla.org/attachment.cgi?id=164215&action=view using Firefox 1.0 release build on win2000. As comment 15 said, it now needs both a high rowspan and colspan to cause the problem (which the URL in this bug had but the testcase doesn't)
Comment 29•20 years ago
|
||
Ouch!! With the latest provided testcase, seamonkey trunk hangs. Had to hard restart the system. Either the fixes didn't make it into the seamonkey trunk or they are inneffective.
Comment 30•20 years ago
|
||
Comment 31•20 years ago
|
||
You can view the profile at: jar:https://bugzilla.mozilla.org/attachment.cgi?id=165723&action=view!/jprof2.html Short summary: Total hit count: 77385 73036 nsCellMap::AppendCell That time is about evenly split between: 39147 operator new(unsigned) [mostly inside __libc_malloc here] and 20849 nsCellMap::SetDataAt(nsTableCellMap&, CellData&, int, int, int)
Assignee | ||
Comment 32•20 years ago
|
||
Assignee | ||
Comment 33•20 years ago
|
||
hmmm even opera does the same.
Comment 34•20 years ago
|
||
updating summary to reflect the issue this bug is now addressing (AIUI)
Summary: Table with large rowspans hangs the browser. → Table with large rowspans and colspans hangs the browser
Assignee | ||
Comment 35•20 years ago
|
||
Attachment #165779 -
Flags: superreview?(bzbarsky)
Attachment #165779 -
Flags: review?(bzbarsky)
Comment 36•20 years ago
|
||
Comment on attachment 165779 [details] [diff] [review] patch r+sr=bzbarsky
Attachment #165779 -
Flags: superreview?(bzbarsky)
Attachment #165779 -
Flags: superreview+
Attachment #165779 -
Flags: review?(bzbarsky)
Attachment #165779 -
Flags: review+
Assignee | ||
Comment 37•20 years ago
|
||
Comment on attachment 165779 [details] [diff] [review] patch fixed on trunk, this is IMHO branch worthy
Attachment #165779 -
Flags: approval1.7.x?
Comment 39•20 years ago
|
||
*** Bug 270427 has been marked as a duplicate of this bug. ***
Comment 40•20 years ago
|
||
Comment on attachment 165779 [details] [diff] [review] patch if this didn't go into Firefox 1.0's gecko, then it shouldn't land for 1.7.5
Attachment #165779 -
Flags: approval1.7.x? → approval1.7.x-
Assignee | ||
Comment 41•20 years ago
|
||
I am thick of that a+/- business, closing this bug if somebody want this checked in into any branch, 1. take the bug 2. ask for a 3. checked it in yourself. Trunk works. Game over.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 42•20 years ago
|
||
*** Bug 276139 has been marked as a duplicate of this bug. ***
Comment 43•19 years ago
|
||
*** Bug 291402 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Flags: blocking1.7.8?
Flags: blocking-aviary1.0.4?
Whiteboard: [sg:dos]
Comment 44•19 years ago
|
||
Lot of dupes on this one recently, might be worth taking as a stability fix on the branches
Flags: blocking1.7.8?
Flags: blocking1.7.8+
Flags: blocking-aviary1.0.4?
Flags: blocking-aviary1.0.4+
Comment 45•19 years ago
|
||
*** Bug 293011 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Flags: blocking-aviary1.0.4?
Updated•19 years ago
|
Flags: blocking-aviary1.0.4? → blocking-aviary1.0.4-
Updated•19 years ago
|
Whiteboard: [sg:dos] → [sg:dos] have patch
Comment 46•19 years ago
|
||
Bernd: This is +'d for 1.0.5, can you put together a patch for the Aviary branch?
Assignee | ||
Comment 47•19 years ago
|
||
Attachment #185587 -
Flags: approval1.7.9?
Attachment #185587 -
Flags: approval-aviary1.0.5?
Comment 48•19 years ago
|
||
Since bz is on vacation, can someone please r+sr the branch patch so we can approve it for 1.0.5 and 1.7.9? Dveditz, jst?
Assignee | ||
Comment 49•19 years ago
|
||
the branch patch is identical to the trunk version, so I don't see why an additional r/sr should be necessary
Updated•19 years ago
|
Attachment #185587 -
Flags: superreview+
Attachment #185587 -
Flags: review+
Comment 50•19 years ago
|
||
bernd: Sorry, I realized that after I posted the comment. dveditz: I don't have permission to approve, so can you do the a= so we can get this checked in? Thanks.
Comment 51•19 years ago
|
||
Comment on attachment 185587 [details] [diff] [review] patch updated to branch a=dveditz for 1.0.5/1.7.9
Attachment #185587 -
Flags: approval1.7.9?
Attachment #185587 -
Flags: approval1.7.9+
Attachment #185587 -
Flags: approval-aviary1.0.5?
Attachment #185587 -
Flags: approval-aviary1.0.5+
Comment 52•19 years ago
|
||
Please add the fixed-aviary1.0.5 and fixed1.7.9 keywords when this lands on the branches.
Flags: blocking1.7.8+ → blocking1.7.9+
Assignee | ||
Comment 53•19 years ago
|
||
fix checked in on branches
Keywords: fixed-aviary1.0.5,
fixed1.7.9
Comment 54•19 years ago
|
||
I wasn't able to crash firefox when viewing the test cases here (the ones that didn't yield 404 errors). verifying as fixed with 200506170x-1.0.5 firefox builds on linux fc3 and mac os x 10.4.1.
Status: RESOLVED → VERIFIED
Comment 55•19 years ago
|
||
v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9) Gecko/20050706 Firefox/1.0.5 using attached testcase.
You need to log in
before you can comment on or make changes to this bug.
Description
•