Bug 14729 - [PATCH] can't merge more than 1027 times
Summary: [PATCH] can't merge more than 1027 times
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 1.5.1
Hardware: PC Windows XP
: P3 critical (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-11-21 04:50 UTC by Tony Poppleton
Modified: 2004-11-16 19:05 UTC (History)
0 users



Attachments
a sample file with 1028 merged cells - causes errors when opening in excep (32.50 KB, application/octet-stream)
2002-11-21 23:27 UTC, Tony Poppleton
Details
Patch file to fix this bug (736 bytes, patch)
2002-11-23 20:30 UTC, Tony Poppleton
Details | Diff
much better patch, which handles reads and writes (5.43 KB, patch)
2003-02-10 03:52 UTC, Tony Poppleton
Details | Diff
started test case for Sheet in hssf.model package (2.26 KB, text/plain)
2003-02-10 03:53 UTC, Tony Poppleton
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Poppleton 2002-11-21 04:50:11 UTC
If more than 1027 merged regions are added to a sheet, then the xls file 
doesn't work (the number of cells merged is unimportant).

here is some code that produces a corrupt excel file... 

import org.apache.poi.hssf.usermodel.*;
import org.apache.poi.hssf.util.*;
import java.io.*;

class POIBug
{
	public static void main(String[] args) throws IOException
	{
		int rows    = 1028;
		int cols    = 1;
		int rowSpan = 1;
		int colSpan = 1;
		//can't merge more than 1027 times
		//ie rows = 514, cols = 2 also doesnt work
		//number of cells merged is not important

		HSSFWorkbook workbook = new HSSFWorkbook();
		HSSFSheet    sheet    = workbook.createSheet();

		//create all the rows and the cells
		for (int row = 0; row < rows; row++)
		{
			HSSFRow r = sheet.createRow(row);
			for (int col = 0; col < cols; col++)
				r.createCell((short) col);
		}

		//merge the approriate cells
		int merged = 0;
		for (int row = 0; row < rows; row += rowSpan)
		{
			for (int col = 0; col < cols; col += colSpan)
			{
				int   rowFrom = row;
				short colFrom = (short) col;
				int   rowTo   = rowFrom + rowSpan - 1;
				short colTo   = (short) (colFrom + colSpan - 
1);
				sheet.addMergedRegion(new Region(rowFrom, 
colFrom, rowTo, colTo));
				merged++;
			}
		}

		System.out.println("Added " + merged + " merged regions");
		FileOutputStream fos = new FileOutputStream("bug.xls");
		workbook.write(fos);
		fos.close();
	}
}
Comment 1 Andy Oliver 2002-11-21 16:33:12 UTC
cool.  Can you merge > 1027 in excel?  (w/o using POI)?  I'd like to know
whether we should just put a bounds check in or if Excel does something
different for larger numbers.  If so attach a sheet (press create a new attachment)
Comment 2 Tony Poppleton 2002-11-21 19:18:14 UTC
I have tried in excel, and yes it works for at least upto 65000 merged cells.  
I can send you the excel file I you like.

I think I know what the cause of the bug is however - I think the CONTINUE 
record is not being used correctly.

In BIFF8 the maximum biff size is of 8224 for the biff contents without the 
header, and a merged cell record requires 8 bytes per range.  An additional 2 
bytes are required at the beginning for the number of merged cell entries.  
For 1027 merged cells this equals 8*1027 + 2 = 8218 bytes, for 1028 merges a 
corrupt xls file is produced, and the biff entry contains 8*1028 + 2 = 8226 
bytes, which exceeds the biff8 record structure!  I assume the bug is 
therefore is the incorrect use of a CONTINUE record.
Comment 3 Andy Oliver 2002-11-21 20:59:27 UTC
so please attach the xls file here (click create attachment).  Your analysis
though sounds an awful like you might be the best person to submit a patch:
http://jakarta.apache.org/poi/getinvolved/index.html
Comment 4 Tony Poppleton 2002-11-21 23:27:21 UTC
Created attachment 3914 [details]
a sample file with 1028 merged cells - causes errors when opening in excep
Comment 5 Tony Poppleton 2002-11-21 23:32:06 UTC
I have had a go at creating a patch, but failed miserably.  I don't understand 
how POI fits together just yet, will give it another go in two weeks time 
(fully booked till then).  I did get as far as confirming my suspicion using 
the BiffViewer util in POI, and here is an excerp from the example file I just 
submitted (above):

============================================
Offset 0x5a8b (23179)
recordid = 0xe5, size =8226
[MERGEDCELLS]
     .sid        =229
     .numregions =1028
     .rowfrom    =0
     .colfrom    =0

So the size is 8226 which exceeds the biff record size of 8224.  I am unsure 
how to divide up the MergedCells record into Continue records to resolve it 
though.
Comment 6 Tony Poppleton 2002-11-22 01:15:18 UTC
I have found a quick one-liner that can solve the problem, but couldn't figure 
out how to get CVS to create the proper patch file.  Could someone in the know 
upload this change please.  Thanks.

In package org.apache.poi.hssf.model, change the following method to:

public int addMergedRegion(int rowFrom, short colFrom, int rowTo,short colTo)
{
 //only 1027 entries can be packed into a single BIFF8 MergedCellsRecord
 if (merged == null || merged.getNumAreas() == 1027)
 ...

The if just had (merger == null) before.  This change causes a new 
MergeCellsRecord to be created (instead of a ContinueRecord - which would also 
solve the problem, but this is much easier to do) when there are too many 
merged areas in the current merged record.  I have tested it and it works.

Thanks,
Tony
Comment 7 Tony Poppleton 2002-11-23 20:30:55 UTC
Created attachment 3932 [details]
Patch file to fix this bug
Comment 8 Andy Oliver 2002-11-23 20:51:09 UTC
thanks.  Can you also submit a junit test that replicates this?  Do you think
you could make the changes necessary to make this READ in the case of
continuations?  I'll apply this shortly.
Comment 9 Tony Poppleton 2003-02-10 03:52:22 UTC
Created attachment 4802 [details]
much better patch, which handles reads and writes
Comment 10 Tony Poppleton 2003-02-10 03:53:43 UTC
Created attachment 4803 [details]
started test case for Sheet in hssf.model package
Comment 11 Tony Poppleton 2003-02-28 22:42:01 UTC
could someone please apply this patch, thanks
Comment 12 Avik Sengupta 2003-03-01 17:52:15 UTC
applied, thanks. This issue was marked resolved, and thus was not shoing up in
my query. Would appreciate if you could complete the testcases.