Last Comment Bug 141818 - Table with large rowspans and colspans hangs the browser
: Table with large rowspans and colspans hangs the browser
Status: VERIFIED FIXED
[sg:dos] have patch
: fixed-aviary1.0.5, fixed1.7.9, hang, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 All
: P3 critical with 3 votes (vote)
: Future
Assigned To: Bernd
: Amarendra Hanumanula
:
Mentors:
http://info.pruszkow.org/~krzynio/cra...
: 182196 192229 203880 229445 265846 267199 267634 270427 276139 291402 293011 (view as bug list)
Depends on:
Blocks: 234240
  Show dependency treegraph
 
Reported: 2002-05-02 12:00 PDT by Amarendra Hanumanula
Modified: 2014-04-26 03:31 PDT (History)
27 users (show)
dveditz: blocking1.7.9+
dveditz: blocking‑aviary1.0.4-
dveditz: blocking‑aviary1.0.5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase attached (327 bytes, text/html)
2002-05-02 12:01 PDT, Amarendra Hanumanula
no flags Details
patch (1.06 KB, patch)
2002-12-23 10:37 PST, Bernd
karnaze: review+
roc: superreview+
Details | Diff | Splinter Review
changes to celldata.h (4.98 KB, patch)
2003-01-18 12:33 PST, Bernd
roc: review+
roc: superreview+
Details | Diff | Splinter Review
Zipped-up profile of the minimal testcase bernd asked me to profile (147.21 KB, application/zip)
2004-11-12 10:45 PST, Boris Zbarsky [:bz] (still a bit busy)
no flags Details
IE large colspan protection (288 bytes, text/html)
2004-11-12 11:18 PST, Bernd
no flags Details
patch (2.38 KB, patch)
2004-11-13 00:55 PST, Bernd
bzbarsky: review+
bzbarsky: superreview+
asa: approval1.7.5-
Details | Diff | Splinter Review
patch updated to branch (2.39 KB, patch)
2005-06-07 12:09 PDT, Bernd
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval‑aviary1.0.5+
dveditz: approval1.7.9+
Details | Diff | Splinter Review

Description User image Amarendra Hanumanula 2002-05-02 12:00:03 PDT
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">&nbsp;</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.
Comment 1 User image Amarendra Hanumanula 2002-05-02 12:01:23 PDT
Created attachment 82074 [details]
testcase attached
Comment 2 User image Jerry 2002-05-29 03:14:43 PDT
it hangs with mozilla rc2 build.
so add hang to it.
Comment 3 User image Miloslaw Smyk 2002-07-01 16:18:45 PDT
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 User image Jesse Ruderman 2002-09-23 23:00:58 PDT
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.
Comment 5 User image Jesse Ruderman 2002-11-06 12:16:37 PST
See also bug 86293, crash with large colspan (rather than rowspan).
Comment 6 User image Michael Lefevre 2002-11-27 08:50:37 PST
*** Bug 182196 has been marked as a duplicate of this bug. ***
Comment 7 User image Bernd 2002-12-23 10:37:47 PST
Created attachment 110010 [details] [diff] [review]
patch
Comment 8 User image Bernd 2002-12-23 10:43:12 PST
taking the bug
Comment 9 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2002-12-23 11:52:03 PST
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'.
Comment 10 User image Bernd 2002-12-24 00:36:09 PST
#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. 
Comment 11 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2003-01-16 13:18:19 PST
#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.
Comment 12 User image Bernd 2003-01-18 12:33:19 PST
Created attachment 111934 [details] [diff] [review]
changes to celldata.h
Comment 13 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2003-01-18 15:49:33 PST
Comment on attachment 110010 [details] [diff] [review]
patch

sr=roc+moz
Comment 14 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2003-01-18 15:49:54 PST
Comment on attachment 111934 [details] [diff] [review]
changes to celldata.h

