Bug 46548 - fixes for Page Settings Block (patch from Dmitriy Kumshayev + some mods)

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@735179 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta5" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">46548 - Print Settings Block fixes - continued PLS records and PSB in sheet sub-streams</action>
            <action dev="POI-DEVELOPERS" type="add">46523 - added implementation for SUMIF function</action>
            <action dev="POI-DEVELOPERS" type="add">Support for reading HSSF column styles</action>
            <action dev="POI-DEVELOPERS" type="fix">Hook up POIXMLTextExtractor.getMetadataTextExtractor() to the already written POIXMLPropertiesTextExtractor</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta5" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">46548 - Print Settings Block fixes - continued PLS records and PSB in sheet sub-streams</action>
            <action dev="POI-DEVELOPERS" type="add">46523 - added implementation for SUMIF function</action>
            <action dev="POI-DEVELOPERS" type="add">Support for reading HSSF column styles</action>
            <action dev="POI-DEVELOPERS" type="fix">Hook up POIXMLTextExtractor.getMetadataTextExtractor() to the already written POIXMLPropertiesTextExtractor</action>

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<RecordBase>(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) {
-                    if (_psBlock == null) {
+                    _psBlock = psb;
-                        _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 ?
+                            // Some apps write PLS, WSBOOL, <psb> but PLS is part of <psb>
-                        // apparently this happens in about 15 test sample files
+                            // 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<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;

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)<P>
  * 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) */
+    /** suggested default (0x0600 - BIFF8) */
-    public final static int VERSION             = 0x06;
+    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();

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 USERSVIEWBEGIN_01AA: return "USERSVIEWBEGIN";
-			case 0x01AB: return "USERSVIEWEND";
+			case USERSVIEWEND_01AB: return "USERSVIEWEND";
 			case 0x01AD: return "QSI";
 
 			case 0x01C0: return "EXCEL9FILE";

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.<br/>
+	 * 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<ContinueRecord> _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<ContinueRecord>();
+					}
+					_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) {
+	public double getMargin(short margin) {
-	   Margin m = getMarginRec(margin);
+		Margin m = getMarginRec(margin);
-	   if (m != null) {
+		if (m != null) {
-		return m.getMargin();
+			return m.getMargin();
-	} else {
+		}
-	   switch ( margin )
+		switch (margin) {
-	   {
+			case Sheet.LeftMargin:   return .75;
-	   case Sheet.LeftMargin:
+			case Sheet.RightMargin:  return .75;
-		   return .75;
+			case Sheet.TopMargin:    return 1.0;
-	   case Sheet.RightMargin:
+			case Sheet.BottomMargin: return 1.0;
-		   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) {
+	public void setMargin(short margin, double size) {
-   Margin m = getMarginRec(margin);
+		Margin m = getMarginRec(margin);
-   if (m  == null) {
+		if (m  == null) {
-	   switch ( margin )
+			switch (margin) {
-	   {
+				case Sheet.LeftMargin:
-	   case Sheet.LeftMargin:
+					_leftMargin = new LeftMarginRecord();
-		   _leftMargin = new LeftMarginRecord();
+					m = _leftMargin;
-		   m = _leftMargin;
+					break;
-		   break;
+				case Sheet.RightMargin:
-	   case Sheet.RightMargin:
+					_rightMargin = new RightMarginRecord();
-		   _rightMargin = new RightMarginRecord();
+					m = _rightMargin;
-		   m = _rightMargin;
+					break;
-		   break;
+				case Sheet.TopMargin:
-	   case Sheet.TopMargin:
+					_topMargin = new TopMarginRecord();
-		   _topMargin = new TopMarginRecord();
+					m = _topMargin;
-		   m = _topMargin;
+					break;
-		   break;
+				case Sheet.BottomMargin:
-	   case Sheet.BottomMargin:
+					_bottomMargin = new BottomMarginRecord();
-		   _bottomMargin = new BottomMarginRecord();
+					m = _bottomMargin;
-		   m = _bottomMargin;
+					break;
-		   break;
+				default :
-	   default :
+					throw new RuntimeException( "Unknown margin constant:  " + margin );
-		   throw new RuntimeException( "Unknown margin constant:  " + margin );
+			}
-	   }
+		}
-   }
+		m.setMargin( size );
-   m.setMargin( size );
+	}
-   }
 
 	/**
 	 * Shifts all the page breaks in the range "count" number of rows/columns

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<Record> records = new ArrayList<Record>();
-        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<Record> records = new ArrayList<Record>();
-        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<Record> inRecs = new ArrayList<Record>();
-        inRecs.add(new BOFRecord());
+        inRecs.add(BOFRecord.createSheetBOF());
         inRecs.add(new RowRecord(rowIx));
         inRecs.add(nr);
         inRecs.add(createWindow2Record());

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
+   Licensed to the Apache Software Foundation (ASF) under one or more
-   contributor license agreements.  See the NOTICE file distributed with
+   contributor license agreements.  See the NOTICE file distributed with
-   this work for additional information regarding copyright ownership.
+   this work for additional information regarding copyright ownership.
-   The ASF licenses this file to You under the Apache License, Version 2.0
+   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 not use this file except in compliance with
-   the License.  You may obtain a copy of the License at
+   the License.  You may obtain a copy of the License at
-
+
-       http://www.apache.org/licenses/LICENSE-2.0
+       http://www.apache.org/licenses/LICENSE-2.0
-
+
-   Unless required by applicable law or agreed to in writing, software
+   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
+   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
+   See the License for the specific language governing permissions and
-   limitations under the License.
+   limitations under the License.
-==================================================================== */
+==================================================================== */
-
+
-package org.apache.poi.hssf.record.aggregates;
+package org.apache.poi.hssf.record.aggregates;
-
+
-import junit.framework.Test;
+import junit.framework.Test;
-import junit.framework.TestSuite;
+import junit.framework.TestSuite;
-
+
-/**
+/**
- * Collects all tests for package <tt>org.apache.poi.hssf.record.aggregates</tt>.
+ * Collects all tests for package <tt>org.apache.poi.hssf.record.aggregates</tt>.
- * 
+ * 
- * @author Josh Micich
+ * @author Josh Micich
- */
+ */
-public final class AllRecordAggregateTests {
+public final class AllRecordAggregateTests {
-	
+	
-	public static Test suite() {
+	public static Test suite() {
-		TestSuite result = new TestSuite(AllRecordAggregateTests.class.getName());
+		TestSuite result = new TestSuite(AllRecordAggregateTests.class.getName());
-
+
-		result.addTestSuite(TestCFRecordsAggregate.class);
+		result.addTestSuite(TestCFRecordsAggregate.class);
-		result.addTestSuite(TestColumnInfoRecordsAggregate.class);
+		result.addTestSuite(TestColumnInfoRecordsAggregate.class);
-		result.addTestSuite(TestFormulaRecordAggregate.class);
+		result.addTestSuite(TestFormulaRecordAggregate.class);
-		result.addTestSuite(TestRowRecordsAggregate.class);
+		result.addTestSuite(TestRowRecordsAggregate.class);
-		result.addTestSuite(TestSharedValueManager.class);
+		result.addTestSuite(TestSharedValueManager.class);
-		result.addTestSuite(TestValueRecordsAggregate.class);
+		result.addTestSuite(TestValueRecordsAggregate.class);
-		return result;
+		result.addTestSuite(TestPageSettingBlock.class);
-	}
+		return result;
-}
+	}
+}

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");
+		}
+	}
+}