[COLLECTIONS-747] MultiKey.getKeys class cast exception.

src/changes/changes.xml

@@ -129,6 +129,9 @@
     <action dev="ggregory" type="update" due-to="Gary Gregory">
       [test] Update org.easymock:easymock 4.1 -> 4.2.
     </action>
+    <action issue="COLLECTIONS-747" dev="ggregory" type="fix" due-to="Gary Gregory, Walter Laan">
+      MultiKey.getKeys class cast exception.
+    </action>
   </release>
   <release version="4.4" date="2019-07-05" description="Maintenance release.">
     <action issue="COLLECTIONS-710" dev="ggregory" type="fix" due-to="Yu Shi, Gary Gregory">

src/main/java/org/apache/commons/collections4/keyvalue/MultiKey.java

@@ -17,6 +17,7 @@
 package org.apache.commons.collections4.keyvalue;
 
 import java.io.Serializable;
+import java.lang.reflect.Array;
 import java.util.Arrays;
 import java.util.Objects;
 
@@ -51,8 +52,69 @@ public class MultiKey<K> implements Serializable {
     /** Serialisation version */
     private static final long serialVersionUID = 4465448607415788805L;
 
+    @SuppressWarnings("unchecked")
+    private static <T> Class<? extends T> getClass(final T value) {
+        return (Class<? extends T>) (value == null ? Object.class : value.getClass());
+    }
+
+    private static <T> Class<? extends T> getComponentType(final T... values) {
+        @SuppressWarnings("unchecked")
+        final Class<? extends T> rootClass = (Class<? extends T>) Object.class;
+        if (values == null) {
+            return rootClass;
+        }
+        Class<? extends T> prevClass = values.length > 0 ? getClass(values[0]) : rootClass;
+        for (int i = 1; i < values.length; i++) {
+            final Class<? extends T> classI = getClass(values[i]);
+            if (prevClass != classI) {
+                return rootClass;
+            }
+            prevClass = classI;
+        }
+        return prevClass;
+    }
+
+    private static <T> T[] newArray(final T key1, final T key2) {
+        @SuppressWarnings("unchecked")
+        final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2), 2);
+        array[0] = key1;
+        array[1] = key2;
+        return array;
+    }
+
+    private static <T> T[] newArray(final T key1, final T key2, final T key3) {
+        @SuppressWarnings("unchecked")
+        final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3), 3);
+        array[0] = key1;
+        array[1] = key2;
+        array[2] = key3;
+        return array;
+    }
+
+    private static <T> T[] newArray(final T key1, final T key2, final T key3, final T key4) {
+        @SuppressWarnings("unchecked")
+        final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3, key4), 4);
+        array[0] = key1;
+        array[1] = key2;
+        array[2] = key3;
+        array[3] = key4;
+        return array;
+    }
+
+    private static <T> T[] newArray(final T key1, final T key2, final T key3, final T key4, final T key5) {
+        @SuppressWarnings("unchecked")
+        final T[] array = (T[]) Array.newInstance(getComponentType(key1, key2, key3, key4, key5), 5);
+        array[0] = key1;
+        array[1] = key2;
+        array[2] = key3;
+        array[3] = key4;
+        array[4] = key5;
+        return array;
+    }
+
     /** The individual keys */
     private final K[] keys;
+
     /** The cached hashCode */
     private transient int hashCode;
 
