86293 - 'colspan'-Tags with large 'span' Values crash the browser.

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[dr.khong]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT 5.0)
BuildID:    2001050515

Large 'span' values in 'colspan'-Tags create Memory-Leaks and crash Mozilla or 
even the System. See Sample-Site for details. Other Browsers don't have this 
Problem.

Reproducible: Always
Steps to Reproduce:
Open http://home.t-online.de/home/hoellengott/moz/kill_mozilla.html or create 
a .html-File containing
<table>
  <colgroup width=200 span=100000000>
  </colgroup>
</table>
and open it.

Actual Results:  Browser allocated lots of memory (>300 MB) and stopped 
responding. The system started lots of paging, soon a "out of virtual memory"-
error almost crashed it. Had to kill Mozilla with strg-alt-del.

Expected Results:  It should not crash and it should not allocate that much 
memory.

See
http://home.t-online.de/home/hoellengott/moz/kill_mozilla.html
for further deatils.

Comment 1

[Oliver Klee]

Adding mlk and crash keywords.

Comment 2

[doctor__j]

Woo Nelly...  That's a thrilling experience...

Comment 3

[Aaron Kaluszka]

confirming WinXP/2001061504

Comment 4

[David Einstein]

Attachment: testcase

Comment 5

[Olivier Zendra]

confirming on Linux (mandrake 7.2 kernel 2.2.19-4.1mdksmp),
w/ moz 0.9.1 build 2001060713.
Moz got killed after eating all remaining RAM+swap (602 MB).

Comment 6

[Aaron Kaluszka]

OS->All

Comment 7

[Bernd]

Attachment: bandaid patch

Comment 8

[Bernd]

The idea behind the patch is if we have really to handle a table with colspan
with more than 1000 columns I guess we will not survive
BasicTableLayoutStrategy.cpp, so we could clip it at 1000 columns while parsing.
Another patch could incorporate the use of GetEffectiveCols(). But I would
expect that implementing this would be the shortest way to the pole position on
the topcrash list.

Comment 9

[Marc Attinasi]

bernd, is the value 1000 arbitrary or meaningful? Either way, your patch with a
nice comment explaining why we have to limit the span would be very good indeed.
[s]r=attinasi

Also, we should probably open a bug about handling more than 1000 spans
correctly instead of just dropping it on the floor, no?

Comment 10

[Alexandru Savulov]

WORKS FOR ME.
no crash, no hang, no excesive memory usage, WORKS FOR ME.

(used with the URL above that has a colspan of 100000....
tested with ns6.1b1 and one of the latest pull_all on W2K, W98SE
EN and DE lang packs.)

can anyone reproduce this? is it already patched?

Comment 11

[Alexandru Savulov]

marking WORKS FOR ME since no one claims the contrary

Comment 12

[dr.khong]

Bug's back again (or still not fixed). DOSing the browser is still possible.

Confirming 1.0 and 1.0.1:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0) Gecko/20020530
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020826

Looking at
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLTableColElement.cpp
it seems like the patch above
http://bugzilla.mozilla.org/attachment.cgi?id=39722&action=view
hasn't been applied.

Comment 13

[Bernd]

we had that a very long time ago 

http://lxr.mozilla.org/classic/source/lib/layout/laytable.c#46

Comment 14

[Jerry]

*** Bug 160816 has been marked as a duplicate of this bug. ***

Comment 15

[karnaze (gone)]

Are there any real pages that are a problem?

Comment 16

[Jerry]

Dup bug 160816 has one url in real world.

Comment 17

[karnaze (gone)]

Bernd, could you add a #define with the max value. I'm not sure when I would get
around to comming up with a better patch.

Comment 18

[Jesse Ruderman]

See also bug 141818, hang with large rowspan (rather than colspan).

Comment 19

[Bernd]

Attachment: patch

Comment 20

[karnaze (gone)]

Comment on attachment 106483 [details] [diff] [review]
patch

r=karnaze

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 106483 [details] [diff] [review]
patch

What am I missing?  Comment 9 asked for a clear comment on this hack.  Please
add that.  While you're at it, comment the identical hack in
nsHTMLTableCellElement.cpp

sr=bzbarsky with that

Comment 22

[Bernd]

fix checked in