diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 0502ddc4b..dbac53ad9 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 46548 - Print Settings Block fixes - continued PLS records and PSB in sheet sub-streams 46523 - added implementation for SUMIF function Support for reading HSSF column styles Hook up POIXMLTextExtractor.getMetadataTextExtractor() to the already written POIXMLPropertiesTextExtractor diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index aaab68a12..93ab10e19 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 46548 - Print Settings Block fixes - continued PLS records and PSB in sheet sub-streams 46523 - added implementation for SUMIF function Support for reading HSSF column styles Hook up POIXMLTextExtractor.getMetadataTextExtractor() to the already written POIXMLPropertiesTextExtractor diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 834d33d74..472fec915 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/src/java/org/apache/poi/hssf/model/Sheet.java @@ -56,6 +56,7 @@ import org.apache.poi.hssf.record.SaveRecalcRecord; 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.ColumnInfoRecordsAggregate; @@ -163,9 +164,18 @@ public final class Sheet implements Model { records = new ArrayList(128); // TODO - take chart streams off into separate java objects - int bofEofNestingLevel = 0; // nesting level can only get to 2 (when charts are present) + int bofEofNestingLevel = 1; // nesting level can only get to 2 (when charts are present) int dimsloc = -1; + if (rs.peekNextSid() == BOFRecord.sid) { + BOFRecord bof = (BOFRecord) rs.getNext(); + if (bof.getType() != BOFRecord.TYPE_WORKSHEET) { + // TODO - fix junit tests throw new RuntimeException("Bad BOF record type"); + } + records.add(bof); + } else { + throw new RuntimeException("BOF record expected"); + } while (rs.hasNext()) { int recSid = rs.peekNextSid(); @@ -200,12 +210,34 @@ public final class Sheet implements Model { if (PageSettingsBlock.isComponentRecord(recSid)) { PageSettingsBlock psb = new PageSettingsBlock(rs); - if (bofEofNestingLevel == 1) { - if (_psBlock == null) { - _psBlock = psb; + if (_psBlock == null) { + _psBlock = psb; + } 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) + // This happens three times in test sample file "29982.xls" + if (rs.peekNextSid() != UnknownRecord.USERSVIEWEND_01AB) { + // not quite the expected situation + throw new RuntimeException("two Page Settings Blocks found in the same sheet"); + } } else { - // more than one 'Page Settings Block' at nesting level 1 ? - // apparently this happens in about 15 test sample files + // 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); @@ -332,7 +364,34 @@ public final class Sheet implements Model { if (log.check( POILogger.DEBUG )) log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited"); } + /** + * 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 temp = new ArrayList(); + 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 _destList; + + public RecordTransferrer(List destList) { + _destList = destList; + } + public void visitRecord(Record r) { + _destList.add(r); + } + } private static final class RecordCloner implements RecordVisitor { private final List _destList; diff --git a/src/java/org/apache/poi/hssf/record/BOFRecord.java b/src/java/org/apache/poi/hssf/record/BOFRecord.java index f67a67028..9e661a553 100644 --- a/src/java/org/apache/poi/hssf/record/BOFRecord.java +++ b/src/java/org/apache/poi/hssf/record/BOFRecord.java @@ -20,6 +20,8 @@ package org.apache.poi.hssf.record; import org.apache.poi.util.HexDump; import org.apache.poi.util.LittleEndianOutput; +import com.sun.java_cup.internal.version; + /** * Title: Beginning Of File (0x0809)

* Description: Somewhat of a misnomer, its used for the beginning of a set of @@ -35,8 +37,8 @@ public final class BOFRecord extends StandardRecord { */ public final static short sid = 0x809; - /** suggested default (0x06 - BIFF8) */ - public final static int VERSION = 0x06; + /** suggested default (0x0600 - BIFF8) */ + public final static int VERSION = 0x0600; /** suggested default 0x10d3 */ public final static int BUILD = 0x10d3; /** suggested default 0x07CC (1996) */ @@ -63,6 +65,19 @@ public final class BOFRecord extends StandardRecord { */ public BOFRecord() { } + + private BOFRecord(int type) { + field_1_version = VERSION; + field_2_type = type; + field_3_build = BUILD; + field_4_year = BUILD_YEAR; + field_5_history = 0x01; + field_6_rversion = VERSION; + } + + public static BOFRecord createSheetBOF() { + return new BOFRecord(TYPE_WORKSHEET); + } public BOFRecord(RecordInputStream in) { field_1_version = in.readShort(); diff --git a/src/java/org/apache/poi/hssf/record/UnknownRecord.java b/src/java/org/apache/poi/hssf/record/UnknownRecord.java index 6ece4782f..e3cef59d9 100644 --- a/src/java/org/apache/poi/hssf/record/UnknownRecord.java +++ b/src/java/org/apache/poi/hssf/record/UnknownRecord.java @@ -42,6 +42,8 @@ public final class UnknownRecord extends StandardRecord { public static final int BITMAP_00E9 = 0x00E9; public static final int PHONETICPR_00EF = 0x00EF; public static final int LABELRANGES_015F = 0x015F; + public static final int USERSVIEWBEGIN_01AA = 0x01AA; + public static final int USERSVIEWEND_01AB = 0x01AB; public static final int QUICKTIP_0800 = 0x0800; public static final int SHEETEXT_0862 = 0x0862; // OOO calls this SHEETLAYOUT public static final int SHEETPROTECTION_0867 = 0x0867; @@ -145,8 +147,8 @@ public final class UnknownRecord extends StandardRecord { case LABELRANGES_015F: return "LABELRANGES"; case 0x01BA: return "CODENAME"; case 0x01A9: return "USERBVIEW"; - case 0x01AA: return "USERSVIEWBEGIN"; - case 0x01AB: return "USERSVIEWEND"; + case USERSVIEWBEGIN_01AA: return "USERSVIEWBEGIN"; + case USERSVIEWEND_01AB: return "USERSVIEWEND"; case 0x01AD: return "QSI"; case 0x01C0: return "EXCEL9FILE"; 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 68d4559a3..828c0e31f 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java @@ -19,11 +19,13 @@ package org.apache.poi.hssf.record.aggregates; import java.util.ArrayList; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import org.apache.poi.hssf.model.RecordStream; import org.apache.poi.hssf.model.Sheet; import org.apache.poi.hssf.record.BottomMarginRecord; +import org.apache.poi.hssf.record.ContinueRecord; import org.apache.poi.hssf.record.FooterRecord; import org.apache.poi.hssf.record.HCenterRecord; import org.apache.poi.hssf.record.HeaderRecord; @@ -60,6 +62,13 @@ public final class PageSettingsBlock extends RecordAggregate { private TopMarginRecord _topMargin; private BottomMarginRecord _bottomMargin; private Record _pls; + /** + * holds any continue records found after the PLS record.
+ * This would not be required if PLS was properly interpreted. + * Currently, PLS is an {@link UnknownRecord} and does not automatically + * include any trailing {@link ContinueRecord}s. + */ + private List _plsContinues; private PrintSetupRecord printSetup; private Record _bitmap; @@ -140,13 +149,19 @@ public final class PageSettingsBlock extends RecordAggregate { case BottomMarginRecord.sid: _bottomMargin = (BottomMarginRecord) rs.getNext(); break; - case 0x004D: // PLS + case UnknownRecord.PLS_004D: _pls = rs.getNext(); + while (rs.peekNextSid()==ContinueRecord.sid) { + if (_plsContinues==null) { + _plsContinues = new LinkedList(); + } + _plsContinues.add((ContinueRecord)rs.getNext()); + } break; case PrintSetupRecord.sid: printSetup = (PrintSetupRecord)rs.getNext(); break; - case 0x00E9: // BITMAP + case UnknownRecord.BITMAP_00E9: _bitmap = rs.getNext(); break; default: @@ -202,6 +217,11 @@ public final class PageSettingsBlock extends RecordAggregate { visitIfPresent(_topMargin, rv); visitIfPresent(_bottomMargin, rv); visitIfPresent(_pls, rv); + if (_plsContinues != null) { + for (ContinueRecord cr : _plsContinues) { + visitIfPresent(cr, rv); + } + } visitIfPresent(printSetup, rv); visitIfPresent(_bitmap, rv); } @@ -335,58 +355,51 @@ public final class PageSettingsBlock extends RecordAggregate { * @param margin which margin to get * @return the size of the margin */ - public double getMargin(short margin) { - Margin m = getMarginRec(margin); - if (m != null) { - return m.getMargin(); - } else { - switch ( margin ) - { - case Sheet.LeftMargin: - return .75; - case Sheet.RightMargin: - return .75; - case Sheet.TopMargin: - return 1.0; - case Sheet.BottomMargin: - return 1.0; - } + public double getMargin(short margin) { + Margin m = getMarginRec(margin); + if (m != null) { + return m.getMargin(); + } + switch (margin) { + case Sheet.LeftMargin: return .75; + case Sheet.RightMargin: return .75; + case Sheet.TopMargin: return 1.0; + case Sheet.BottomMargin: return 1.0; + } throw new RuntimeException( "Unknown margin constant: " + margin ); - } - } + } /** * Sets the size of the margin in inches. * @param margin which margin to get * @param size the size of the margin */ - public void setMargin(short margin, double size) { - Margin m = getMarginRec(margin); - if (m == null) { - switch ( margin ) - { - case Sheet.LeftMargin: - _leftMargin = new LeftMarginRecord(); - m = _leftMargin; - break; - case Sheet.RightMargin: - _rightMargin = new RightMarginRecord(); - m = _rightMargin; - break; - case Sheet.TopMargin: - _topMargin = new TopMarginRecord(); - m = _topMargin; - break; - case Sheet.BottomMargin: - _bottomMargin = new BottomMarginRecord(); - m = _bottomMargin; - break; - default : - throw new RuntimeException( "Unknown margin constant: " + margin ); - } - } - m.setMargin( size ); - } + public void setMargin(short margin, double size) { + Margin m = getMarginRec(margin); + if (m == null) { + switch (margin) { + case Sheet.LeftMargin: + _leftMargin = new LeftMarginRecord(); + m = _leftMargin; + break; + case Sheet.RightMargin: + _rightMargin = new RightMarginRecord(); + m = _rightMargin; + break; + case Sheet.TopMargin: + _topMargin = new TopMarginRecord(); + m = _topMargin; + break; + case Sheet.BottomMargin: + _bottomMargin = new BottomMarginRecord(); + m = _bottomMargin; + break; + default : + throw new RuntimeException( "Unknown margin constant: " + margin ); + } + } + m.setMargin( size ); + } /** * Shifts all the page breaks in the range "count" number of rows/columns diff --git a/src/testcases/org/apache/poi/hssf/data/ex46548-23133.xls b/src/testcases/org/apache/poi/hssf/data/ex46548-23133.xls new file mode 100644 index 000000000..cc1fe3a29 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/ex46548-23133.xls differ diff --git a/src/testcases/org/apache/poi/hssf/model/TestSheet.java b/src/testcases/org/apache/poi/hssf/model/TestSheet.java index bd8678720..7a857d6ca 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestSheet.java +++ b/src/testcases/org/apache/poi/hssf/model/TestSheet.java @@ -68,7 +68,7 @@ public final class TestSheet extends TestCase { public void testCreateSheet() { // Check we're adding row and cell aggregates List records = new ArrayList(); - records.add( new BOFRecord() ); + records.add(BOFRecord.createSheetBOF()); records.add( new DimensionsRecord() ); records.add(createWindow2Record()); records.add(EOFRecord.instance); @@ -187,6 +187,7 @@ public final class TestSheet extends TestCase { new CellRangeAddress(0, 1, 0, 2), }; MergeCellsRecord merged = new MergeCellsRecord(cras, 0, cras.length); + records.add(BOFRecord.createSheetBOF()); records.add(new DimensionsRecord()); records.add(new RowRecord(0)); records.add(new RowRecord(1)); @@ -449,7 +450,7 @@ public final class TestSheet extends TestCase { public void testUncalcSize_bug45066() { List records = new ArrayList(); - records.add(new BOFRecord()); + records.add(BOFRecord.createSheetBOF()); records.add(new UncalcedRecord()); records.add(new DimensionsRecord()); records.add(createWindow2Record()); @@ -600,7 +601,7 @@ public final class TestSheet extends TestCase { nr.setValue(3.0); List inRecs = new ArrayList(); - inRecs.add(new BOFRecord()); + inRecs.add(BOFRecord.createSheetBOF()); inRecs.add(new RowRecord(rowIx)); inRecs.add(nr); inRecs.add(createWindow2Record()); 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 aafe08258..e19f182f7 100644 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java @@ -1,41 +1,42 @@ -/* ==================================================================== - 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 junit.framework.Test; -import junit.framework.TestSuite; - -/** - * Collects all tests for package org.apache.poi.hssf.record.aggregates. - * - * @author Josh Micich - */ -public final class AllRecordAggregateTests { - - public static Test suite() { - TestSuite result = new TestSuite(AllRecordAggregateTests.class.getName()); - - result.addTestSuite(TestCFRecordsAggregate.class); - result.addTestSuite(TestColumnInfoRecordsAggregate.class); - result.addTestSuite(TestFormulaRecordAggregate.class); - result.addTestSuite(TestRowRecordsAggregate.class); - result.addTestSuite(TestSharedValueManager.class); - result.addTestSuite(TestValueRecordsAggregate.class); - return result; - } -} +/* ==================================================================== + 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 junit.framework.Test; +import junit.framework.TestSuite; + +/** + * Collects all tests for package org.apache.poi.hssf.record.aggregates. + * + * @author Josh Micich + */ +public final class AllRecordAggregateTests { + + public static Test suite() { + TestSuite result = new TestSuite(AllRecordAggregateTests.class.getName()); + + result.addTestSuite(TestCFRecordsAggregate.class); + result.addTestSuite(TestColumnInfoRecordsAggregate.class); + result.addTestSuite(TestFormulaRecordAggregate.class); + result.addTestSuite(TestRowRecordsAggregate.class); + result.addTestSuite(TestSharedValueManager.class); + result.addTestSuite(TestValueRecordsAggregate.class); + result.addTestSuite(TestPageSettingBlock.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 new file mode 100644 index 000000000..518ed5f30 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingBlock.java @@ -0,0 +1,50 @@ +/* ==================================================================== + 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 junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.usermodel.HSSFPrintSetup; +import org.apache.poi.hssf.usermodel.HSSFSheet; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; + +/** + * 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"); + } + } +}