From: Josh Micich Date: Sun, 10 May 2009 21:34:28 +0000 (+0000) Subject: Bug 46953 - fixed PageSettingsBlock parsing logic in Sheet. Bug caused sheet EOFs... X-Git-Tag: REL_3_5_BETA6~41 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c028c0d1f163c6101ee746b60395ec9a72879c51;p=poi.git Bug 46953 - fixed PageSettingsBlock parsing logic in Sheet. Bug caused sheet EOFs to get misplaced. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@773412 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 39f0cfc0e0..ef0430c73e 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor 47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId 46568 - Fixed XSLFPowerPointExtractor to properly process line breaks 39056 - Fixed POIFSFileSystem to set CLSID of root when constructing instances from InputStream diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 82563c9f67..7e1027f6dd 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor 47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId 46568 - Fixed XSLFPowerPointExtractor to properly process line breaks 39056 - Fixed POIFSFileSystem to set CLSID of root when constructing instances from InputStream diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 40a2e1e92d..948b5befcb 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/src/java/org/apache/poi/hssf/model/Sheet.java @@ -50,6 +50,7 @@ import org.apache.poi.hssf.record.PrintHeadersRecord; 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 @@ public final class Sheet implements Model { } 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, but PLS is part of - // 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, but PLS is part of + // 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 @@ public final class Sheet implements Model { 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) { diff --git a/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java b/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java index 99694dcc99..1731a1c3a1 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java @@ -35,6 +35,7 @@ import org.apache.poi.hssf.record.Margin; 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 @@ public final class PageSettingsBlock extends RecordAggregate { 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; } } diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java b/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java index e19f182f77..57fe336b60 100644 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java @@ -36,7 +36,7 @@ public final class AllRecordAggregateTests { result.addTestSuite(TestRowRecordsAggregate.class); result.addTestSuite(TestSharedValueManager.class); result.addTestSuite(TestValueRecordsAggregate.class); - result.addTestSuite(TestPageSettingBlock.class); + result.addTestSuite(TestPageSettingsBlock.class); return result; } } diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java deleted file mode 100644 index 07a49fd1e6..0000000000 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java +++ /dev/null @@ -1,115 +0,0 @@ -/* ==================================================================== - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -==================================================================== */ - -package org.apache.poi.hssf.record.aggregates; - -import java.util.Arrays; - -import junit.framework.AssertionFailedError; -import junit.framework.TestCase; - -import org.apache.poi.hssf.HSSFTestDataSamples; -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.DimensionsRecord; -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.NumberRecord; -import org.apache.poi.hssf.record.Record; -import org.apache.poi.hssf.record.UnknownRecord; -import org.apache.poi.hssf.record.WindowTwoRecord; -import org.apache.poi.hssf.usermodel.HSSFPrintSetup; -import org.apache.poi.hssf.usermodel.HSSFSheet; -import org.apache.poi.hssf.usermodel.HSSFWorkbook; -import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector; -import org.apache.poi.util.HexRead; - -/** - * Tess for {@link PageSettingsBlock} - * - * @author Dmitriy Kumshayev - */ -public final class TestPageSettingBlock extends TestCase { - - public void testPrintSetup_bug46548() { - - // PageSettingBlock in this file contains PLS (sid=x004D) record - // followed by ContinueRecord (sid=x003C) - HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex46548-23133.xls"); - HSSFSheet sheet = wb.getSheetAt(0); - HSSFPrintSetup ps = sheet.getPrintSetup(); - - try { - ps.getCopies(); - } catch (NullPointerException e) { - e.printStackTrace(); - throw new AssertionFailedError("Identified bug 46548: PageSettingBlock missing PrintSetupRecord record"); - } - } - - /** - * Bug 46840 occurred because POI failed to recognise HEADERFOOTER as part of the - * {@link PageSettingsBlock}. - * - */ - public void testHeaderFooter_bug46840() { - - 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"), - 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"), - new DimensionsRecord(), - new WindowTwoRecord(), - ur(UnknownRecord.USERSVIEWBEGIN_01AA, "ED 77 3B 86 BC 3F 37 4C A9 58 60 23 43 68 54 4B 01 00 00 00 64 00 00 00 40 00 00 00 02 00 00 00 3D 80 04 00 00 00 00 00 00 00 0C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 F0 3F FF FF 01 00"), - new HeaderRecord("&LSales Figures"), - new FooterRecord("&LJanuary"), - 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"), - ur(UnknownRecord.USERSVIEWEND_01AB, "01, 00"), - - 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 46480"); - } - throw e; - } - - RecordCollector rv = new RecordCollector(); - sheet.visitContainedRecords(rv, rowIx); - Record[] outRecs = rv.getRecords(); - assertEquals(13, outRecs.length); - } - - private static UnknownRecord ur(int sid, String hexData) { - return new UnknownRecord(sid, HexRead.readFromString(hexData)); - } -} diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java new file mode 100644 index 0000000000..076bbfeb37 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java @@ -0,0 +1,158 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.record.aggregates; + +import java.util.Arrays; + +import junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.hssf.HSSFTestDataSamples; +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.DimensionsRecord; +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; +import org.apache.poi.hssf.record.WindowTwoRecord; +import org.apache.poi.hssf.usermodel.HSSFPrintSetup; +import org.apache.poi.hssf.usermodel.HSSFSheet; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector; +import org.apache.poi.util.HexRead; + +/** + * Tess for {@link PageSettingsBlock} + * + * @author Dmitriy Kumshayev + */ +public final class TestPageSettingsBlock extends TestCase { + + public void testPrintSetup_bug46548() { + + // PageSettingBlock in this file contains PLS (sid=x004D) record + // followed by ContinueRecord (sid=x003C) + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex46548-23133.xls"); + HSSFSheet sheet = wb.getSheetAt(0); + HSSFPrintSetup ps = sheet.getPrintSetup(); + + try { + ps.getCopies(); + } catch (NullPointerException e) { + e.printStackTrace(); + throw new AssertionFailedError("Identified bug 46548: PageSettingBlock missing PrintSetupRecord record"); + } + } + + /** + * Bug 46840 occurred because POI failed to recognise HEADERFOOTER as part of the + * {@link PageSettingsBlock}. + * + */ + public void testHeaderFooter_bug46840() { + + 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"), + 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"), + new DimensionsRecord(), + new WindowTwoRecord(), + ur(UnknownRecord.USERSVIEWBEGIN_01AA, "ED 77 3B 86 BC 3F 37 4C A9 58 60 23 43 68 54 4B 01 00 00 00 64 00 00 00 40 00 00 00 02 00 00 00 3D 80 04 00 00 00 00 00 00 00 0C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 F0 3F FF FF 01 00"), + new HeaderRecord("&LSales Figures"), + new FooterRecord("&LJanuary"), + 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"), + ur(UnknownRecord.USERSVIEWEND_01AB, "01, 00"), + + 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 46480"); + } + throw e; + } + + RecordCollector rv = new RecordCollector(); + sheet.visitContainedRecords(rv, rowIx); + Record[] outRecs = rv.getRecords(); + 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)); + } +}