[PATCH]: fix occasional segfault in dw::Table::reallocChildren
This patch fixes an occasional segfault when adding columns to a table that contains cells that span multiple rows or columns. This was reported to the list last month by Matthias Franz. I've been bitten by this on several web sites (see the details below), so it's a useful fix. Regards, Jeremy Henty Only in dw2-cur/: Makefile Only in dw2-cur/: Makefile.in Only in dw2-cur/: aclocal.m4 Only in dw2-cur/: autom4te.cache Only in dw2-cur/: config.guess Only in dw2-cur/: config.log Only in dw2-cur/: config.status Only in dw2-cur/: config.sub Only in dw2-cur/: configure Only in dw2-cur/: depcomp Only in dw2-cur/doc: Makefile Only in dw2-cur/doc: Makefile.in Only in dw2-cur/dw: .deps Only in dw2-cur/dw: Makefile Only in dw2-cur/dw: Makefile.in diff -pru -- dw2-ref/dw/table.cc dw2-cur/dw/table.cc --- dw2-ref/dw/table.cc 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/dw/table.cc 2007-11-16 13:02:59.000000000 +0000 @@ -357,7 +357,7 @@ void Table::reallocChildren (int newNumC break; case Child::SPAN_SPACE: if (children->get(children->get(n)->spanSpace.startRow - * newNumCols + + * numCols + children->get(n)->spanSpace.startCol) ->cell.colspanOrig == 0) { colspan0Col = children->get(n)->spanSpace.startCol; Only in dw2-cur/: install-sh Only in dw2-cur/lout: .deps Only in dw2-cur/lout: Makefile Only in dw2-cur/lout: Makefile.in Only in dw2-cur/: missing Only in dw2-cur/test: .deps Only in dw2-cur/test: Makefile Only in dw2-cur/test: Makefile.in Analysis of the bug: A dillo table stores its children in a single vector that is the concatenation of the rows. This means that when the number of columns increases the contents of the vector must be rearranged. The code does this in place, starting at the end and working backwards. When the code reaches a point that corresponds to a multi-row/column cell, it finds a placeholder that stores the row/column of the actual cell. The important point is that the actual cell is stored in the vector *before* any placeholders that refer to it. Therefore the cell has not yet been moved when the code reaches the placeholder (because it works backwards from the end). The code must calculate the index of the actual cell from the row/column in the placeholder based on the *old* number of columns. Unfortunately the code uses the *new* number of columns, so it may retrieve a cell from the wrong place. Sometimes it retrieves a NULL and segfaults when it tries to dereference it. Test cases: <html><head></head><body><table><tbody> <tr><td colspan="2"></td></tr> <tr><td rowspan="2"></td></tr> <tr><td colspan="2"></td></tr> </tbody></table></body></html> [NB: the next one is from Matthias Franz.] <html> <table> <tr></tr> <tr><td colspan="3"></td></tr> <td colspan="3"> </table> </html> Online pages that trigger the bug: <a href="http://legacyeditorial.gettyimages.com/source/search/details_pop.aspx?iid=77727174&cdi=0"> <a href="http://www.eweek.com/article2/0,1759,2216435,00.asp"> <a href="http://www.possible-area.org/bad_timing161107.php"> <a href="http://www.eweek.com/article2/0,1895,2216978,00.asp"> <a href="http://www.nostalgia.com/nf_moreinfo.html?sku=10576">
Hi Jeremy, On Fri, Nov 16, 2007 at 03:02:37PM +0000, Jeremy Henty wrote:
This patch fixes an occasional segfault when adding columns to a table that contains cells that span multiple rows or columns. This was reported to the list last month by Matthias Franz. I've been bitten by this on several web sites (see the details below), so it's a useful fix.
Yes it is. Committed! BTW, thanks for the excellent explanation.
Analysis of the bug:
A dillo table stores its children in a single vector that is the concatenation of the rows. This means that when the number of columns increases the contents of the vector must be rearranged. The code does this in place, starting at the end and working backwards. When the code reaches a point that corresponds to a multi-row/column cell, it finds a placeholder that stores the row/column of the actual cell.
The important point is that the actual cell is stored in the vector *before* any placeholders that refer to it. Therefore the cell has not yet been moved when the code reaches the placeholder (because it works backwards from the end). The code must calculate the index of the actual cell from the row/column in the placeholder based on the *old* number of columns.
Unfortunately the code uses the *new* number of columns, so it may retrieve a cell from the wrong place. Sometimes it retrieves a NULL and segfaults when it tries to dereference it.
Test cases:
<html><head></head><body><table><tbody> <tr><td colspan="2"></td></tr> <tr><td rowspan="2"></td></tr> <tr><td colspan="2"></td></tr> </tbody></table></body></html>
[NB: the next one is from Matthias Franz.]
<html> <table> <tr></tr> <tr><td colspan="3"></td></tr> <td colspan="3"> </table> </html>
Online pages that trigger the bug:
<a href="http://legacyeditorial.gettyimages.com/source/search/details_pop.aspx?iid=77727174&cdi=0"> <a href="http://www.eweek.com/article2/0,1759,2216435,00.asp"> <a href="http://www.possible-area.org/bad_timing161107.php"> <a href="http://www.eweek.com/article2/0,1895,2216978,00.asp"> <a href="http://www.nostalgia.com/nf_moreinfo.html?sku=10576">
-- Cheers Jorge.-
participants (2)
-
jcid＠dillo.org -
onepoint＠starurchin.org