You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2020/02/17 15:29:43 UTC

[commons-lang] branch master updated: LANG-1433: MethodUtils will throw a NPE if invokeMethod() is called for a var-args method (#407)

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git


The following commit(s) were added to refs/heads/master by this push:
     new 553a047  LANG-1433: MethodUtils will throw a NPE if invokeMethod() is called for a var-args method (#407)
553a047 is described below

commit 553a0474ea11a9b61a8de618a3512f29f5a17368
Author: Christian Franzen <ch...@inform-software.com>
AuthorDate: Mon Feb 17 16:29:34 2020 +0100

    LANG-1433: MethodUtils will throw a NPE if invokeMethod() is called for a var-args method (#407)
    
    * LANG-1433: MethodUtils will throw a NPE if invokeMethod() is called for a var-args method with null parameter value
    
    * LANG-1433: Result of invokeMethod() is not deterministic for overloaded methods that can not be uniquly resolved from parameter types
    
    * LANG-1433: Fixed checkstyle errors
---
 .../apache/commons/lang3/reflect/MemberUtils.java  |  5 ++-
 .../apache/commons/lang3/reflect/MethodUtils.java  | 47 +++++++++++++++-------
 .../commons/lang3/reflect/MethodUtilsTest.java     | 16 ++++++++
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/src/main/java/org/apache/commons/lang3/reflect/MemberUtils.java b/src/main/java/org/apache/commons/lang3/reflect/MemberUtils.java
index d7e2bd2..73fa2f1 100644
--- a/src/main/java/org/apache/commons/lang3/reflect/MemberUtils.java
+++ b/src/main/java/org/apache/commons/lang3/reflect/MemberUtils.java
@@ -162,7 +162,7 @@ abstract class MemberUtils {
             // When isVarArgs is true, srcArgs and dstArgs may differ in length.
             // There are two special cases to consider:
             final boolean noVarArgsPassed = srcArgs.length < destArgs.length;
-            final boolean explicitArrayForVarags = srcArgs.length == destArgs.length && srcArgs[srcArgs.length-1].isArray();
+            final boolean explicitArrayForVarags = srcArgs.length == destArgs.length && srcArgs[srcArgs.length-1] != null && srcArgs[srcArgs.length-1].isArray();
 
             final float varArgsCost = 0.001f;
             final Class<?> destClass = destArgs[destArgs.length-1].getComponentType();
@@ -227,6 +227,9 @@ abstract class MemberUtils {
      * @return The cost of promoting the primitive
      */
     private static float getPrimitivePromotionCost(final Class<?> srcClass, final Class<?> destClass) {
+        if (srcClass == null) {
+            return 1.5f;
+        }
         float cost = 0.0f;
         Class<?> cls = srcClass;
         if (!cls.isPrimitive()) {
diff --git a/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java b/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
index 8ebc790..c014450 100644
--- a/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
+++ b/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
@@ -25,6 +25,8 @@ import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -59,6 +61,13 @@ import org.apache.commons.lang3.Validate;
  */
 public class MethodUtils {
 
+  private static final Comparator<Method> METHOD_BY_SIGNATURE = new Comparator<Method>() {
+    @Override
+    public int compare(final Method m1, final Method m2) {
+        return m1.toString ().compareTo (m2.toString ());
+    }
+  };
+
     /**
      * <p>{@link MethodUtils} instances should NOT be constructed in standard programming.
      * Instead, the class should be used as
@@ -462,8 +471,8 @@ public class MethodUtils {
      * @since 3.5
      */
     static Object[] getVarArgs(final Object[] args, final Class<?>[] methodParameterTypes) {
-        if (args.length == methodParameterTypes.length
-                && args[args.length - 1].getClass().equals(methodParameterTypes[methodParameterTypes.length - 1])) {
+        if (args.length == methodParameterTypes.length && (args[args.length - 1] == null ||
+                args[args.length - 1].getClass().equals(methodParameterTypes[methodParameterTypes.length - 1]))) {
             // The args array is already in the canonical form for the method.
             return args;
         }
@@ -679,20 +688,28 @@ public class MethodUtils {
         } catch (final NoSuchMethodException e) { // NOPMD - Swallow the exception
         }
         // search through all methods
-        Method bestMatch = null;
         final Method[] methods = cls.getMethods();
+        final List<Method> matchingMethods = new ArrayList<>();
         for (final Method method : methods) {
             // compare name and parameters
             if (method.getName().equals(methodName) &&
                     MemberUtils.isMatchingMethod(method, parameterTypes)) {
-                // get accessible version of method
-                final Method accessibleMethod = getAccessibleMethod(method);
-                if (accessibleMethod != null && (bestMatch == null || MemberUtils.compareMethodFit(
-                            accessibleMethod,
-                            bestMatch,
-                            parameterTypes) < 0)) {
-                    bestMatch = accessibleMethod;
-                }
+                matchingMethods.add (method);
+            }
+        }
+
+        // Sort methods by signature to force deterministic result
+        Collections.sort (matchingMethods, METHOD_BY_SIGNATURE);
+
+        Method bestMatch = null;
+        for (final Method method : matchingMethods) {
+            // get accessible version of method
+            final Method accessibleMethod = getAccessibleMethod(method);
+            if (accessibleMethod != null && (bestMatch == null || MemberUtils.compareMethodFit(
+                        accessibleMethod,
+                        bestMatch,
+                        parameterTypes) < 0)) {
+                bestMatch = accessibleMethod;
             }
         }
         if (bestMatch != null) {
@@ -703,10 +720,12 @@ public class MethodUtils {
             final Class<?>[] methodParameterTypes = bestMatch.getParameterTypes();
             final Class<?> methodParameterComponentType = methodParameterTypes[methodParameterTypes.length - 1].getComponentType();
             final String methodParameterComponentTypeName = ClassUtils.primitiveToWrapper(methodParameterComponentType).getName();
-            final String parameterTypeName = parameterTypes[parameterTypes.length - 1].getName();
-            final String parameterTypeSuperClassName = parameterTypes[parameterTypes.length - 1].getSuperclass().getName();
 
-            if (!methodParameterComponentTypeName.equals(parameterTypeName)
+            final Class<?> lastParameterType = parameterTypes[parameterTypes.length - 1];
+            final String parameterTypeName = (lastParameterType==null) ? null : lastParameterType.getName();
+            final String parameterTypeSuperClassName = (lastParameterType==null) ? null : lastParameterType.getSuperclass().getName();
+
+            if (parameterTypeName!= null && parameterTypeSuperClassName != null && !methodParameterComponentTypeName.equals(parameterTypeName)
                     && !methodParameterComponentTypeName.equals(parameterTypeSuperClassName)) {
                 return null;
             }
diff --git a/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
index 9df5a14..168bc2a 100644
--- a/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
@@ -447,6 +447,22 @@ public class MethodUtilsTest {
     }
 
     @Test
+    public void testInvokeMethod_VarArgsWithNullValues() throws Exception {
+        assertEquals("String...", MethodUtils.invokeMethod(testBean, "varOverload",
+                "a", null, "c"));
+        assertEquals("String...", MethodUtils.invokeMethod(testBean, "varOverload",
+                                                                "a", "b", null));
+    }
+
+    @Test
+    public void testInvokeMethod_VarArgsNotUniqueResolvable() throws Exception {
+      assertEquals("Boolean...", MethodUtils.invokeMethod(testBean, "varOverload",
+                                                         new Object[] {null}));
+      assertEquals("Object...", MethodUtils.invokeMethod(testBean, "varOverload",
+                                                         (Object[]) null));
+    }
+
+    @Test
     public void testInvokeExactMethod() throws Exception {
         assertEquals("foo()", MethodUtils.invokeExactMethod(testBean, "foo",
                 (Object[]) ArrayUtils.EMPTY_CLASS_ARRAY));