Fixed bug 17373 - incorrect handling of continue records after unknown records corrupting a template. Patch submitted by Csaba Nagy -- thanks

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

src/java/org/apache/poi/hssf/eventmodel/EventRecordFactory.java

@@ -89,7 +89,6 @@ import org.apache.poi.hssf.record.FnGroupCountRecord;
 import org.apache.poi.hssf.record.FontRecord;
 import org.apache.poi.hssf.record.FooterRecord;
 import org.apache.poi.hssf.record.FormatRecord;
-import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.GridsetRecord;
 import org.apache.poi.hssf.record.GutsRecord;
 import org.apache.poi.hssf.record.HCenterRecord;
@@ -151,6 +150,7 @@ import org.apache.poi.util.LittleEndian;
  * @author Andrew C. Oliver (acoliver@apache.org) - probably to blame for the bugs (so yank his chain on the list)
  * @author Marc Johnson (mjohnson at apache dot org) - methods taken from RecordFactory
  * @author Glen Stampoultzis (glens at apache.org) - methods taken from RecordFactory
+ * @author Csaba Nagy (ncsaba at yahoo dot com)
  */
 public class EventRecordFactory
 {
@@ -289,8 +289,6 @@ public class EventRecordFactory
      * @param in the InputStream from which the records will be
      *           obtained
      *
-     * @return an array of Records created from the InputStream
-     *
      * @exception RecordFormatException on error processing the
      *            InputStream
      */
@@ -337,7 +335,9 @@ public class EventRecordFactory
 
                         if (record != null)
                         {
-                            if (rectype == ContinueRecord.sid)
+                            if (rectype == ContinueRecord.sid &&
+                                ! (last_record instanceof ContinueRecord) && // include continuation records after
+                                ! (last_record instanceof UnknownRecord) )   // unknown records or previous continuation records
                             {
                                 if (last_record == null)
                                 {

src/java/org/apache/poi/hssf/record/ContinueRecord.java

@@ -65,6 +65,7 @@ import org.apache.poi.util.LittleEndian;
  *               stream; content is tailored to that prior record<P>
  * @author Marc Johnson (mjohnson at apache dot org)
  * @author Andrew C. Oliver (acoliver at apache dot org)
+ * @author Csaba Nagy (ncsaba at yahoo dot com)
  * @version 2.0-pre
  */
 
@@ -116,17 +117,19 @@ public class ContinueRecord
     public byte [] serialize()
     {
         byte[] retval = new byte[ field_1_data.length + 4 ];
-
+        serialize(0, retval);
-        LittleEndian.putShort(retval, 0, sid);
-        LittleEndian.putShort(retval, 2, ( short ) field_1_data.length);
-        System.arraycopy(field_1_data, 0, retval, 4, field_1_data.length);
         return retval;
     }
 
     public int serialize(int offset, byte [] data)
     {
-        throw new RecordFormatException(
+
-            "You're not supposed to serialize Continue records like this directly");
+        LittleEndian.putShort(data, offset, sid);
+        LittleEndian.putShort(data, offset + 2, ( short ) field_1_data.length);
+        System.arraycopy(field_1_data, 0, data, offset + 4, field_1_data.length);
+        return field_1_data.length + 4;
+        // throw new RecordFormatException(
+        //    "You're not supposed to serialize Continue records like this directly");
     }
 
     /**
@@ -212,7 +215,7 @@ public class ContinueRecord
 
     protected void fillFields(byte [] ignored_parm1, short ignored_parm2)
     {
-
+        this.field_1_data = ignored_parm1;
         // throw new RecordFormatException("Are you crazy?  Don't fill a continue record");
         // do nothing
     }

src/java/org/apache/poi/hssf/record/RecordFactory.java

@@ -69,10 +69,11 @@ import org.apache.poi.util.LittleEndian;
  * Description:  Takes a stream and outputs an array of Record objects.<P>
  *
  * @deprecated use EventRecordFactory instead
- * @see org.apache.poi.hssf.record.EventRecordFactory
+ * @see org.apache.poi.hssf.eventmodel.EventRecordFactory
  * @author Andrew C. Oliver (acoliver at apache dot org)
  * @author Marc Johnson (mjohnson at apache dot org)
  * @author Glen Stampoultzis (glens at apache.org)
+ * @author Csaba Nagy (ncsaba at yahoo dot com)
  * @version 1.0-pre
  */
 
@@ -208,7 +209,9 @@ public class RecordFactory
 
                         if (record != null)
                         {
-                            if (rectype == ContinueRecord.sid)
+                            if (rectype == ContinueRecord.sid &&
+                                ! (last_record instanceof ContinueRecord) && // include continuation records after
+                                ! (last_record instanceof UnknownRecord) )   // unknown records or previous continuation records
                             {
                                 if (last_record == null)
                                 {

src/testcases/org/apache/poi/hssf/eventmodel/TestEventRecordFactory.java

@@ -3,10 +3,13 @@ package org.apache.poi.hssf.eventmodel;
 import java.io.ByteArrayInputStream;
 import java.io.EOFException;
 import java.util.Iterator;
+import java.util.Arrays;
 
 import org.apache.poi.hssf.record.BOFRecord;
 import org.apache.poi.hssf.record.EOFRecord;
 import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.UnknownRecord;
+import org.apache.poi.hssf.record.ContinueRecord;
 
 import junit.framework.TestCase;
 
@@ -14,6 +17,7 @@ import junit.framework.TestCase;
  * enclosing_type describe the purpose here
  * 
  * @author Andrew C. Oliver acoliver@apache.org
+ * @author Csaba Nagy (ncsaba at yahoo dot com)
  */
 public class TestEventRecordFactory extends TestCase
 {
@@ -173,4 +177,59 @@ public class TestEventRecordFactory extends TestCase
       //  fail("not implemented");
     }
     
+
+    /**
+     * TEST NAME:  Test Creating ContinueRecords After Unknown Records From An InputStream <P>
+     * OBJECTIVE:  Test that the RecordFactory given an InputStream
+     *             constructs the expected records.<P>
+     * SUCCESS:    Record factory creates the expected records.<P>
+     * FAILURE:    The wrong records are created or contain the wrong values <P>
+     *
+     */
+     public void testContinuedUnknownRecord()
+     {
+        final byte[]   data    = new byte[]
+        {
+            0, -1, 0, 0, // an unknown record with 0 length
+            0x3C , 0, 3, 0, 1, 2, 3, // a continuation record with 3 bytes of data
+            0x3C , 0, 1, 0, 4 // one more continuation record with 1 byte of data
+        };
+
+        final int[] recCnt = { 0 };
+        final int[] offset = { 0 };
+        factory.registerListener(
+          new ERFListener() {
+              private String[] expectedRecordTypes = {
+                  UnknownRecord.class.getName(),
+                  ContinueRecord.class.getName(),
+                  ContinueRecord.class.getName()
+              };
+              public boolean processRecord(Record rec)
+              {
+                  // System.out.println(rec.toString());
+                  assertEquals(
+                    "Record type",
+                    expectedRecordTypes[recCnt[0]],
+                    rec.getClass().getName()
+                  );
+                  compareData(rec, "Record " + recCnt[0] + ": ");
+                  recCnt[0]++;
+                  return true;
+              }
+              private void compareData(Record record, String message) {
+                  byte[] recData = record.serialize();
+                  for (int i = 0; i < recData.length; i++) {
+                      assertEquals(message + " data byte " + i, data[offset[0]++], recData[i]);
+                  }
+              }
+          },
+          new short[] {-256, 0x3C}
+        );
+
+        factory.processRecords(new ByteArrayInputStream(data));
+        assertEquals("nr. of processed records", 3, recCnt[0]);
+        assertEquals("nr. of processed bytes", data.length, offset[0]);
+    }
+
+
 }

src/testcases/org/apache/poi/hssf/record/TestRecordFactory.java

@@ -55,12 +55,15 @@
 
 package org.apache.poi.hssf.record;
 
+import java.io.ByteArrayInputStream;
+
 import junit.framework.TestCase;
 
 /**
  * Tests the record factory
  * @author Glen Stampoultzis (glens at apache.org)
  * @author Andrew C. Oliver (acoliver at apache dot org)
+ * @author Csaba Nagy (ncsaba at yahoo dot com)
  */
 
 public class TestRecordFactory
@@ -158,6 +161,47 @@ public class TestRecordFactory
         assertEquals(21, numberRecord.getXFIndex());
     }
 
+    /**
+     * TEST NAME:  Test Creating ContinueRecords After Unknown Records From An InputStream <P>
+     * OBJECTIVE:  Test that the RecordFactory given an InputStream
+     *             constructs the expected array of records.<P>
+     * SUCCESS:    Record factory creates the expected records.<P>
+     * FAILURE:    The wrong records are created or contain the wrong values <P>
+     *
+     */
+    public void testContinuedUnknownRecord()
+    {
+        byte[]   data    = new byte[]
+        {
+            0, -1, 0, 0, // an unknown record with 0 length
+            0x3C , 0, 3, 0, 1, 2, 3, // a continuation record with 3 bytes of data
+            0x3C , 0, 1, 0, 4 // one more continuation record with 1 byte of data
+        };
+
+        ByteArrayInputStream bois = new ByteArrayInputStream(data);
+        Record[] records = (Record[])
+          RecordFactory.createRecords(bois).toArray(new Record[0]);
+        assertEquals("Created record count", 3, records.length);
+        assertEquals("1st record's type",
+                     UnknownRecord.class.getName(),
+                     records[ 0 ].getClass().getName());
+        assertEquals("1st record's sid", (short)-256, records[0].getSid());
+        assertEquals("2nd record's type",
+                     ContinueRecord.class.getName(),
+                     records[ 1 ].getClass().getName());
+        ContinueRecord record = (ContinueRecord) records[1];
+        assertEquals("2nd record's sid", 0x3C, record.getSid());
+        assertEquals("1st data byte", 1, record.getData()[ 0 ]);
+        assertEquals("2nd data byte", 2, record.getData()[ 1 ]);
+        assertEquals("3rd data byte", 3, record.getData()[ 2 ]);
+        assertEquals("3rd record's type",
+                     ContinueRecord.class.getName(),
+                     records[ 2 ].getClass().getName());
+        record = (ContinueRecord) records[2];
+        assertEquals("3nd record's sid", 0x3C, record.getSid());
+        assertEquals("4th data byte", 4, record.getData()[ 0 ]);
+    }
+
     public static void main(String [] ignored_args)
     {
         System.out