You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2010/01/16 13:39:20 UTC

svn commit: r899929 - in /commons/proper/lang/trunk: src/main/java/org/apache/commons/lang3/ArrayUtils.java src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java xdocs/changes.xml

Author: sebb
Date: Sat Jan 16 12:39:20 2010
New Revision: 899929

URL: http://svn.apache.org/viewvc?rev=899929&view=rev
Log:
LANG-571 ArrayUtils.add(T[] array[, offset], T element) can create unexpected ClassCastException

Modified:
    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
    commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java
    commons/proper/lang/trunk/xdocs/changes.xml

Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java?rev=899929&r1=899928&r2=899929&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java (original)
+++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java Sat Jan 16 12:39:20 2010
@@ -3280,13 +3280,21 @@
      * @param element  the object to add, may be <code>null</code>
      * @return A new array containing the existing elements plus the new element
      * The returned array type will be that of the input array (unless null),
-     * in which case it will have the same type as the element (unless that is also null)
-     * in which case the returned type will be Object[].
+     * in which case it will have the same type as the element.
+     * If both are null, an IllegalArgumentException is thrown
      * @since 2.1
+     * @throws IllegalArgumentException if both arguments are null
      */
     public static <T> T[] add(T[] array, T element) {
-        Class<?> type = array != null ? array.getClass() : (element != null ? element.getClass() : Object.class);
-        // TODO - this is NOT safe to ignore - see LANG-571
+        Class<?> type;
+        if (array != null){
+            type = array.getClass();
+        } else if (element != null) {
+            type = element.getClass();
+        } else {
+            throw new IllegalArgumentException("Arguments cannot both be null");            
+        }
+        @SuppressWarnings("unchecked") // type must be T
         T[] newArray = (T[]) copyArrayGrow1(array, type);
         newArray[newArray.length - 1] = element;
         return newArray;
@@ -3554,6 +3562,7 @@
      * @return A new array containing the existing elements and the new element
      * @throws IndexOutOfBoundsException if the index is out of range
      * (index < 0 || index > array.length).
+     * @throws IllegalArgumentException if both array and element are null
      */
     public static <T> T[] add(T[] array, int index, T element) {
         Class<?> clss = null;
@@ -3562,9 +3571,7 @@
         } else if (element != null) {
             clss = element.getClass();
         } else {
-            // TODO this is not type-safe - see LANG-571
-            final T[] emptyArray = (T[]) new Object[] { null };
-            return emptyArray;
+            throw new IllegalArgumentException("Array and element cannot both be null");            
         }
         @SuppressWarnings("unchecked") // the add method creates an array of type clss, which is type T
         final T[] newArray = (T[]) add(array, index, element, clss);

Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java?rev=899929&r1=899928&r2=899929&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java (original)
+++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java Sat Jan 16 12:39:20 2010
@@ -221,6 +221,25 @@
         assertTrue(Arrays.equals((new Object[]{null}), newArray));
         assertEquals(Object.class, newArray.getClass().getComponentType());
     }
