You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2008/06/06 10:32:54 UTC
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
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