@@ -65,11 +127,11 @@ public class MultiKey<K> implements Serializable {
      * @param key1  the first key
      * @param key2  the second key
      */
-    @SuppressWarnings("unchecked")
     public MultiKey(final K key1, final K key2) {
-        this((K[]) new Object[] { key1, key2 }, false);
+        this(newArray(key1, key2), false);
     }
 
+
     /**
      * Constructor taking three keys.
      * <p>
@@ -80,9 +142,8 @@ public class MultiKey<K> implements Serializable {
      * @param key2  the second key
      * @param key3  the third key
      */
-    @SuppressWarnings("unchecked")
     public MultiKey(final K key1, final K key2, final K key3) {
-        this((K[]) new Object[] {key1, key2, key3}, false);
+        this(newArray(key1, key2, key3), false);
     }
 
     /**
@@ -96,9 +157,8 @@ public class MultiKey<K> implements Serializable {
      * @param key3  the third key
      * @param key4  the fourth key
      */
-    @SuppressWarnings("unchecked")
     public MultiKey(final K key1, final K key2, final K key3, final K key4) {
-        this((K[]) new Object[] {key1, key2, key3, key4}, false);
+        this(newArray(key1, key2, key3, key4), false);
     }
 
     /**
@@ -113,9 +173,8 @@ public class MultiKey<K> implements Serializable {
      * @param key4  the fourth key
      * @param key5  the fifth key
      */
-    @SuppressWarnings("unchecked")
     public MultiKey(final K key1, final K key2, final K key3, final K key4, final K key5) {
-        this((K[]) new Object[] {key1, key2, key3, key4, key5}, false);
+        this(newArray(key1, key2, key3, key4, key5), false);
     }
 
     /**
@@ -160,51 +219,23 @@ public class MultiKey<K> implements Serializable {
     public MultiKey(final K[] keys, final boolean makeClone) {
         super();
         Objects.requireNonNull(keys, "keys");
-        if (makeClone) {
+        this.keys = makeClone ? keys.clone() : keys;
-            this.keys = keys.clone();
-        } else {
-            this.keys = keys;
-        }
-
         calculateHashCode(keys);
     }
 
-    //-----------------------------------------------------------------------
     /**
-     * Gets a clone of the array of keys.
+     * Calculate the hash code of the instance using the provided keys.
-     * <p>
+     * @param keys the keys to calculate the hash code for
-     * The keys should be immutable
-     * If they are not then they must not be changed.
-     *
-     * @return the individual keys
      */
-    public K[] getKeys() {
+    private void calculateHashCode(final Object[] keys)
-        return keys.clone();
+    {
+        int total = 0;
+        for (final Object key : keys) {
+            if (key != null) {
+                total ^= key.hashCode();
             }
-
-    /**
-     * Gets the key at the specified index.
-     * <p>
-     * The key should be immutable.
-     * If it is not then it must not be changed.
-     *
-     * @param index  the index to retrieve
-     * @return the key at the index
-     * @throws IndexOutOfBoundsException if the index is invalid
-     * @since 3.1
-     */
-    public K getKey(final int index) {
-        return keys[index];
         }
-
+        hashCode = total;
-    /**
-     * Gets the size of the list of keys.
-     *
-     * @return the size of the list of keys
-     * @since 3.1
-     */
-    public int size() {
-        return keys.length;
     }
 
     //-----------------------------------------------------------------------
@@ -229,6 +260,34 @@ public class MultiKey<K> implements Serializable {
         return false;
     }
 
+    /**
+     * Gets the key at the specified index.
+     * <p>
+     * The key should be immutable.
+     * If it is not then it must not be changed.
+     *
+     * @param index  the index to retrieve
+     * @return the key at the index
+     * @throws IndexOutOfBoundsException if the index is invalid
+     * @since 3.1
+     */
+    public K getKey(final int index) {
+        return keys[index];
+    }
+
+    //-----------------------------------------------------------------------
+    /**
+     * Gets a clone of the array of keys.
+     * <p>
+     * The keys should be immutable
+     * If they are not then they must not be changed.
+     *
+     * @return the individual keys
+     */
+    public K[] getKeys() {
+        return keys.clone();
+    }
+
     /**
      * Gets the combined hash code that is computed from all the keys.
      * <p>
@@ -244,31 +303,6 @@ public class MultiKey<K> implements Serializable {
         return hashCode;
     }
 
-    /**
-     * Gets a debugging string version of the key.
-     *
-     * @return a debugging string
-     */
-    @Override
-    public String toString() {
-        return "MultiKey" + Arrays.toString(keys);
-    }
-
-    /**
-     * Calculate the hash code of the instance using the provided keys.
-     * @param keys the keys to calculate the hash code for
-     */
-    private void calculateHashCode(final Object[] keys)
-    {
-        int total = 0;
-        for (final Object key : keys) {
-            if (key != null) {
-                total ^= key.hashCode();
-            }
-        }
-        hashCode = total;
-    }
-
     /**
      * Recalculate the hash code after deserialization. The hash code of some
      * keys might have change (hash codes based on the system hash code are
@@ -279,4 +313,24 @@ public class MultiKey<K> implements Serializable {
         calculateHashCode(keys);
         return this;
     }
+
+    /**
+     * Gets the size of the list of keys.
+     *
+     * @return the size of the list of keys
+     * @since 3.1
+     */
+    public int size() {
+        return keys.length;
+    }
+
+    /**
+     * Gets a debugging string version of the key.
+     *
+     * @return a debugging string
+     */
+    @Override
+    public String toString() {
+        return "MultiKey" + Arrays.toString(keys);
+    }
 }

src/test/java/org/apache/commons/collections4/keyvalue/MultiKeyTest.java

@@ -53,6 +53,7 @@ public class MultiKeyTest {
         }
 
     }
+    
     static class SystemHashCodeSimulatingKey implements Serializable {
 
         private static final long serialVersionUID = -1736147315703444603L;
@@ -82,13 +83,12 @@ public class MultiKeyTest {
             return this;
         }
     }
-
     Integer ONE = Integer.valueOf(1);
+
     Integer TWO = Integer.valueOf(2);
     Integer THREE = Integer.valueOf(3);
     Integer FOUR = Integer.valueOf(4);
     Integer FIVE = Integer.valueOf(5);
-
     //-----------------------------------------------------------------------
     @Test
     public void testConstructors() throws Exception {
@@ -294,4 +294,12 @@ public class MultiKeyTest {
         assertEquals(7, new MultiKey<>(new Integer[] { ONE, TWO, ONE, TWO, ONE, TWO, ONE }).size());
     }
 
+    @Test
+    public void testTwoArgCtor() {
+        MultiKeyTest key1 = new MultiKeyTest();
+        MultiKeyTest key2 = new MultiKeyTest();
+        MultiKeyTest[] keys = new MultiKey<>(key1, key2).getKeys();
+        assertNotNull(keys);
+    }
+
 }