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/05/10 23:34:29 UTC
svn commit: r773412 - 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: Sun May 10 21:34:28 2009
New Revision: 773412
URL: http://svn.apache.org/viewvc?rev=773412&view=rev
Log:
Bug 46953 - fixed PageSettingsBlock parsing logic in Sheet. Bug caused sheet EOFs to get misplaced.
Added:
poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
- copied, changed from r773130, poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java
Removed:
poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java
Modified:
poi/trunk/src/documentation/content/xdocs/changes.xml
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/AllRecordAggregateTests.java
Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=773412&r1=773411&r2=773412&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Sun May 10 21:34:28 2009
@@ -37,6 +37,7 @@
<!-- Don't forget to update status.xml too! -->
<release version="3.5-beta6" date="2009-??-??">
+ <action dev="POI-DEVELOPERS" type="fix">46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor</action>
<action dev="POI-DEVELOPERS" type="fix">47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId</action>
<action dev="POI-DEVELOPERS" type="fix">46568 - Fixed XSLFPowerPointExtractor to properly process line breaks</action>
<action dev="POI-DEVELOPERS" type="fix">39056 - Fixed POIFSFileSystem to set CLSID of root when constructing instances from InputStream</action>
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=773412&r1=773411&r2=773412&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Sun May 10 21:34:28 2009
@@ -34,6 +34,7 @@
<!-- Don't forget to update changes.xml too! -->
<changes>
<release version="3.5-beta6" date="2009-??-??">
+ <action dev="POI-DEVELOPERS" type="fix">46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor</action>
<action dev="POI-DEVELOPERS" type="fix">47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId</action>
<action dev="POI-DEVELOPERS" type="fix">46568 - Fixed XSLFPowerPointExtractor to properly process line breaks</action>
<action dev="POI-DEVELOPERS" type="fix">39056 - Fixed POIFSFileSystem to set CLSID of root when constructing instances from InputStream</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=773412&r1=773411&r2=773412&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 Sun May 10 21:34:28 2009
@@ -50,6 +50,7 @@
import org.apache.poi.hssf.record.ProtectRecord;
import org.apache.poi.hssf.record.Record;
import org.apache.poi.hssf.record.RecordBase;
+import org.apache.poi.hssf.record.RecordFormatException;
import org.apache.poi.hssf.record.RefModeRecord;
import org.apache.poi.hssf.record.RowRecord;
import org.apache.poi.hssf.record.SCLRecord;
@@ -210,40 +211,47 @@
}
if (PageSettingsBlock.isComponentRecord(recSid)) {
- PageSettingsBlock psb = new PageSettingsBlock(rs);
+ PageSettingsBlock psb;
if (_psBlock == null) {
+ psb = new PageSettingsBlock(rs);
_psBlock = psb;
+ records.add(psb);
+ continue;
+ }
+ if (bofEofNestingLevel == 2) {
+ psb = new PageSettingsBlock(rs);
+ // It's normal for a chart to have its own PageSettingsBlock
+ // Fall through and add psb here, because chart records
+ // are stored loose among the sheet records.
+ // this latest psb does not clash with _psBlock
+ } else if (windowTwo != null) {
+ // probably 'Custom View Settings' sub-stream which is found between
+ // USERSVIEWBEGIN(01AA) and USERSVIEWEND(01AB)
+ // TODO - create UsersViewAggregate to hold these sub-streams, and simplify this code a bit
+ // This happens three times in test sample file "29982.xls"
+ // Also several times in bugzilla samples 46840-23373 and 46840-23374
+ if (recSid == UnknownRecord.HEADER_FOOTER_089C) {
+ _psBlock.addLateHeaderFooter(rs.getNext());
+ continue;
+ }
+ psb = new PageSettingsBlock(rs);
+ if (rs.peekNextSid() != UnknownRecord.USERSVIEWEND_01AB) {
+ // not quite the expected situation
+ throw new RuntimeException("two Page Settings Blocks found in the same sheet");
+ }
} else {
- if (bofEofNestingLevel == 2) {
- // It's normal for a chart to have its own PageSettingsBlock
- // Fall through and add psb here, because chart records
- // are stored loose among the sheet records.
- // this latest psb does not clash with _psBlock
- } else if (windowTwo != null) {
- // probably 'Custom View Settings' sub-stream which is found between
- // USERSVIEWBEGIN(01AA) and USERSVIEWEND(01AB)
- // TODO - create UsersViewAggregate to hold these sub-streams, and simplify this code a bit
- // This happens three times in test sample file "29982.xls"
- // Also several times in bugzilla samples 46840-23373 and 46840-23374
- if (recSid == UnknownRecord.HEADER_FOOTER_089C) {
- _psBlock.addLateHeaderFooter(rs.getNext());
- } else if (rs.peekNextSid() != UnknownRecord.USERSVIEWEND_01AB) {
- // not quite the expected situation
- throw new RuntimeException("two Page Settings Blocks found in the same sheet");
- }
- } else {
- // 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");
- }
- records.remove(prevPsbIx); // WSBOOL will drop down one position.
- psb = mergePSBs(_psBlock, psb);
- _psBlock = psb;
+ psb = new PageSettingsBlock(rs);
+ // 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");
}
+ records.remove(prevPsbIx); // WSBOOL will drop down one position.
+ psb = mergePSBs(_psBlock, psb);
+ _psBlock = psb;
}
records.add(psb);
continue;
@@ -274,6 +282,11 @@
bofEofNestingLevel++;
if (log.check( POILogger.DEBUG ))
log.log(POILogger.DEBUG, "Hit BOF record. Nesting increased to " + bofEofNestingLevel);
+ BOFRecord bof = (BOFRecord)rec;
+ // TODO - extract chart sub-stream into record aggregate
+ if (bof.getType() != BOFRecord.TYPE_CHART) {
+ throw new RecordFormatException("Bad BOF record type: " + bof.getType());
+ }
}
else if (recSid == EOFRecord.sid)
{
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=773412&r1=773411&r2=773412&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 Sun May 10 21:34:28 2009
@@ -35,6 +35,7 @@
import org.apache.poi.hssf.record.PageBreakRecord;
import org.apache.poi.hssf.record.PrintSetupRecord;
import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
import org.apache.poi.hssf.record.RightMarginRecord;
import org.apache.poi.hssf.record.TopMarginRecord;
import org.apache.poi.hssf.record.UnknownRecord;
@@ -539,6 +540,9 @@
if (_headerFooter != null) {
throw new IllegalStateException("This page settings block already has a header/footer record");
}
+ if (rec.getSid() != UnknownRecord.HEADER_FOOTER_089C) {
+ throw new RecordFormatException("Unexpected header-footer record sid: 0x" + Integer.toHexString(rec.getSid()));
+ }
_headerFooter = rec;
}
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java?rev=773412&r1=773411&r2=773412&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java Sun May 10 21:34:28 2009
@@ -36,7 +36,7 @@
result.addTestSuite(TestRowRecordsAggregate.class);
result.addTestSuite(TestSharedValueManager.class);
result.addTestSuite(TestValueRecordsAggregate.class);
- result.addTestSuite(TestPageSettingBlock.class);
+ result.addTestSuite(TestPageSettingsBlock.class);
return result;
}
}
Copied: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java (from r773130, poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java)
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java?p2=poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java&p1=poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java&r1=773130&r2=773412&rev=773412&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java Sun May 10 21:34:28 2009
@@ -30,6 +30,7 @@
import org.apache.poi.hssf.record.EOFRecord;
import org.apache.poi.hssf.record.FooterRecord;
import org.apache.poi.hssf.record.HeaderRecord;
+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.UnknownRecord;
@@ -45,7 +46,7 @@
*
* @author Dmitriy Kumshayev
*/
-public final class TestPageSettingBlock extends TestCase {
+public final class TestPageSettingsBlock extends TestCase {
public void testPrintSetup_bug46548() {
@@ -109,6 +110,48 @@
assertEquals(13, outRecs.length);
}
+ /**
+ * Bug 46953 occurred because POI didn't handle late PSB records properly.
+ */
+ public void testLateHeaderFooter_bug46953() {
+
+ int rowIx = 5;
+ int colIx = 6;
+ NumberRecord nr = new NumberRecord();
+ nr.setRow(rowIx);
+ nr.setColumn((short) colIx);
+ nr.setValue(3.0);
+
+ Record[] recs = {
+ BOFRecord.createSheetBOF(),
+ new HeaderRecord("&LSales Figures"),
+ new FooterRecord("&LJanuary"),
+ new DimensionsRecord(),
+ new WindowTwoRecord(),
+ ur(UnknownRecord.HEADER_FOOTER_089C, "9C 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 C4 60 00 00 00 00 00 00 00 00"),
+ EOFRecord.instance,
+ };
+ RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+ Sheet sheet = Sheet.createSheet(rs);
+
+ RecordCollector rv = new RecordCollector();
+ sheet.visitContainedRecords(rv, rowIx);
+ Record[] outRecs = rv.getRecords();
+ if (outRecs[4] == EOFRecord.instance) {
+ throw new AssertionFailedError("Identified bug 46953 - EOF incorrectly appended to PSB");
+ }
+ 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(UnknownRecord.HEADER_FOOTER_089C, outRecs[4].getSid());
+ assertEquals(DimensionsRecord.class, outRecs[5].getClass());
+ assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
+ assertEquals(EOFRecord.instance, outRecs[7]);
+ }
+
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