You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2009/06/01 20:41:01 UTC
svn commit: r780774 - in /poi/trunk/src: documentation/content/xdocs/
java/org/apache/poi/hssf/model/ java/org/apache/poi/hssf/record/aggregates/
testcases/org/apache/poi/hssf/record/aggregates/
Author: josh
Date: Mon Jun 1 18:41:01 2009
New Revision: 780774
URL: http://svn.apache.org/viewvc?rev=780774&view=rev
Log:
Bugzilla 47199 - Fixed PageSettingsBlock/Sheet to tolerate margin records after other non-PSB records
Modified:
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java
poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Mon Jun 1 18:41:01 2009
@@ -35,6 +35,7 @@
<release version="3.5-beta7" date="2009-??-??">
</release>
<release version="3.5-beta6" date="2009-06-11">
+ <action dev="POI-DEVELOPERS" type="fix">47199 - Fixed PageSettingsBlock/Sheet to tolerate margin records after other non-PSB records</action>
<action dev="POI-DEVELOPERS" type="fix">47069 - Fixed HSSFSheet#getFirstRowNum and HSSFSheet#getLastRowNum to return correct values after removal of all rows</action>
<action dev="POI-DEVELOPERS" type="fix">47278 - Fixed XSSFCell to avoid generating xsi:nil entries in shared string table</action>
<action dev="POI-DEVELOPERS" type="fix">47206 - Fixed XSSFCell to properly read inline strings</action>
Modified: poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java Mon Jun 1 18:41:01 2009
@@ -57,7 +57,6 @@
import org.apache.poi.hssf.record.ScenarioProtectRecord;
import org.apache.poi.hssf.record.SelectionRecord;
import org.apache.poi.hssf.record.UncalcedRecord;
-import org.apache.poi.hssf.record.UnknownRecord;
import org.apache.poi.hssf.record.WSBoolRecord;
import org.apache.poi.hssf.record.WindowTwoRecord;
import org.apache.poi.hssf.record.aggregates.ChartSubstreamRecordAggregate;
@@ -207,7 +206,7 @@
records.add(rra); //only add the aggregate once
continue;
}
-
+
if (CustomViewSettingsRecordAggregate.isBeginRecord(recSid)) {
// This happens three times in test sample file "29982.xls"
// Also several times in bugzilla samples 46840-23373 and 46840-23374
@@ -217,28 +216,13 @@
if (PageSettingsBlock.isComponentRecord(recSid)) {
if (_psBlock == null) {
- // typical case - just one PSB (so far)
+ // first PSB record encountered - read all of them:
_psBlock = new PageSettingsBlock(rs);
records.add(_psBlock);
- continue;
- }
- if (recSid == UnknownRecord.HEADER_FOOTER_089C) {
- // test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls
- _psBlock.addLateHeaderFooter(rs.getNext());
- continue;
- }
- // Some apps write PLS, WSBOOL, <psb> but PLS is part of <psb>
- // This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"
- // In this case the first PSB is two records back
- int prevPsbIx = records.size()-2;
- if (_psBlock != records.get(prevPsbIx) || !(records.get(prevPsbIx+1) instanceof WSBoolRecord)) {
- // not quite the expected situation
- throw new RuntimeException("two Page Settings Blocks found in the same sheet");
+ } else {
+ // one or more PSB records found after some intervening non-PSB records
+ _psBlock.addLateRecords(rs);
}
- records.remove(prevPsbIx); // WSBOOL will drop down one position.
- PageSettingsBlock latePsb = new PageSettingsBlock(rs);
- _psBlock = mergePSBs(_psBlock, latePsb);
- records.add(_psBlock);
continue;
}
@@ -247,7 +231,7 @@
_mergedCellsTable.read(rs);
continue;
}
-
+
if (recSid == BOFRecord.sid) {
ChartSubstreamRecordAggregate chartAgg = new ChartSubstreamRecordAggregate(rs);
if (false) {
@@ -372,34 +356,7 @@
recs.add(r);
}});
}
- /**
- * Hack to recover from the situation where the page settings block has been split by
- * an intervening {@link WSBoolRecord}
- */
- private static PageSettingsBlock mergePSBs(PageSettingsBlock a, PageSettingsBlock b) {
- List<Record> temp = new ArrayList<Record>();
- RecordTransferrer rt = new RecordTransferrer(temp);
- a.visitContainedRecords(rt);
- b.visitContainedRecords(rt);
- RecordStream rs = new RecordStream(temp, 0);
- PageSettingsBlock result = new PageSettingsBlock(rs);
- if (rs.hasNext()) {
- throw new RuntimeException("PageSettingsBlocks did not merge properly");
- }
- return result;
- }
-
- private static final class RecordTransferrer implements RecordVisitor {
-
- private final List<Record> _destList;
- public RecordTransferrer(List<Record> destList) {
- _destList = destList;
- }
- public void visitRecord(Record r) {
- _destList.add(r);
- }
- }
private static final class RecordCloner implements RecordVisitor {
private final List<RecordBase> _destList;
@@ -1099,7 +1056,7 @@
*/
public void setColumnWidth(int column, int width) {
if(width > 255*256) throw new IllegalArgumentException("The maximum column width for an individual cell is 255 characters.");
-
+
setColumn(column, null, new Integer(width), null, null, null);
}
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java Mon Jun 1 18:41:01 2009
@@ -125,36 +125,47 @@
private boolean readARecord(RecordStream rs) {
switch (rs.peekNextSid()) {
case HorizontalPageBreakRecord.sid:
+ checkNotPresent(_rowBreaksRecord);
_rowBreaksRecord = (PageBreakRecord) rs.getNext();
break;
case VerticalPageBreakRecord.sid:
+ checkNotPresent(_columnBreaksRecord);
_columnBreaksRecord = (PageBreakRecord) rs.getNext();
break;
case HeaderRecord.sid:
+ checkNotPresent(_header);
_header = (HeaderRecord) rs.getNext();
break;
case FooterRecord.sid:
+ checkNotPresent(_footer);
_footer = (FooterRecord) rs.getNext();
break;
case HCenterRecord.sid:
+ checkNotPresent(_hCenter);
_hCenter = (HCenterRecord) rs.getNext();
break;
case VCenterRecord.sid:
+ checkNotPresent(_vCenter);
_vCenter = (VCenterRecord) rs.getNext();
break;
case LeftMarginRecord.sid:
+ checkNotPresent(_leftMargin);
_leftMargin = (LeftMarginRecord) rs.getNext();
break;
case RightMarginRecord.sid:
+ checkNotPresent(_rightMargin);
_rightMargin = (RightMarginRecord) rs.getNext();
break;
case TopMarginRecord.sid:
+ checkNotPresent(_topMargin);
_topMargin = (TopMarginRecord) rs.getNext();
break;
case BottomMarginRecord.sid:
+ checkNotPresent(_bottomMargin);
_bottomMargin = (BottomMarginRecord) rs.getNext();
break;
case UnknownRecord.PLS_004D:
+ checkNotPresent(_pls);
_pls = rs.getNext();
while (rs.peekNextSid()==ContinueRecord.sid) {
if (_plsContinues==null) {
@@ -164,15 +175,19 @@
}
break;
case PrintSetupRecord.sid:
+ checkNotPresent(_printSetup);
_printSetup = (PrintSetupRecord)rs.getNext();
break;
case UnknownRecord.BITMAP_00E9:
+ checkNotPresent(_bitmap);
_bitmap = rs.getNext();
break;
case UnknownRecord.PRINTSIZE_0033:
+ checkNotPresent(_printSize);
_printSize = rs.getNext();
break;
case UnknownRecord.HEADER_FOOTER_089C:
+ checkNotPresent(_headerFooter);
_headerFooter = rs.getNext();
break;
default:
@@ -182,6 +197,13 @@
return true;
}
+ private void checkNotPresent(Record rec) {
+ if (rec != null) {
+ throw new RecordFormatException("Duplicate PageSettingsBlock record (sid=0x"
+ + Integer.toHexString(rec.getSid()) + ")");
+ }
+ }
+
private PageBreakRecord getRowBreaksRecord() {
if (_rowBreaksRecord == null) {
_rowBreaksRecord = new HorizontalPageBreakRecord();
@@ -551,4 +573,40 @@
}
_headerFooter = rec;
}
+
+ /**
+ * This method reads PageSettingsBlock records from the supplied RecordStream until the first
+ * non-PageSettingsBlock record is encountered. As each record is read, it is incorporated
+ * into this PageSettingsBlock.
+ * <p/>
+ * The latest Excel version seems to write the PageSettingsBlock uninterrupted. However there
+ * are several examples (that Excel reads OK) where these records are not written together:
+ * <ul>
+ * <li><b>HEADER_FOOTER(0x089C) after WINDOW2</b> - This record is new in 2007. Some apps
+ * seem to have scattered this record long after the PageSettingsBlock where it belongs
+ * test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls</li>
+ * <li><b>PLS, WSBOOL, PageSettingsBlock</b> - WSBOOL is not a PSB record.
+ * This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"</li>
+ * <li><b>Margins after DIMENSION</b> - All of PSB should be before DIMENSION. (Bug-47199)</li>
+ * </ul>
+ * These were probably written by other applications (or earlier versions of Excel). It was
+ * decided to not write specific code for detecting each of these cases. POI now tolerates
+ * PageSettingsBlock records scattered all over the sheet record stream, and in any order, but
+ * does not allow duplicates of any of those records.
+ *
+ * <p/>
+ * <b>Note</b> - when POI writes out this PageSettingsBlock, the records will always be written
+ * in one consolidated block (in the standard ordering) regardless of how scattered the records
+ * were when they were originally read.
+ *
+ * @throws RecordFormatException if any PSB record encountered has the same type (sid) as
+ * a record that is already part of this PageSettingsBlock
+ */
+ public void addLateRecords(RecordStream rs) {
+ while(true) {
+ if (!readARecord(rs)) {
+ break;
+ }
+ }
+ }
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java Mon Jun 1 18:41:01 2009
@@ -26,6 +26,7 @@
import org.apache.poi.hssf.model.RecordStream;
import org.apache.poi.hssf.model.Sheet;
import org.apache.poi.hssf.record.BOFRecord;
+import org.apache.poi.hssf.record.BottomMarginRecord;
import org.apache.poi.hssf.record.DimensionsRecord;
import org.apache.poi.hssf.record.EOFRecord;
import org.apache.poi.hssf.record.FooterRecord;
@@ -33,6 +34,7 @@
import org.apache.poi.hssf.record.IndexRecord;
import org.apache.poi.hssf.record.NumberRecord;
import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
import org.apache.poi.hssf.record.UnknownRecord;
import org.apache.poi.hssf.record.WindowTwoRecord;
import org.apache.poi.hssf.usermodel.HSSFPrintSetup;
@@ -135,7 +137,7 @@
Sheet sheet = Sheet.createSheet(rs);
RecordCollector rv = new RecordCollector();
- sheet.visitContainedRecords(rv, rowIx);
+ sheet.visitContainedRecords(rv, 0);
Record[] outRecs = rv.getRecords();
if (outRecs[4] == EOFRecord.instance) {
throw new AssertionFailedError("Identified bug 46953 - EOF incorrectly appended to PSB");
@@ -151,6 +153,85 @@
assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
assertEquals(EOFRecord.instance, outRecs[7]);
}
+ /**
+ * Bug 47199 was due to the margin records being located well after the initial PSB records.
+ * The example file supplied (attachment 23710) had three non-PSB record types
+ * between the PRINTSETUP record and first MARGIN record:
+ * <ul>
+ * <li>PRINTSETUP(0x00A1)</li>
+ * <li>DEFAULTCOLWIDTH(0x0055)</li>
+ * <li>COLINFO(0x007D)</li>
+ * <li>DIMENSIONS(0x0200)</li>
+ * <li>BottomMargin(0x0029)</li>
+ * </ul>
+ */
+ public void testLateMargins_bug47199() {
+
+ Record[] recs = {
+ BOFRecord.createSheetBOF(),
+ new HeaderRecord("&LSales Figures"),
+ new FooterRecord("&LJanuary"),
+ new DimensionsRecord(),
+ createBottomMargin(0.787F),
+ new WindowTwoRecord(),
+ EOFRecord.instance,
+ };
+ RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+
+ Sheet sheet;
+ try {
+ sheet = Sheet.createSheet(rs);
+ } catch (RuntimeException e) {
+ if (e.getMessage().equals("two Page Settings Blocks found in the same sheet")) {
+ throw new AssertionFailedError("Identified bug 47199a - failed to process late margings records");
+ }
+ throw e;
+ }
+
+ RecordCollector rv = new RecordCollector();
+ sheet.visitContainedRecords(rv, 0);
+ Record[] outRecs = rv.getRecords();
+ assertEquals(recs.length+1, outRecs.length); // +1 for index record
+
+ assertEquals(BOFRecord.class, outRecs[0].getClass());
+ assertEquals(IndexRecord.class, outRecs[1].getClass());
+ assertEquals(HeaderRecord.class, outRecs[2].getClass());
+ assertEquals(FooterRecord.class, outRecs[3].getClass());
+ assertEquals(DimensionsRecord.class, outRecs[5].getClass());
+ assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
+ assertEquals(EOFRecord.instance, outRecs[7]);
+ }
+
+ private Record createBottomMargin(float value) {
+ BottomMarginRecord result = new BottomMarginRecord();
+ result.setMargin(value);
+ return result;
+ }
+
+ /**
+ * The PageSettingsBlock should not allow multiple copies of the same record. This extra assertion
+ * was added while fixing bug 47199. All existing POI test samples comply with this requirement.
+ */
+ public void testDuplicatePSBRecord_bug47199() {
+
+ // Hypothetical setup of PSB records which should cause POI to crash
+ Record[] recs = {
+ new HeaderRecord("&LSales Figures"),
+ new HeaderRecord("&LInventory"),
+ };
+ RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+
+ try {
+ new PageSettingsBlock(rs);
+ throw new AssertionFailedError("Identified bug 47199b - duplicate PSB records should not be allowed");
+ } catch (RecordFormatException e) {
+ if (e.getMessage().equals("Duplicate PageSettingsBlock record (sid=0x14)")) {
+ // expected during successful test
+ } else {
+ throw new AssertionFailedError("Expected RecordFormatException due to duplicate PSB record");
+ }
+ }
+ }
private static UnknownRecord ur(int sid, String hexData) {
return new UnknownRecord(sid, HexRead.readFromString(hexData));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org