poi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r663855 - in /poi/trunk/src: documentation/content/xdocs/changes.xml documentation/content/xdocs/status.xml java/org/apache/poi/hssf/record/ObjRecord.java testcases/org/apache/poi/hssf/record/TestObjRecord.java
Date Fri, 06 Jun 2008 08:32:54 GMT
Author: josh
Date: Fri Jun  6 01:32:54 2008
New Revision: 663855

URL: http://svn.apache.org/viewvc?rev=663855&view=rev
Log:
Fix for 45133 - OBJ Record (5Dh) needs to pad the sub-record data to a 4-byte boundary

Modified:
    poi/trunk/src/documentation/content/xdocs/changes.xml
    poi/trunk/src/documentation/content/xdocs/status.xml
    poi/trunk/src/java/org/apache/poi/hssf/record/ObjRecord.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java

Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=663855&r1=663854&r2=663855&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Fri Jun  6 01:32:54 2008
@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to
pad the sub-record data to a 4-byte boundary</action>
            <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce
RowRecordsAggregate before ValueRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas()
to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats
like [Black]YYYY as being date based</action>

Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=663855&r1=663854&r2=663855&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Fri Jun  6 01:32:54 2008
@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to
pad the sub-record data to a 4-byte boundary</action>
            <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce
RowRecordsAggregate before ValueRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas()
to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats
like [Black]YYYY as being date based</action>

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/ObjRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/ObjRecord.java?rev=663855&r1=663854&r2=663855&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/ObjRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/ObjRecord.java Fri Jun  6 01:32:54 2008
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,27 +15,21 @@
    limitations under the License.
 ==================================================================== */
         
-
-
 package org.apache.poi.hssf.record;
 
-
-
-import org.apache.poi.util.*;
-
 import java.io.ByteArrayInputStream;
-import java.util.List;
-import java.util.Iterator;
 import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.poi.util.LittleEndian;
 
 /**
  * The obj record is used to hold various graphic objects and controls.
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class ObjRecord
-    extends Record
-{
+public final class ObjRecord extends Record {
     public final static short      sid                             = 0x5D;
     private List subrecords;
 
@@ -47,6 +40,7 @@
     public ObjRecord()
     {
         subrecords = new ArrayList(2);
+        // TODO - ensure 2 sub-records (ftCmo  15h, and ftEnd  00h) are always created
     }
 
     /**
@@ -80,6 +74,7 @@
         //following wont work properly
         int subSize = 0;
         byte[] subRecordData = in.readRemainder();
+
         RecordInputStream subRecStream = new RecordInputStream(new ByteArrayInputStream(subRecordData));
         while(subRecStream.hasNextRecord()) {
           subRecStream.nextRecord();
@@ -89,28 +84,19 @@
         }
 
         /**
-         * Check if the RecordInputStream skipped EndSubRecord,
-         * if it did then append it explicitly.
-         * See Bug 41242 for details.
+         * Add the EndSubRecord explicitly.
+         * 
+         * TODO - the reason the EndSubRecord is always skipped is because its 'sid' is zero
and
+         * that causes subRecStream.hasNextRecord() to return false.
+         * There may be more than the size of EndSubRecord left-over, if there is any padding

+         * after that record.  The content of the EndSubRecord and the padding is all zeros.
+         * So there's not much to look at past the last substantial record.
+         * 
+         * See Bugs 41242/45133 for details.
          */
-        if (subRecordData.length - subSize == 4){
+        if (subRecordData.length - subSize >= 4) {
             subrecords.add(new EndSubRecord());
         }
-
-        /* JMH the size present/not present in the code below
-           needs to be considered in the RecordInputStream??
-        int pos = offset;
-        while (pos - offset <= size-2) // atleast one "short" must be present
-        {
-            short subRecordSid = LittleEndian.getShort(data, pos);
-            short subRecordSize = -1; // set default to "< 0"
-            if (pos-offset <= size-4) { // see if size info is present, else default to
-1
-                subRecordSize = LittleEndian.getShort(data, pos + 2);
-            }
-            Record subRecord = SubRecord.createSubRecord(subRecordSid, subRecordSize, data,
pos + 4);
-            subrecords.add(subRecord);
-            pos += subRecord.getRecordSize();
-        }*/
     }
 
     public String toString()
