Bug 56911: Fix IndexOutOfBoundsException in PlfLfo.add() and add minimal test, however these classes look quite untested and thus require more test-coverage to make them more robust

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

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;
+    }
 }

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 )

src/scratchpad/testcases/org/apache/poi/hwpf/model/PlfLfoTest.java

@@ -0,0 +1,38 @@
+package org.apache.poi.hwpf.model;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class PlfLfoTest {
+    @Test
+    public void testAdd() {
+        PlfLfo p = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);
+        assertEquals(0, p.getLfoMac());
+        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());
+        assertEquals(1, p.getLfoMac());
+        assertNotNull(p.getLfo(1));
+        assertNotNull(p.getLfoData(1));
+    }
+
+    @Test
+    public void testEquals() {
+        PlfLfo p = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);
+        PlfLfo p2 = new PlfLfo(new byte[] {0, 0, 0, 0}, 0, 0);
+        assertEquals(0, p.getLfoMac());
+        assertEquals(0, p2.getLfoMac());
+
+        assertTrue(p.equals(p2));
+        //noinspection ObjectEqualsNull
+        assertFalse(p.equals(null));
+
+        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());
+        assertEquals(1, p.getLfoMac());
+
+        assertFalse(p.equals(p2));
+
+        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());
+        assertEquals(1, p2.getLfoMac());
+        assertTrue(p.equals(p2));
+    }
+}