Fix for bug #42844 - Include some of the crazy continue record handling that we have...

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

diff --git a/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java b/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java
index a3800561e20f0051d0cb2607d2613e2c7f957833..1cd80d535bdd6aa9957c53aa206992e1a001964c 100644 (file)
--- a/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java
+++ b/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java
@@ -27,6 +27,10 @@ import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.hssf.record.RecordFactory;
 import org.apache.poi.hssf.record.ContinueRecord;
+import org.apache.poi.hssf.record.DrawingRecord;
+import org.apache.poi.hssf.record.DrawingGroupRecord;
+import org.apache.poi.hssf.record.ObjRecord;
+import org.apache.poi.hssf.record.TextObjectRecord;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 
 /**
@@ -140,11 +144,12 @@ public class HSSFEventFactory
                {
                   
                        Record rec       = null;
+                       Record lastRec   = null;
+                       DrawingRecord lastDrawingRecord = new DrawingRecord();
 
                        while (in.hasNextRecord())
                        {
-                          in.nextRecord();
-
+                               in.nextRecord();
                                sid = in.getSid();;
                 
                 //
@@ -190,9 +195,34 @@ public class HSSFEventFactory
                                        // if there is only one
                                        // records, it will go here too.
                                }
-                               else
-                                       {
-                                  throw new RecordFormatException("Records should handle ContinueRecord internally. Should not see this exception");
+                               else {
+                                       // Normally, ContinueRecords are handled internally
+                                       // However, in a few cases, there is a gap between a record at
+                                       //  its Continue, so we have to handle them specially
+                                       // This logic is much like in RecordFactory.createRecords()
+                                       Record[] recs = RecordFactory.createRecord(in);
+                                       ContinueRecord crec = (ContinueRecord)recs[0];
+                                       if((lastRec instanceof ObjRecord) || (lastRec instanceof TextObjectRecord)) {
+                                               // You can have Obj records between a DrawingRecord
+                                               //  and its continue!
+                                               lastDrawingRecord.processContinueRecord( crec.getData() );
+                                               // Trigger them on the drawing record, now it's complete
+                                               rec = lastDrawingRecord;
+                                       }
+                                       else if((lastRec instanceof DrawingGroupRecord)) {
+                                               ((DrawingGroupRecord)lastRec).processContinueRecord(crec.getData());
+                                               // Trigger them on the drawing record, now it's complete
+                                               rec = lastRec;
+                                       }
+                                       else {
+                                               throw new RecordFormatException("Records should handle ContinueRecord internally. Should not see this exception");
+                                       }
+                               }
+
+                               // Update our tracking of the last record
+                               lastRec = rec;
+                               if(rec instanceof DrawingRecord) {
+                                       lastDrawingRecord = (DrawingRecord)rec;
                                }
                        }
                        if (rec != null)

diff --git a/src/testcases/org/apache/poi/hssf/data/ContinueRecordProblem.xls b/src/testcases/org/apache/poi/hssf/data/ContinueRecordProblem.xls
new file mode 100644 (file)
index 0000000..c74b864
Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/ContinueRecordProblem.xls differ

diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java
new file mode 100644 (file)
index 0000000..055629f
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java
@@ -0,0 +1,88 @@
+/* ====================================================================
+   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.eventusermodel;
+import org.apache.poi.hssf.eventusermodel.HSSFEventFactory;
+import org.apache.poi.hssf.eventusermodel.HSSFListener;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.util.ArrayList;
+
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.ContinueRecord;
+import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+
+import junit.framework.TestCase;
+
+public class TestHSSFEventFactory extends TestCase {
+       private String dirname;
+       
+       public TestHSSFEventFactory() {
+               dirname = System.getProperty("HSSF.testdata.path");
+       }
+
+       public void testWithMissingRecords() throws Exception {
+               File f = new File(dirname + "/SimpleWithSkip.xls");
+
+               HSSFRequest req = new HSSFRequest();
+               MockHSSFListener mockListen = new MockHSSFListener();
+               req.addListenerForAllRecords(mockListen);
+               
+               POIFSFileSystem fs = new POIFSFileSystem(new FileInputStream(f));
+               HSSFEventFactory factory = new HSSFEventFactory();
+               factory.processWorkbookEvents(req, fs);
+
+               // Check we got the records
+               assertTrue( mockListen.records.size() > 100 );
+       }
+
+       public void testWithCrazyContinueRecords() throws Exception {
+               // Some files have crazy ordering of their continue records
+               // Check that we don't break on them (bug #42844)
+
+               File f = new File(dirname + "/ContinueRecordProblem.xls");
+               
+               HSSFRequest req = new HSSFRequest();
+               MockHSSFListener mockListen = new MockHSSFListener();
+               req.addListenerForAllRecords(mockListen);
+               
+               POIFSFileSystem fs = new POIFSFileSystem(new FileInputStream(f));
+               HSSFEventFactory factory = new HSSFEventFactory();
+               factory.processWorkbookEvents(req, fs);
+
+               // Check we got the records
+               assertTrue( mockListen.records.size() > 100 );
+
+               // And none of them are continue ones
+               Record[] r = (Record[])mockListen.records.toArray( 
+                                                       new Record[mockListen.records.size()] );
+               for(int i=0; i<r.length; i++) {
+                       assertFalse( r[i] instanceof ContinueRecord );
+               }
+       }
+
+
+       private static class MockHSSFListener implements HSSFListener {
+               private MockHSSFListener() {}
+               private ArrayList records = new ArrayList();
+
+               public void processRecord(Record record) {
+                       records.add(record);
+               }
+       }
+}