diff --git a/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java b/src/java/org/apache/poi/hssf/eventusermodel/HSSFEventFactory.java index a3800561e2..1cd80d535b 100644 --- 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 index 0000000000..c74b864f79 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 index 0000000000..055629fa48 --- /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