@@ -140,6 +126,8 @@
             Record record = (Record) iterator.next();
             pos += record.serialize(pos, data);
         }
+        // assume padding (if present) does not need to be written.
+        // it is probably zero already, and it probably doesn't matter anyway
 
         return getRecordSize();
     }
@@ -155,7 +143,9 @@
             Record record = (Record) iterator.next();
             size += record.getRecordSize();
         }
-        return 4  + size;
+        int oddBytes = size & 0x03;
+        int padding = oddBytes == 0 ? 0 : 4 - oddBytes;
+        return 4  + size + padding;
     }
 
     public short getSid()
@@ -192,9 +182,4 @@
 
         return rec;
     }
-
-}  // END OF CLASS
-
-
-
-
+}

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java?rev=663855&r1=663854&r2=663855&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java Fri Jun  6 01:32:54
2008
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -18,18 +17,19 @@
 
 package org.apache.poi.hssf.record;
 
-import junit.framework.*;
-
 import java.util.Arrays;
 import java.util.List;
 
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
 /**
  * Tests the serialization and deserialization of the ObjRecord class works correctly.
  * Test data taken directly from a real Excel file.
  *
  * @author Yegor Kozlov
  */
-public class TestObjRecord extends TestCase {
+public final class TestObjRecord extends TestCase {
     /**
      * OBJ record data containing two sub-records.
      * The data taken directly from a real Excel file.
@@ -38,22 +38,27 @@
      *     [ftCmo]
      *     [ftEnd]
      */
-    public static byte[] recdata = {
+    private static final byte[] recdata = {
         0x15, 0x00, 0x12, 0x00, 0x06, 0x00, 0x01, 0x00, 0x11, 0x60,
         (byte)0xF4, 0x02, 0x41, 0x01, 0x14, 0x10, 0x1F, 0x02, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+        // TODO - this data seems to require two extra bytes padding. not sure where original
file is.
+        // it's not bug 38607 attachment 17639
     };
 
+    private static final byte[] recdataNeedingPadding = {
+    	21, 0, 18, 0, 0, 0, 1, 0, 17, 96, 0, 0, 0, 0, 56, 111, -52, 3, 0, 0, 0, 0, 6, 0, 2,
0, 0, 0, 0, 0, 0, 0
+    };
 
-    public void testLoad() throws Exception {
+    public void testLoad() {
         ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length,
recdata));
 
-        assertEquals( recdata.length, record.getRecordSize() - 4);
+        assertEquals(28, record.getRecordSize() - 4);
 
         List subrecords = record.getSubRecords();
-        assertEquals( 2, subrecords.size() );
-        assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord);
-        assertTrue( subrecords.get(1) instanceof EndSubRecord );
+        assertEquals(2, subrecords.size() );
+        assertTrue(subrecords.get(0) instanceof CommonObjectDataSubRecord);
+        assertTrue(subrecords.get(1) instanceof EndSubRecord );
 
     }
 
@@ -61,8 +66,8 @@
         ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length,
recdata));
 
         byte [] recordBytes = record.serialize();
-        assertEquals(recdata.length, recordBytes.length - 4);
-        byte[] subData = new byte[recordBytes.length - 4];
+        assertEquals(28, recordBytes.length - 4);
+        byte[] subData = new byte[recdata.length];
         System.arraycopy(recordBytes, 4, subData, 0, subData.length);
         assertTrue(Arrays.equals(recdata, subData));
     }
@@ -92,4 +97,20 @@
         assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord);
         assertTrue( subrecords.get(1) instanceof EndSubRecord );
     }
+    
+    public void testReadWriteWithPadding_bug45133() {
+        ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdataNeedingPadding.length,
recdataNeedingPadding));
+        
+        if (record.getRecordSize() == 34) {
+        	throw new AssertionFailedError("Identified bug 45133");
+        }
+
+        assertEquals(36, record.getRecordSize());
+
+        List subrecords = record.getSubRecords();
+        assertEquals(3, subrecords.size() );
+        assertEquals(CommonObjectDataSubRecord.class, subrecords.get(0).getClass());
+        assertEquals(GroupMarkerSubRecord.class, subrecords.get(1).getClass());
+        assertEquals(EndSubRecord.class, subrecords.get(2).getClass());
+    }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org


Mime
View raw message