mirror of https://github.com/apache/poi.git
Fix for bug 45145 - made sure RowRecordsAggregate comes before ValueRecordsAggregate. Also fixed BiffViewer to show correct record offsets
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@663765 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Josh Micich
parent
91f7d013b3
commit
db40de323a
|
|
@ -37,6 +37,7 @@
|
|||
|
||||
<!-- Don't forget to update status.xml too! -->
|
||||
<release version="3.1-final" date="2008-06-??">
|
||||
<action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
|
||||
<action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@
|
|||
<!-- Don't forget to update changes.xml too! -->
|
||||
<changes>
|
||||
<release version="3.1-final" date="2008-06-??">
|
||||
<action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
|
||||
<action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
|
||||
|
|
|
|||
|
|
@ -87,7 +87,8 @@ public final class BiffViewer {
|
|||
records.add(record);
|
||||
if (activeRecord != null)
|
||||
activeRecord.dump(ps);
|
||||
activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), (int)recStream.getPos(), record);
|
||||
int startPos = (int)(recStream.getPos()-recStream.getLength() - 4);
|
||||
activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), startPos, record);
|
||||
}
|
||||
if (dump) {
|
||||
recStream.dumpBytes(ps);
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ public final class Sheet implements Model {
|
|||
protected ArrayList records = null;
|
||||
int preoffset = 0; // offset of the sheet in a new file
|
||||
int loc = 0;
|
||||
protected int dimsloc = 0;
|
||||
protected int dimsloc = -1; // TODO - is it legal for dims record to be missing?
|
||||
protected DimensionsRecord dims;
|
||||
protected DefaultColWidthRecord defaultcolwidth = null;
|
||||
protected DefaultRowHeightRecord defaultrowheight = null;
|
||||
|
|
@ -295,6 +295,8 @@ public final class Sheet implements Model {
|
|||
}
|
||||
else if ( rec.getSid() == IndexRecord.sid )
|
||||
{
|
||||
// ignore INDEX record because it is only needed by Excel,
|
||||
// and POI always re-calculates its contents
|
||||
rec = null;
|
||||
}
|
||||
|
||||
|
|
@ -329,8 +331,8 @@ public final class Sheet implements Model {
|
|||
}
|
||||
}
|
||||
retval.records = records;
|
||||
retval.checkCells();
|
||||
retval.checkRows();
|
||||
retval.checkCells();
|
||||
if (log.check( POILogger.DEBUG ))
|
||||
log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited");
|
||||
return retval;
|
||||
|
|
@ -486,7 +488,15 @@ public final class Sheet implements Model {
|
|||
if (cells == null)
|
||||
{
|
||||
cells = new ValueRecordsAggregate();
|
||||
records.add(getDimsLoc() + 1, cells);
|
||||
// In the worksheet stream, the row records always occur before the cell (value)
|
||||
// records. Therefore POI's aggregates (RowRecordsAggregate, ValueRecordsAggregate)
|
||||
// should follow suit. Some methods in this class tolerate either order, while
|
||||
// others have been found to fail (see bug 45145).
|
||||
int rraIndex = getDimsLoc() + 1;
|
||||
if (records.get(rraIndex).getClass() != RowRecordsAggregate.class) {
|
||||
throw new IllegalStateException("Cannot create value records before row records exist");
|
||||
}
|
||||
records.add(rraIndex+1, cells);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -836,46 +846,61 @@ public final class Sheet implements Model {
|
|||
return pos-offset;
|
||||
}
|
||||
|
||||
private int serializeIndexRecord(final int BOFRecordIndex, final int offset, byte[] data) {
|
||||
IndexRecord index = new IndexRecord();
|
||||
index.setFirstRow(rows.getFirstRowNum());
|
||||
index.setLastRowAdd1(rows.getLastRowNum()+1);
|
||||
//Calculate the size of the records from the end of the BOF
|
||||
//and up to the RowRecordsAggregate...
|
||||
int sheetRecSize = 0;
|
||||
for (int j = BOFRecordIndex+1; j < records.size(); j++)
|
||||
{
|
||||
Record tmpRec = (( Record ) records.get(j));
|
||||
if (tmpRec instanceof UncalcedRecord) {
|
||||
continue;
|
||||
}
|
||||
if (tmpRec instanceof RowRecordsAggregate) {
|
||||
break;
|
||||
}
|
||||
sheetRecSize+= tmpRec.getRecordSize();
|
||||
}
|
||||
if (_isUncalced) {
|
||||
sheetRecSize += UncalcedRecord.getStaticRecordSize();
|
||||
}
|
||||
//Add the references to the DBCells in the IndexRecord (one for each block)
|
||||
int blockCount = rows.getRowBlockCount();
|
||||
//Calculate the size of this IndexRecord
|
||||
int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
|
||||
/**
|
||||
* @param indexRecordOffset also happens to be the end of the BOF record
|
||||
* @return the size of the serialized INDEX record
|
||||
*/
|
||||
private int serializeIndexRecord(final int bofRecordIndex, final int indexRecordOffset,
|
||||
byte[] data) {
|
||||
IndexRecord index = new IndexRecord();
|
||||
index.setFirstRow(rows.getFirstRowNum());
|
||||
index.setLastRowAdd1(rows.getLastRowNum() + 1);
|
||||
// Calculate the size of the records from the end of the BOF
|
||||
// and up to the RowRecordsAggregate...
|
||||
|
||||
int rowBlockOffset = 0;
|
||||
int cellBlockOffset = 0;
|
||||
int dbCellOffset = 0;
|
||||
for (int block=0;block<blockCount;block++) {
|
||||
rowBlockOffset += rows.getRowBlockSize(block);
|
||||
cellBlockOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows.getStartRowNumberForBlock(block),
|
||||
rows.getEndRowNumberForBlock(block));
|
||||
//Note: The offsets are relative to the Workbook BOF. Assume that this is
|
||||
//0 for now.....
|
||||
index.addDbcell(offset + indexRecSize + sheetRecSize + dbCellOffset + rowBlockOffset + cellBlockOffset);
|
||||
//Add space required to write the dbcell record(s) (whose references were just added).
|
||||
dbCellOffset += (8 + (rows.getRowCountForBlock(block) * 2));
|
||||
}
|
||||
return index.serialize(offset, data);
|
||||
// 'initial sheet records' are between INDEX and first ROW record.
|
||||
int sizeOfInitialSheetRecords = 0;
|
||||
// start just after BOF record (INDEX is not present in this list)
|
||||
for (int j = bofRecordIndex + 1; j < records.size(); j++) {
|
||||
Record tmpRec = ((Record) records.get(j));
|
||||
if (tmpRec instanceof UncalcedRecord) {
|
||||
continue;
|
||||
}
|
||||
if (tmpRec instanceof RowRecordsAggregate) {
|
||||
break;
|
||||
}
|
||||
sizeOfInitialSheetRecords += tmpRec.getRecordSize();
|
||||
}
|
||||
if (_isUncalced) {
|
||||
sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
|
||||
}
|
||||
|
||||
// Add the references to the DBCells in the IndexRecord (one for each block)
|
||||
// Note: The offsets are relative to the Workbook BOF. Assume that this is
|
||||
// 0 for now.....
|
||||
|
||||
int blockCount = rows.getRowBlockCount();
|
||||
// Calculate the size of this IndexRecord
|
||||
int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
|
||||
|
||||
int currentOffset = indexRecordOffset + indexRecSize + sizeOfInitialSheetRecords;
|
||||
|
||||
for (int block = 0; block < blockCount; block++) {
|
||||
// each row-block has a DBCELL record.
|
||||
// The offset of each DBCELL record needs to be updated in the INDEX record
|
||||
|
||||
// account for row records in this row-block
|
||||
currentOffset += rows.getRowBlockSize(block);
|
||||
// account for cell value records after those
|
||||
currentOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows
|
||||
.getStartRowNumberForBlock(block), rows.getEndRowNumberForBlock(block));
|
||||
|
||||
// currentOffset is now the location of the DBCELL record for this row-block
|
||||
index.addDbcell(currentOffset);
|
||||
// Add space required to write the DBCELL record (whose reference was just added).
|
||||
currentOffset += (8 + (rows.getRowCountForBlock(block) * 2));
|
||||
}
|
||||
return index.serialize(indexRecordOffset, data);
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ import java.util.List;
|
|||
public final class ValueRecordsAggregate
|
||||
extends Record
|
||||
{
|
||||
public final static short sid = -1000;
|
||||
public final static short sid = -1001; // 1000 clashes with RowRecordsAggregate
|
||||
int firstcell = -1;
|
||||
int lastcell = -1;
|
||||
CellValueRecordInterface[][] records;
|
||||
|
|
|
|||
|
|
@ -19,11 +19,15 @@ package org.apache.poi.hssf.model;
|
|||
|
||||
import junit.framework.AssertionFailedError;
|
||||
import junit.framework.TestCase;
|
||||
|
||||
import org.apache.poi.hssf.eventmodel.ERFListener;
|
||||
import org.apache.poi.hssf.eventmodel.EventRecordFactory;
|
||||
import org.apache.poi.hssf.record.*;
|
||||
import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
|
||||
import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
|
||||
import org.apache.poi.hssf.record.aggregates.ValueRecordsAggregate;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
|
|
@ -34,8 +38,7 @@ import java.util.List;
|
|||
* @author Glen Stampoultzis (glens at apache.org)
|
||||
*/
|
||||
public final class TestSheet extends TestCase {
|
||||
public void testCreateSheet() throws Exception
|
||||
{
|
||||
public void testCreateSheet() {
|
||||
// Check we're adding row and cell aggregates
|
||||
List records = new ArrayList();
|
||||
records.add( new BOFRecord() );
|
||||
|
|
@ -52,8 +55,7 @@ public final class TestSheet extends TestCase {
|
|||
assertTrue( sheet.records.get(pos++) instanceof EOFRecord );
|
||||
}
|
||||
|
||||
public void testAddMergedRegion()
|
||||
{
|
||||
public void testAddMergedRegion() {
|
||||
Sheet sheet = Sheet.createSheet();
|
||||
int regionsToAdd = 4096;
|
||||
int startRecords = sheet.getRecords().size();
|
||||
|
|
@ -91,8 +93,7 @@ public final class TestSheet extends TestCase {
|
|||
}
|
||||
}
|
||||
|
||||
public void testRemoveMergedRegion()
|
||||
{
|
||||
public void testRemoveMergedRegion() {
|
||||
Sheet sheet = Sheet.createSheet();
|
||||
int regionsToAdd = 4096;
|
||||
|
||||
|
|
@ -139,13 +140,11 @@ public final class TestSheet extends TestCase {
|
|||
assertEquals("Should be no more merged regions", 0, sheet.getNumMergedRegions());
|
||||
}
|
||||
|
||||
public void testGetMergedRegionAt()
|
||||
{
|
||||
public void testGetMergedRegionAt() {
|
||||
//TODO
|
||||
}
|
||||
|
||||
public void testGetNumMergedRegions()
|
||||
{
|
||||
public void testGetNumMergedRegions() {
|
||||
//TODO
|
||||
}
|
||||
|
||||
|
|
@ -163,14 +162,13 @@ public final class TestSheet extends TestCase {
|
|||
|
||||
Sheet sheet = Sheet.createSheet(records, 0);
|
||||
assertNotNull("Row [2] was skipped", sheet.getRow(2));
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* Make sure page break functionality works (in memory)
|
||||
*
|
||||
*/
|
||||
public void testRowPageBreaks(){
|
||||
public void testRowPageBreaks() {
|
||||
short colFrom = 0;
|
||||
short colTo = 255;
|
||||
|
||||
|
|
@ -226,7 +224,7 @@ public final class TestSheet extends TestCase {
|
|||
* Make sure column pag breaks works properly (in-memory)
|
||||
*
|
||||
*/
|
||||
public void testColPageBreaks(){
|
||||
public void testColPageBreaks() {
|
||||
short rowFrom = 0;
|
||||
short rowTo = (short)65535;
|
||||
|
||||
|
|
@ -292,20 +290,20 @@ public final class TestSheet extends TestCase {
|
|||
final short DEFAULT_IDX = 0xF; // 15
|
||||
short xfindex = Short.MIN_VALUE;
|
||||
Sheet sheet = Sheet.createSheet();
|
||||
|
||||
|
||||
// without ColumnInfoRecord
|
||||
xfindex = sheet.getXFIndexForColAt((short) 0);
|
||||
assertEquals(DEFAULT_IDX, xfindex);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 1);
|
||||
assertEquals(DEFAULT_IDX, xfindex);
|
||||
|
||||
|
||||
ColumnInfoRecord nci = ( ColumnInfoRecord ) sheet.createColInfo();
|
||||
sheet.columns.insertColumn(nci);
|
||||
|
||||
|
||||
// single column ColumnInfoRecord
|
||||
nci.setFirstColumn((short) 2);
|
||||
nci.setLastColumn((short) 2);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 0);
|
||||
assertEquals(DEFAULT_IDX, xfindex);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 1);
|
||||
|
|
@ -318,7 +316,7 @@ public final class TestSheet extends TestCase {
|
|||
// ten column ColumnInfoRecord
|
||||
nci.setFirstColumn((short) 2);
|
||||
nci.setLastColumn((short) 11);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 1);
|
||||
assertEquals(DEFAULT_IDX, xfindex);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 2);
|
||||
|
|
@ -333,7 +331,7 @@ public final class TestSheet extends TestCase {
|
|||
// single column ColumnInfoRecord starting at index 0
|
||||
nci.setFirstColumn((short) 0);
|
||||
nci.setLastColumn((short) 0);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 0);
|
||||
assertEquals(TEST_IDX, xfindex);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 1);
|
||||
|
|
@ -342,7 +340,7 @@ public final class TestSheet extends TestCase {
|
|||
// ten column ColumnInfoRecord starting at index 0
|
||||
nci.setFirstColumn((short) 0);
|
||||
nci.setLastColumn((short) 9);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
nci.setXFIndex(TEST_IDX);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 0);
|
||||
assertEquals(TEST_IDX, xfindex);
|
||||
xfindex = sheet.getXFIndexForColAt((short) 7);
|
||||
|
|
@ -354,7 +352,7 @@ public final class TestSheet extends TestCase {
|
|||
}
|
||||
|
||||
/**
|
||||
* Prior to bug 45066, POI would get the estimated sheet size wrong
|
||||
* Prior to bug 45066, POI would get the estimated sheet size wrong
|
||||
* when an <tt>UncalcedRecord</tt> was present.<p/>
|
||||
*/
|
||||
public void testUncalcSize_bug45066() {
|
||||
|
|
@ -363,7 +361,7 @@ public final class TestSheet extends TestCase {
|
|||
records.add(new BOFRecord());
|
||||
records.add(new UncalcedRecord());
|
||||
records.add(new EOFRecord());
|
||||
Sheet sheet = Sheet.createSheet( records, 0, 0 );
|
||||
Sheet sheet = Sheet.createSheet(records, 0, 0);
|
||||
|
||||
int estimatedSize = sheet.getSize();
|
||||
int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
|
||||
|
|
@ -372,5 +370,73 @@ public final class TestSheet extends TestCase {
|
|||
}
|
||||
assertEquals(50, serializedSize);
|
||||
}
|
||||
|
||||
/**
|
||||
* Prior to bug 45145 <tt>RowRecordsAggregate</tt> and <tt>ValueRecordsAggregate</tt> could
|
||||
* sometimes occur in reverse order. This test reproduces one of those situations and makes
|
||||
* sure that RRA comes before VRA.<br/>
|
||||
*
|
||||
* The code here represents a normal POI use case where a spreadsheet is created from scratch.
|
||||
*/
|
||||
public void testRowValueAggregatesOrder_bug45145() {
|
||||
|
||||
Sheet sheet = Sheet.createSheet();
|
||||
|
||||
RowRecord rr = new RowRecord(5);
|
||||
sheet.addRow(rr);
|
||||
|
||||
CellValueRecordInterface cvr = new BlankRecord();
|
||||
cvr.setColumn((short)0);
|
||||
cvr.setRow(5);
|
||||
sheet.addValueRecord(5, cvr);
|
||||
|
||||
|
||||
int dbCellRecordPos = getDbCellRecordPos(sheet);
|
||||
if (dbCellRecordPos == 264) {
|
||||
// The overt symptom of the bug
|
||||
// DBCELL record pos is calculated wrong if VRA comes before RRA
|
||||
throw new AssertionFailedError("Identified bug 45145");
|
||||
}
|
||||
|
||||
// make sure that RRA and VRA are in the right place
|
||||
int rraIx = sheet.getDimsLoc()+1;
|
||||
List recs = sheet.getRecords();
|
||||
assertEquals(RowRecordsAggregate.class, recs.get(rraIx).getClass());
|
||||
assertEquals(ValueRecordsAggregate.class, recs.get(rraIx+1).getClass());
|
||||
|
||||
assertEquals(254, dbCellRecordPos);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the value calculated for the position of the first DBCELL record for this sheet.
|
||||
* That value is found on the IndexRecord.
|
||||
*/
|
||||
private static int getDbCellRecordPos(Sheet sheet) {
|
||||
int size = sheet.getSize();
|
||||
byte[] data = new byte[size];
|
||||
sheet.serialize(0, data);
|
||||
EventRecordFactory erf = new EventRecordFactory();
|
||||
MyIndexRecordListener myIndexListener = new MyIndexRecordListener();
|
||||
erf.registerListener(myIndexListener, new short[] { IndexRecord.sid, });
|
||||
erf.processRecords(new ByteArrayInputStream(data));
|
||||
IndexRecord indexRecord = myIndexListener.getIndexRecord();
|
||||
int dbCellRecordPos = indexRecord.getDbcellAt(0);
|
||||
return dbCellRecordPos;
|
||||
}
|
||||
|
||||
private static final class MyIndexRecordListener implements ERFListener {
|
||||
|
||||
private IndexRecord _indexRecord;
|
||||
public MyIndexRecordListener() {
|
||||
// no-arg constructor
|
||||
}
|
||||
public boolean processRecord(Record rec) {
|
||||
_indexRecord = (IndexRecord)rec;
|
||||
return true;
|
||||
}
|
||||
public IndexRecord getIndexRecord() {
|
||||
return _indexRecord;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue