]> source.dussan.org Git - poi.git/commitdiff
Bugzilla 47415 - Fixed PageSettingsBlock to allow multiple PLS records
authorJosh Micich <josh@apache.org>
Wed, 24 Jun 2009 19:45:44 +0000 (19:45 +0000)
committerJosh Micich <josh@apache.org>
Wed, 24 Jun 2009 19:45:44 +0000 (19:45 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@788157 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/status.xml
src/java/org/apache/poi/hssf/record/UnknownRecord.java
src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java
src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java

index 77d0ecabe7f06d1cd222be08ff2fa5bb966d24c7..34fe02d10fc38eefd3ed97bca9ba8311df871f6f 100644 (file)
@@ -33,6 +33,7 @@
 
     <changes>
         <release version="3.5-beta7" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">47415 - Fixed PageSettingsBlock to allow multiple PLS records</action>
            <action dev="POI-DEVELOPERS" type="fix">47412 - Fixed concurrency issue with EscherProperties.initProps()</action>
            <action dev="POI-DEVELOPERS" type="fix">47143 - Fixed OOM in HSSFWorkbook#getAllPictures when reading .xls files containing metafiles</action>
            <action dev="POI-DEVELOPERS" type="add">Added implementation for ISNA()</action>
index 607eb917cef6cefd7f1b0ad517bb56f9a43e4d07..750cc5dab5053680f543f5367a90dffbeef56934 100644 (file)
@@ -42,6 +42,9 @@ public final class UnknownRecord extends StandardRecord {
         * The few POI test samples with this record have data { 0x03, 0x00 }.
         */
        public static final int PRINTSIZE_0033       = 0x0033;
+       /**
+        * Environment-Specific Print Record
+        */
        public static final int PLS_004D             = 0x004D;
        public static final int SHEETPR_0081         = 0x0081;
        public static final int SORT_0090            = 0x0090;
index 1b9228bec8259975975e5d4f96390d30510d2e84..097eff1120741b93534f4c2f8bc5ebcff274994b 100644 (file)
@@ -6,7 +6,7 @@
    (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
+          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,
@@ -19,7 +19,6 @@ 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;
@@ -44,14 +43,52 @@ import org.apache.poi.hssf.record.VerticalPageBreakRecord;
 
 /**
  * Groups the page settings records for a worksheet.<p/>
- * 
+ *
  * See OOO excelfileformat.pdf sec 4.4 'Page Settings Block'
- * 
+ *
  * @author Josh Micich
  */
 public final class PageSettingsBlock extends RecordAggregate {
-       // Every one of these component records is optional 
-       // (The whole PageSettingsBlock may not be present) 
+
+       /**
+        * PLS is potentially a <i>continued</i> record, but is currently uninterpreted by POI
+        */
+       private static final class PLSAggregate extends RecordAggregate {
+               private static final ContinueRecord[] EMPTY_CONTINUE_RECORD_ARRAY = { };
+               private final 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 ContinueRecord[] _plsContinues;
+
+               public PLSAggregate(RecordStream rs) {
+                       _pls = rs.getNext();
+                       if (rs.peekNextSid()==ContinueRecord.sid) {
+                               List<ContinueRecord> temp = new ArrayList<ContinueRecord>();
+                               while (rs.peekNextSid()==ContinueRecord.sid) {
+                                       temp.add((ContinueRecord)rs.getNext());
+                               }
+                               _plsContinues = new ContinueRecord[temp.size()];
+                               temp.toArray(_plsContinues);
+                       } else {
+                               _plsContinues = EMPTY_CONTINUE_RECORD_ARRAY;
+                       }
+               }
+
+               @Override
+               public void visitContainedRecords(RecordVisitor rv) {
+                       rv.visitRecord(_pls);
+                       for (int i = 0; i < _plsContinues.length; i++) {
+                               rv.visitRecord(_plsContinues[i]);
+                       }
+               }
+       }
+
+       // Every one of these component records is optional
+       // (The whole PageSettingsBlock may not be present)
        private PageBreakRecord _rowBreaksRecord;
        private PageBreakRecord _columnBreaksRecord;
        private HeaderRecord _header;
@@ -62,20 +99,14 @@ public final class PageSettingsBlock extends RecordAggregate {
        private RightMarginRecord _rightMargin;
        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 final List<PLSAggregate> _plsRecords;
        private PrintSetupRecord _printSetup;
        private Record _bitmap;
        private Record _headerFooter;
        private Record _printSize;
 
        public PageSettingsBlock(RecordStream rs) {
+               _plsRecords = new ArrayList<PLSAggregate>();
                while(true) {
                        if (!readARecord(rs)) {
                                break;
@@ -87,6 +118,7 @@ public final class PageSettingsBlock extends RecordAggregate {
         * Creates a PageSettingsBlock with default settings
         */
        public PageSettingsBlock() {
+               _plsRecords = new ArrayList<PLSAggregate>();
                _rowBreaksRecord = new HorizontalPageBreakRecord();
                _columnBreaksRecord = new VerticalPageBreakRecord();
                _header = new HeaderRecord("");
@@ -97,7 +129,7 @@ public final class PageSettingsBlock extends RecordAggregate {
        }
 
        /**
-        * @return <code>true</code> if the specified Record sid is one belonging to the 
+        * @return <code>true</code> if the specified Record sid is one belonging to the
         * 'Page Settings Block'.
         */
        public static boolean isComponentRecord(int sid) {
@@ -115,8 +147,8 @@ public final class PageSettingsBlock extends RecordAggregate {
                        case UnknownRecord.PLS_004D:
                        case PrintSetupRecord.sid:
                        case UnknownRecord.BITMAP_00E9:
-                       case UnknownRecord.PRINTSIZE_0033: 
-                       case UnknownRecord.HEADER_FOOTER_089C: // extra header/footer settings supported by Excel 2007 
+                       case UnknownRecord.PRINTSIZE_0033:
+                       case UnknownRecord.HEADER_FOOTER_089C: // extra header/footer settings supported by Excel 2007
                                return true;
                }
                return false;
@@ -165,14 +197,7 @@ public final class PageSettingsBlock extends RecordAggregate {
                                _bottomMargin = (BottomMarginRecord) rs.getNext();
                                break;
                        case UnknownRecord.PLS_004D:
-                               checkNotPresent(_pls);
-                               _pls = rs.getNext();
-                               while (rs.peekNextSid()==ContinueRecord.sid) {
-                                       if (_plsContinues==null) {
-                                               _plsContinues = new LinkedList<ContinueRecord>();
-                                       }
-                                       _plsContinues.add((ContinueRecord)rs.getNext());
-                               }
+                               _plsRecords.add(new PLSAggregate(rs));
                                break;
                        case PrintSetupRecord.sid:
                                checkNotPresent(_printSetup);
@@ -243,7 +268,7 @@ public final class PageSettingsBlock extends RecordAggregate {
 
                visitIfPresent(_rowBreaksRecord, rv);
                visitIfPresent(_columnBreaksRecord, rv);
-               // Write out empty header / footer records if these are missing 
+               // Write out empty header / footer records if these are missing
                if (_header == null) {
                        rv.visitRecord(new HeaderRecord(""));
                } else {
@@ -260,11 +285,8 @@ public final class PageSettingsBlock extends RecordAggregate {
                visitIfPresent(_rightMargin, rv);
                visitIfPresent(_topMargin, rv);
                visitIfPresent(_bottomMargin, rv);
-               visitIfPresent(_pls, rv);
-               if (_plsContinues != null) {
-                       for (ContinueRecord cr : _plsContinues) {
-                               visitIfPresent(cr, rv);
-                       }
+               for (RecordAggregate pls : _plsRecords) {
+                       pls.visitContainedRecords(rv);
                }
                visitIfPresent(_printSetup, rv);
                visitIfPresent(_bitmap, rv);
@@ -569,11 +591,10 @@ public final class PageSettingsBlock extends RecordAggregate {
        public HCenterRecord getHCenter() {
                return _hCenter;
        }
-       
+
        /**
         * HEADERFOOTER is new in 2007.  Some apps seem to have scattered this record long after
         * the {@link PageSettingsBlock} where it belongs.
-        * 
         */
        public void addLateHeaderFooter(Record rec) {
                if (_headerFooter != null) {
@@ -596,21 +617,21 @@ public final class PageSettingsBlock extends RecordAggregate {
         * <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. 
+        * <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 
+        * 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 
+        * 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) {
index f3730efd7fb51b0f3f812af63ff93c9ea31f0842..e3a01b476278df613c0962cd137134f581e19cfa 100644 (file)
@@ -27,6 +27,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.ContinueRecord;
 import org.apache.poi.hssf.record.DimensionsRecord;
 import org.apache.poi.hssf.record.EOFRecord;
 import org.apache.poi.hssf.record.FooterRecord;
@@ -47,19 +48,19 @@ 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)  
+
+               // 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) {
@@ -71,7 +72,6 @@ public final class TestPageSettingsBlock extends TestCase {
        /**
         * Bug 46840 occurred because POI failed to recognise HEADERFOOTER as part of the
         * {@link PageSettingsBlock}.
-        * 
         */
        public void testHeaderFooter_bug46840() {
 
@@ -157,7 +157,7 @@ public final class TestPageSettingsBlock extends TestCase {
        }
        /**
         * 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 
+        * 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>
@@ -238,7 +238,7 @@ public final class TestPageSettingsBlock extends TestCase {
        private static UnknownRecord ur(int sid, String hexData) {
                return new UnknownRecord(sid, HexRead.readFromString(hexData));
        }
-       
+
        /**
         * Excel tolerates missing header / footer records, but adds them (empty) in when re-saving.
         * This is not critical functionality but it has been decided to keep POI consistent with
@@ -257,7 +257,7 @@ public final class TestPageSettingsBlock extends TestCase {
                RecordCollector rc = new RecordCollector();
                psb.visitContainedRecords(rc);
                Record[] outRecs = rc.getRecords();
-               
+
                if (outRecs.length == 2) {
                        throw new AssertionFailedError("PageSettingsBlock didn't add missing header/footer records");
                }
@@ -266,11 +266,53 @@ public final class TestPageSettingsBlock extends TestCase {
                assertEquals(FooterRecord.class, outRecs[1].getClass());
                assertEquals(HCenterRecord.class, outRecs[2].getClass());
                assertEquals(VCenterRecord.class, outRecs[3].getClass());
-               
-               // make sure the added header / footer records are empty 
+
+               // make sure the added header / footer records are empty
                HeaderRecord hr = (HeaderRecord) outRecs[0];
                assertEquals("", hr.getText());
                FooterRecord fr = (FooterRecord) outRecs[1];
                assertEquals("", fr.getText());
        }
+
+       /**
+        * Apparently it's OK to have more than one PLS record.
+        * Attachment 23866 from bug 47415 had a PageSettingsBlock with two PLS records.  This file
+        * seems to open OK in Excel(2007) but both PLS records are removed (perhaps because the
+        * specified printers were not available on the testing machine).  Since the example file does
+        * not upset Excel, POI will preserve multiple PLS records.</p>
+        *
+        * As of June 2009, PLS is still uninterpreted by POI
+        */
+       public void testDuplicatePLS_bug47415() {
+               Record plsA = ur(UnknownRecord.PLS_004D, "BA AD F0 0D");
+               Record plsB = ur(UnknownRecord.PLS_004D, "DE AD BE EF");
+               Record contB1 = new ContinueRecord(HexRead.readFromString("FE ED"));
+               Record contB2 = new ContinueRecord(HexRead.readFromString("FA CE"));
+               Record[] recs = {
+                               new HeaderRecord("&LSales Figures"),
+                               new FooterRecord("&LInventory"),
+                               new HCenterRecord(),
+                               new VCenterRecord(),
+                               plsA,
+                               plsB, contB1, contB2, // make sure continuing PLS is still OK
+               };
+               RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+               PageSettingsBlock psb;
+               try {
+                       psb = new PageSettingsBlock(rs);
+               } catch (RecordFormatException e) {
+                       if ("Duplicate PageSettingsBlock record (sid=0x4d)".equals(e.getMessage())) {
+                               throw new AssertionFailedError("Identified bug 47415");
+                       }
+                       throw e;
+               }
+
+               // serialize the PSB to see what records come out
+               RecordCollector rc = new RecordCollector();
+               psb.visitContainedRecords(rc);
+               Record[] outRecs = rc.getRecords();
+
+               // records were assembled in standard order, so this simple check is OK
+               assertTrue(Arrays.equals(recs, outRecs));
+       }
 }