Bug 56911: Fix IndexOutOfBoundsException in PlfLfo.add() and add minimal test, howeve...

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

diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/LFOData.java b/src/scratchpad/src/org/apache/poi/hwpf/model/LFOData.java
index ff2e69c9c9b564d5b8af2192f57c0cdc68f126c5..89f822b223c2f79ca11b665e009325c80217d5df 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hwpf/model/LFOData.java
+++ b/src/scratchpad/src/org/apache/poi/hwpf/model/LFOData.java
@@ -19,6 +19,7 @@
 package org.apache.poi.hwpf.model;
 
 import java.io.IOException;
+import java.util.Arrays;
 
 import org.apache.poi.hwpf.model.io.HWPFOutputStream;
 import org.apache.poi.util.Internal;
@@ -88,4 +89,23 @@ public class LFOData
         }
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        LFOData lfoData = (LFOData) o;
+
+        if (_cp != lfoData._cp) return false;
+        // Probably incorrect - comparing Object[] arrays with Arrays.equals
+        return Arrays.equals(_rgLfoLvl, lfoData._rgLfoLvl);
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = _cp;
+        result = 31 * result + Arrays.hashCode(_rgLfoLvl);
+        return result;
+    }
 }

diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/PlfLfo.java b/src/scratchpad/src/org/apache/poi/hwpf/model/PlfLfo.java
index 5f02c1ed4a02fef96829b4da1f27f74ceb056801..90618da7435995e975bef5bbd9ffb091ece77e56 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hwpf/model/PlfLfo.java
+++ b/src/scratchpad/src/org/apache/poi/hwpf/model/PlfLfo.java
@@ -115,34 +115,28 @@ public class PlfLfo
 
     void add( LFO lfo, LFOData lfoData )
     {
-        final int newLfoMac = _lfoMac + 1;
+        // _lfoMac is the size of the array
+        _rgLfo = Arrays.copyOf(_rgLfo, _lfoMac + 1);
+        _rgLfo[_lfoMac] = lfo;
 
-        _rgLfo = Arrays.copyOf(_rgLfo, newLfoMac);
-        _rgLfo[_lfoMac + 1] = lfo;
+        _rgLfoData = Arrays.copyOf(_rgLfoData, _lfoMac + 1);
+        _rgLfoData[_lfoMac] = lfoData;
 
-        _rgLfoData = Arrays.copyOf(_rgLfoData, newLfoMac);
-        _rgLfoData[_lfoMac + 1] = lfoData;
-
-        this._lfoMac = newLfoMac;
+        _lfoMac = _lfoMac + 1;
     }
 
     @Override
-    public boolean equals( Object obj )
-    {
-        if ( this == obj )
+    public boolean equals( Object obj ) {
+        if (this == obj)
             return true;
-        if ( obj == null )
+        if (obj == null)
             return false;
-        if ( getClass() != obj.getClass() )
+        if (getClass() != obj.getClass())
             return false;
         PlfLfo other = (PlfLfo) obj;
-        if ( _lfoMac != other._lfoMac )
-            return false;
-        if ( !Arrays.equals( _rgLfo, other._rgLfo ) )
-            return false;
-        if ( !Arrays.equals( _rgLfoData, other._rgLfoData ) )
-            return false;
-        return true;
+        return _lfoMac == other._lfoMac &&
+                Arrays.equals(_rgLfo, other._rgLfo) &&
+                Arrays.equals(_rgLfoData, other._rgLfoData);
     }
 
     /**
@@ -167,6 +161,11 @@ public class PlfLfo
                 + " not found" );
     }
 
+    /**
+     * @param ilfo 1-based index
+     * @return The {@link LFO} stored at the given index
+     * @throws NoSuchElementException
+     */
     public LFO getLfo( int ilfo ) throws NoSuchElementException
     {
         if ( ilfo <= 0 || ilfo > _lfoMac )
@@ -177,6 +176,11 @@ public class PlfLfo
         return _rgLfo[ilfo - 1];
     }
 
+    /**
+     * @param ilfo 1-based index
+     * @return The {@link LFOData} stored at the given index
+     * @throws NoSuchElementException
+     */
     public LFOData getLfoData( int ilfo ) throws NoSuchElementException
     {
         if ( ilfo <= 0 || ilfo > _lfoMac )

diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/model/PlfLfoTest.java b/src/scratchpad/testcases/org/apache/poi/hwpf/model/PlfLfoTest.java
new file mode 100644 (file)
index 0000000..130c691
--- /dev/null
+++ b/src/scratchpad/testcases/org/apache/poi/hwpf/model/PlfLfoTest.java
@@ -0,0 +1,38 @@
+package org.apache.poi.hwpf.model;\r
+\r
+import org.junit.Test;\r
+\r
+import static org.junit.Assert.*;\r
+\r
+public class PlfLfoTest {\r
+    @Test\r
+    public void testAdd() {\r
+        PlfLfo p = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);\r
+        assertEquals(0, p.getLfoMac());\r
+        p.add(new LFO(new byte[] {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}, 0), new LFOData());\r
+        assertEquals(1, p.getLfoMac());\r
+        assertNotNull(p.getLfo(1));\r
+        assertNotNull(p.getLfoData(1));\r
+    }\r
+\r
+    @Test\r
+    public void testEquals() {\r
+        PlfLfo p = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);\r
+        PlfLfo p2 = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);\r
+        assertEquals(0, p.getLfoMac());\r
+        assertEquals(0, p2.getLfoMac());\r
+\r
+        assertTrue(p.equals(p2));\r
+        //noinspection ObjectEqualsNull\r
+        assertFalse(p.equals(null));\r
+\r
+        p.add(new LFO(new byte[] {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}, 0), new LFOData());\r
+        assertEquals(1, p.getLfoMac());\r
+\r
+        assertFalse(p.equals(p2));\r
+\r
+        p2.add(new LFO(new byte[] {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}, 0), new LFOData());\r
+        assertEquals(1, p2.getLfoMac());\r
+        assertTrue(p.equals(p2));\r
+    }\r
+}
\ No newline at end of file