From db40de323a94e6e8f309313df889e3412bf0c983 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Thu, 5 Jun 2008 22:24:05 +0000 Subject: [PATCH] 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 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../org/apache/poi/hssf/dev/BiffViewer.java | 3 +- src/java/org/apache/poi/hssf/model/Sheet.java | 109 ++++++++++------- .../aggregates/ValueRecordsAggregate.java | 2 +- .../org/apache/poi/hssf/model/TestSheet.java | 110 ++++++++++++++---- 6 files changed, 160 insertions(+), 66 deletions(-) diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 5cc5a00e0c..be46902dad 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate 45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes 45087 - Correctly detect date formats like [Black]YYYY as being date based 45060 - Improved token class transformation during formula parsing diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index b8c44d9b9d..ae5136b7ec 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate 45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes 45087 - Correctly detect date formats like [Black]YYYY as being date based 45060 - Improved token class transformation during formula parsing diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java index cd43651912..9396e679de 100644 --- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java @@ -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); diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 871ecc100f..b95ad6bba3 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/src/java/org/apache/poi/hssf/model/Sheet.java @@ -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; + /** + * @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... + + // '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(); } - 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); - - int rowBlockOffset = 0; - int cellBlockOffset = 0; - int dbCellOffset = 0; - for (int block=0;blockUncalcedRecord was present.

*/ 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 RowRecordsAggregate and ValueRecordsAggregate could + * sometimes occur in reverse order. This test reproduces one of those situations and makes + * sure that RRA comes before VRA.
+ * + * 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; + } + } } -- 2.39.5