r+sr=roc+moz
Comment 15 User image Bernd 2003-01-19 07:26:18 PST
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 User image Jo Hermans 2003-02-07 02:52:39 PST
*** Bug 192229 has been marked as a duplicate of this bug. ***
Comment 17 User image Philip K. Warren 2003-02-26 14:47:48 PST
This checkin has caused a regression in layout for 64-bit builds (Bug 194726).
Comment 18 User image Bernd 2003-04-30 21:53:52 PDT
*** Bug 203880 has been marked as a duplicate of this bug. ***
Comment 19 User image Andriy Rysin 2003-08-12 18:54:23 PDT
JFYI, don't have any problems neither with testcase nor with url with Mozilla
1.4 on Linux
Comment 20 User image Bernd 2003-12-26 07:53:49 PST
*** Bug 229445 has been marked as a duplicate of this bug. ***
Comment 21 User image basic 2004-10-28 09:35:20 PDT
*** Bug 265846 has been marked as a duplicate of this bug. ***
Comment 22 User image Andrew Schultz 2004-11-01 13:39:45 PST
*** Bug 267199 has been marked as a duplicate of this bug. ***
Comment 23 User image Juliano F. Ravasi 2004-11-01 14:14:25 PST
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.
Comment 24 User image Phil Ringnalda (:philor) 2004-11-04 17:23:10 PST
*** Bug 267634 has been marked as a duplicate of this bug. ***
Comment 25 User image Tracy Walker [:tracy] 2004-11-11 19:20:20 PST
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 User image David G King 2004-11-11 19:59:04 PST
AFAIK both patches attached to this bug report have been checked into the trunk.
Comment 27 User image David G King 2004-11-11 20:05:40 PST
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 User image Michael Lefevre 2004-11-12 03:54:18 PST
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 User image Tracy Walker [:tracy] 2004-11-12 10:24:44 PST
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 User image Boris Zbarsky [:bz] (still a bit busy) 2004-11-12 10:45:25 PST
Created attachment 165723 [details]
Zipped-up profile of the minimal testcase bernd asked me to profile
Comment 31 User image Boris Zbarsky [:bz] (still a bit busy) 2004-11-12 10:48:21 PST
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)

Comment 32 User image Bernd 2004-11-12 11:18:56 PST
Created attachment 165727 [details]
IE large colspan protection
Comment 33 User image Bernd 2004-11-12 11:24:39 PST
hmmm even opera does the same.
Comment 34 User image Michael Lefevre 2004-11-12 11:41:43 PST
updating summary to reflect the issue this bug is now addressing (AIUI)
Comment 35 User image Bernd 2004-11-13 00:55:55 PST
Created attachment 165779 [details] [diff] [review]
patch
Comment 36 User image Boris Zbarsky [:bz] (still a bit busy) 2004-11-15 23:07:29 PST
Comment on attachment 165779 [details] [diff] [review]
patch

r+sr=bzbarsky
Comment 37 User image Bernd 2004-11-16 21:35:21 PST
Comment on attachment 165779 [details] [diff] [review]
patch

fixed on trunk, this is IMHO branch worthy
Comment 38 User image Bernd 2004-11-17 07:27:02 PST
fixed on trunk remove blocking mark
Comment 39 User image Martijn Wargers [:mwargers] 2004-11-17 16:05:42 PST
*** Bug 270427 has been marked as a duplicate of this bug. ***
Comment 40 User image Asa Dotzler [:asa] 2004-11-23 13:07:00 PST
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
Comment 41 User image Bernd 2004-11-30 12:27:07 PST
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.
Comment 42 User image Tavis Ormandy 2004-12-27 13:26:15 PST
*** Bug 276139 has been marked as a duplicate of this bug. ***
Comment 43 User image Daniel Veditz [:dveditz] 2005-04-22 09:42:10 PDT
*** Bug 291402 has been marked as a duplicate of this bug. ***
Comment 44 User image Daniel Veditz [:dveditz] 2005-05-05 16:55:47 PDT
Lot of dupes on this one recently, might be worth taking as a stability fix on
the branches
Comment 45 User image Daniel Veditz [:dveditz] 2005-05-05 16:56:22 PDT
*** Bug 293011 has been marked as a duplicate of this bug. ***
Comment 46 User image Jay Patel [:jay] 2005-06-07 11:30:11 PDT
Bernd:  This is +'d for 1.0.5, can you put together a patch for the Aviary branch?  
Comment 47 User image Bernd 2005-06-07 12:09:59 PDT
Created attachment 185587 [details] [diff] [review]
patch updated to branch
Comment 48 User image Jay Patel [:jay] 2005-06-07 18:00:55 PDT
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?
Comment 49 User image Bernd 2005-06-07 21:56:48 PDT
the branch patch is identical to the trunk version, so I don't see why an
additional r/sr should be necessary
Comment 50 User image Jay Patel [:jay] 2005-06-09 12:37:15 PDT
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 User image Daniel Veditz [:dveditz] 2005-06-09 13:26:29 PDT
Comment on attachment 185587 [details] [diff] [review]
patch updated to branch

a=dveditz for 1.0.5/1.7.9
Comment 52 User image Daniel Veditz [:dveditz] 2005-06-09 13:29:45 PDT
Please add the fixed-aviary1.0.5 and fixed1.7.9 keywords when this lands on the
branches.
Comment 53 User image Bernd 2005-06-10 03:29:44 PDT
fix checked in on branches
Comment 54 User image sairuh (rarely reading bugmail) 2005-06-17 12:54:36 PDT
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.
Comment 55 User image Jay Patel [:jay] 2005-07-06 17:05:21 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.