fix field updating in AbstractEscherOptRecord;

prevent from shooting into foot by updating AbstractEscherOptRecord with incorrect values

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

diff --git a/src/java/org/apache/poi/ddf/AbstractEscherOptRecord.java b/src/java/org/apache/poi/ddf/AbstractEscherOptRecord.java
index 2b97b2885f98ca6b4f84e7574216b425a08da819..5ff11dea0c67e25dcceb2afcf3182d27a79bccb3 100644 (file)
--- a/src/java/org/apache/poi/ddf/AbstractEscherOptRecord.java
+++ b/src/java/org/apache/poi/ddf/AbstractEscherOptRecord.java
@@ -47,10 +47,11 @@ public abstract class AbstractEscherOptRecord extends EscherRecord
             EscherRecordFactory recordFactory )
     {
         int bytesRemaining = readHeader( data, offset );
+        short propertiesCount = readInstance( data, offset );
         int pos = offset + 8;
 
         EscherPropertyFactory f = new EscherPropertyFactory();
-        properties = f.createProperties( data, pos, getInstance() );
+        properties = f.createProperties( data, pos, propertiesCount );
         return bytesRemaining + 8;
     }
 

diff --git a/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java b/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java
index c220dcd6c2f8ac98a5584f613a176661b9d9268e..4b39b81599154024f7a12b7613ebff1701a01607 100644 (file)
--- a/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java
+++ b/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java
@@ -21,6 +21,8 @@ import java.lang.reflect.Constructor;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.poi.util.LittleEndian;
+
 /**
  * Generates escher records when provided the byte array containing those records.
  *
@@ -55,43 +57,45 @@ public class DefaultEscherRecordFactory implements EscherRecordFactory {
      * @return The generated escher record
      */
     public EscherRecord createRecord(byte[] data, int offset) {
-        EscherRecord.EscherRecordHeader header = EscherRecord.EscherRecordHeader.readHeader( data, offset );
+        short options = LittleEndian.getShort( data, offset );
+        short recordId = LittleEndian.getShort( data, offset + 2 );
+        // int remainingBytes = LittleEndian.getInt( data, offset + 4 );
 
         // Options of 0x000F means container record
         // However, EscherTextboxRecord are containers of records for the
         //  host application, not of other Escher records, so treat them
         //  differently
-        if ( ( header.getOptions() & (short) 0x000F ) == (short) 0x000F
-             && header.getRecordId() != EscherTextboxRecord.RECORD_ID ) {
+        if ( ( options & (short) 0x000F ) == (short) 0x000F
+             && recordId != EscherTextboxRecord.RECORD_ID ) {
             EscherContainerRecord r = new EscherContainerRecord();
-            r.setRecordId( header.getRecordId() );
-            r.setOptions( header.getOptions() );
+            r.setRecordId( recordId );
+            r.setOptions( options );
             return r;
         }
 
-        if (header.getRecordId() >= EscherBlipRecord.RECORD_ID_START
-                && header.getRecordId() <= EscherBlipRecord.RECORD_ID_END) {
+        if (recordId >= EscherBlipRecord.RECORD_ID_START
+                && recordId <= EscherBlipRecord.RECORD_ID_END) {
             EscherBlipRecord r;
-            if (header.getRecordId() == EscherBitmapBlip.RECORD_ID_DIB ||
-                    header.getRecordId() == EscherBitmapBlip.RECORD_ID_JPEG ||
-                    header.getRecordId() == EscherBitmapBlip.RECORD_ID_PNG)
+            if (recordId == EscherBitmapBlip.RECORD_ID_DIB ||
+                    recordId == EscherBitmapBlip.RECORD_ID_JPEG ||
+                    recordId == EscherBitmapBlip.RECORD_ID_PNG)
             {
                 r = new EscherBitmapBlip();
             }
-            else if (header.getRecordId() == EscherMetafileBlip.RECORD_ID_EMF ||
-                    header.getRecordId() == EscherMetafileBlip.RECORD_ID_WMF ||
-                    header.getRecordId() == EscherMetafileBlip.RECORD_ID_PICT)
+            else if (recordId == EscherMetafileBlip.RECORD_ID_EMF ||
+                    recordId == EscherMetafileBlip.RECORD_ID_WMF ||
+                    recordId == EscherMetafileBlip.RECORD_ID_PICT)
             {
                 r = new EscherMetafileBlip();
             } else {
                 r = new EscherBlipRecord();
             }
-            r.setRecordId( header.getRecordId() );
-            r.setOptions( header.getOptions() );
+            r.setRecordId( recordId );
+            r.setOptions( options );
             return r;
         }
 
-        Constructor<? extends EscherRecord> recordConstructor = recordsMap.get(Short.valueOf(header.getRecordId()));
+        Constructor<? extends EscherRecord> recordConstructor = recordsMap.get(Short.valueOf(recordId));
         EscherRecord escherRecord = null;
         if (recordConstructor == null) {
             return new UnknownEscherRecord();
@@ -101,8 +105,8 @@ public class DefaultEscherRecordFactory implements EscherRecordFactory {
         } catch (Exception e) {
             return new UnknownEscherRecord();
         }
-        escherRecord.setRecordId(header.getRecordId());
-        escherRecord.setOptions(header.getOptions());
+        escherRecord.setRecordId(recordId);
+        escherRecord.setOptions(options);
         return escherRecord;
     }
 

diff --git a/src/java/org/apache/poi/ddf/EscherOptRecord.java b/src/java/org/apache/poi/ddf/EscherOptRecord.java
index af009054cece9f5f97065abffbb697e21e412cb9..33c894cab9ff4abea32517e1bb89df7417edd87c 100644 (file)
--- a/src/java/org/apache/poi/ddf/EscherOptRecord.java
+++ b/src/java/org/apache/poi/ddf/EscherOptRecord.java
@@ -26,15 +26,25 @@ package org.apache.poi.ddf;
  */
 public class EscherOptRecord extends AbstractEscherOptRecord
 {
-    public static final short RECORD_ID = (short) 0xF00B;
     public static final String RECORD_DESCRIPTION = "msofbtOPT";
+    public static final short RECORD_ID = (short) 0xF00B;
+
+    @Override
+    public short getInstance()
+    {
+        setInstance( (short) properties.size() );
+        return super.getInstance();
+    }
 
     /**
      * Automatically recalculate the correct option
      */
+    @Deprecated
     public short getOptions()
     {
-        setOptions( (short) ( ( properties.size() << 4 ) | 0x3 ) );
+        // update values
+        getInstance();
+        getVersion();
         return super.getOptions();
     }
 
@@ -42,4 +52,21 @@ public class EscherOptRecord extends AbstractEscherOptRecord
     {
         return "Opt";
     }
+
+    @Override
+    public short getVersion()
+    {
+        setVersion( (short) 0x3 );
+        return super.getVersion();
+    }
+
+    @Override
+    public void setVersion( short value )
+    {
+        if ( value != 0x3 )
+            throw new IllegalArgumentException( RECORD_DESCRIPTION
+                    + " can have only '0x3' version" );
+
+        super.setVersion( value );
+    }
 }

diff --git a/src/java/org/apache/poi/ddf/EscherRecord.java b/src/java/org/apache/poi/ddf/EscherRecord.java
index 55840fa2e0d790222a3105b0974403a768ff8269..1055875e9f8fa2d9a0eb55a6f10b2d48d26a6d73 100644 (file)
--- a/src/java/org/apache/poi/ddf/EscherRecord.java
+++ b/src/java/org/apache/poi/ddf/EscherRecord.java
@@ -77,11 +77,24 @@ public abstract class EscherRecord {
      * @return          the number of bytes remaining in this record.  This
      *                  may include the children if this is a container.
      */
-    protected int readHeader( byte[] data, int offset ) {
-        EscherRecordHeader header = EscherRecordHeader.readHeader(data, offset);
-        _options = header.getOptions();
-        _recordId = header.getRecordId();
-        return header.getRemainingBytes();
+    protected int readHeader( byte[] data, int offset )
+    {
+        _options = LittleEndian.getShort( data, offset );
+        _recordId = LittleEndian.getShort( data, offset + 2 );
+        int remainingBytes = LittleEndian.getInt( data, offset + 4 );
+        return remainingBytes;
+    }
+
+    /**
+     * Read the options field from header and return instance part of it.
+     * @param data      the byte array to read from
+     * @param offset    the offset to start reading from
+     * @return          value of instance part of options field
+     */
+    protected static short readInstance( byte data[], int offset )
+    {
+        final short options = LittleEndian.getShort( data, offset );
+        return fInstance.getShortValue( options );
     }
 
     /**
@@ -112,6 +125,9 @@ public abstract class EscherRecord {
      */
     @Deprecated
     public void setOptions( short options ) {
+        // call to handle correct/incorrect values
+        setVersion( fVersion.getShortValue( options ) );
+        setInstance( fInstance.getShortValue( options ) );
         _options = options;
     }
 
@@ -252,7 +268,7 @@ public abstract class EscherRecord {
      */
     public void setInstance( short value )
     {
-        fInstance.setShortValue( _options, value );
+        _options = fInstance.setShortValue( _options, value );
     }
 
     /**
@@ -273,64 +289,6 @@ public abstract class EscherRecord {
      */
     public void setVersion( short value )
     {
-        fVersion.setShortValue( _options, value );
-    }
-
-    /**
-     * This class reads the standard escher header.
-     */
-    static class EscherRecordHeader
-    {
-        private short options;
-        private short recordId;
-        private int remainingBytes;
-
-        private EscherRecordHeader() {
-            // fields uninitialised
-        }
-
-        public static EscherRecordHeader readHeader( byte[] data, int offset )
-        {
-            EscherRecordHeader header = new EscherRecordHeader();
-            header.options = LittleEndian.getShort(data, offset);
-            header.recordId = LittleEndian.getShort(data, offset + 2);
-            header.remainingBytes = LittleEndian.getInt( data, offset + 4 );
-            return header;
-        }
-
-        public byte getVersion()
-        {
-            return (byte) fVersion.getShortValue( options );
-        }
-
-        public short getInstance()
-        {
-            return fInstance.getShortValue( options );
-        }
-
-        public short getOptions()
-        {
-            return options;
-        }
-
-        public short getRecordId()
-        {
-            return recordId;
-        }
-
-        public int getRemainingBytes()
-        {
-            return remainingBytes;
-        }
-
-        public String toString()
-        {
-            return "EscherRecordHeader{" +
-                    "ver=" + getVersion() +
-                    "instance=" + getInstance() +
-                    ", recordId=" + recordId +
-                    ", remainingBytes=" + remainingBytes +
-                    "}";
-        }
+        _options = fVersion.setShortValue( _options, value );
     }
 }