+    
+    public void testLANG571(){
+        String[] stringArray=null;
+        String aString=null;
+        try {
+            @SuppressWarnings("unused")
+            String[] sa = ArrayUtils.add(stringArray, aString);
+            fail("Should have caused IllegalArgumentException");
+        } catch (IllegalArgumentException iae){
+            //expected
+        }
+        try {
+            @SuppressWarnings("unused")
+            String[] sa = ArrayUtils.add(stringArray, 0, aString);
+            fail("Should have caused IllegalArgumentException");
+        } catch (IllegalArgumentException iae){
+            //expected
+        }
+    }
 
     public void testAddObjectArrayToObjectArray() {
         assertNull(ArrayUtils.addAll((Object[]) null, (Object[]) null));

Modified: commons/proper/lang/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/xdocs/changes.xml?rev=899929&r1=899928&r2=899929&view=diff
==============================================================================
--- commons/proper/lang/trunk/xdocs/changes.xml (original)
+++ commons/proper/lang/trunk/xdocs/changes.xml Sat Jan 16 12:39:20 2010
@@ -23,6 +23,7 @@
 
   <release version="3.0" date="Unreleased" description="Backwards incompatible update of Commons Lang to Java 5">
     <action type="update">Push down WordUtils to "text" sub-package.</action>
+    <action type="fix" issue="LANG-571">ArrayUtils.add(T[] array[, offset], T element) can create unexpected ClassCastException</action>
   </release>
 
   <release version="2.4" date="2008-03-18" description="">



Re: svn commit: r899929 - in /commons/proper/lang/trunk: src/main/java/org/apache/commons/lang3/ArrayUtils.java src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 16/01/2010, Henri Yandell <fl...@gmail.com> wrote:
> Alternatively... should add(null, null) simply return null?

IMO it's a program bug to call it with both arguments null.

The caller is going to have to check for null return in order to avoid
an NPE; if they don't do so immediately the NPE could occur a long way
from the original problem.

Best to report the problem as close to the source as possible - much
easier to debug.

>  Or throw NullPointerException?
>

It's not a null Pointer.

>  On Sat, Jan 16, 2010 at 4:39 AM,  <se...@apache.org> wrote:
>  > Author: sebb
>  > Date: Sat Jan 16 12:39:20 2010
>  > New Revision: 899929
>  >
>  > URL: http://svn.apache.org/viewvc?rev=899929&view=rev
>  > Log:
>  > LANG-571 ArrayUtils.add(T[] array[, offset], T element) can create unexpected ClassCastException
>  >
>  > Modified:
>  >    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
>  >    commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java
>  >    commons/proper/lang/trunk/xdocs/changes.xml
>  >
>  > Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
>  > URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java?rev=899929&r1=899928&r2=899929&view=diff
>  > ==============================================================================
>  > --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java (original)
>  > +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java Sat Jan 16 12:39:20 2010
>  > @@ -3280,13 +3280,21 @@
>  >      * @param element  the object to add, may be <code>null</code>
>  >      * @return A new array containing the existing elements plus the new element
>  >      * The returned array type will be that of the input array (unless null),
>  > -     * in which case it will have the same type as the element (unless that is also null)
>  > -     * in which case the returned type will be Object[].
>  > +     * in which case it will have the same type as the element.
>  > +     * If both are null, an IllegalArgumentException is thrown
>  >      * @since 2.1
>  > +     * @throws IllegalArgumentException if both arguments are null
>  >      */
>
>
> ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r899929 - in /commons/proper/lang/trunk: src/main/java/org/apache/commons/lang3/ArrayUtils.java src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java xdocs/changes.xml

Posted by Henri Yandell <fl...@gmail.com>.
Alternatively... should add(null, null) simply return null?

Or throw NullPointerException?

On Sat, Jan 16, 2010 at 4:39 AM,  <se...@apache.org> wrote:
> Author: sebb
> Date: Sat Jan 16 12:39:20 2010
> New Revision: 899929
>
> URL: http://svn.apache.org/viewvc?rev=899929&view=rev
> Log:
> LANG-571 ArrayUtils.add(T[] array[, offset], T element) can create unexpected ClassCastException
>
> Modified:
>    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
>    commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/ArrayUtilsAddTest.java
>    commons/proper/lang/trunk/xdocs/changes.xml
>
> Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java?rev=899929&r1=899928&r2=899929&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java (original)
> +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java Sat Jan 16 12:39:20 2010
> @@ -3280,13 +3280,21 @@
>      * @param element  the object to add, may be <code>null</code>
>      * @return A new array containing the existing elements plus the new element
>      * The returned array type will be that of the input array (unless null),
> -     * in which case it will have the same type as the element (unless that is also null)
> -     * in which case the returned type will be Object[].
> +     * in which case it will have the same type as the element.
> +     * If both are null, an IllegalArgumentException is thrown
>      * @since 2.1
> +     * @throws IllegalArgumentException if both arguments are null
>      */

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org