Annotation lookup matches unrelated overloads via assignable-parameter

resolution

MethodUtils.getAnnotation(Method, Class<A>, boolean, boolean)

Author: garydgregory

src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java

@@ -272,11 +272,56 @@ public static <A extends Annotation> A getAnnotation(final Method method, final
         A annotation = method.getAnnotation(annotationCls);
         if (annotation == null && searchSupers) {
             final Class<?> mcls = method.getDeclaringClass();
+            final String methodName = method.getName();
+            final Class<?>[] paramTypes = method.getParameterTypes();
             final List<Class<?>> classes = getAllSuperclassesAndInterfaces(mcls);
             for (final Class<?> acls : classes) {
-                final Method equivalentMethod = ignoreAccess ? getMatchingMethod(acls, method.getName(), method.getParameterTypes())
-                        : getMatchingAccessibleMethod(acls, method.getName(), method.getParameterTypes());
-                if (equivalentMethod != null) {
+                // First, attempt an exact parameter-type match (getDeclaredMethod) to
+                // find a true override. This avoids matching unrelated overloads that
+                // are merely assignable-compatible (e.g. process(Integer) vs
+                // process(Number)).
+                Method equivalentMethod = null;
+                try {
+                    equivalentMethod = acls.getDeclaredMethod(methodName, paramTypes);
+                } catch (final NoSuchMethodException ignored) {
+                    // No exact match; check for generic-bridge scenario: the declaring
+                    // class may use a type variable whose erased form is Object (or
+                    // another bound). In that case the parent method's erased
+                    // parameter types differ from the child's concrete types, so we
+                    // scan declared methods for a same-name method whose *erased*
+                    // parameter count matches and whose erased types are assignable
+                    // from our concrete types.
+                    for (final Method candidate : acls.getDeclaredMethods()) {
+                        if (!candidate.getName().equals(methodName)) {
+                            continue;
+                        }
+                        final Class<?>[] candidateParams = candidate.getParameterTypes();
+                        if (candidateParams.length != paramTypes.length) {
+                            continue;
+                        }
+                        // Require that every concrete param type is assignable to the
+                        // candidate's (erased) param type AND that the candidate is
+                        // generic (has at least one TypeVariable in its generic
+                        // parameter types). This prevents matching plain overloads.
+                        boolean genericMatch = false;
+                        boolean paramsMatch = true;
+                        final java.lang.reflect.Type[] genericParams = candidate.getGenericParameterTypes();
+                        for (int i = 0; i < candidateParams.length; i++) {
+                            if (genericParams[i] instanceof java.lang.reflect.TypeVariable) {
+                                genericMatch = true;
+                            }
+                            if (!ClassUtils.isAssignable(paramTypes[i], candidateParams[i], true)) {
+                                paramsMatch = false;
+                                break;
+                            }
+                        }
+                        if (paramsMatch && genericMatch) {
+                            equivalentMethod = candidate;
+                            break;
+                        }
+                    }
+                }
+                if (equivalentMethod != null && (ignoreAccess || MemberUtils.isAccessible(equivalentMethod))) {
                     annotation = equivalentMethod.getAnnotation(annotationCls);
                     if (annotation != null) {
                         break;

src/test/java/org/apache/commons/lang3/reflect/MethodUtilsAnnotationsTest.java

@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.lang3.reflect;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.lang.reflect.Method;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests {@link MethodUtils#getAnnotation(Method, Class, boolean, boolean)}.
+ * <p>
+ * getMatchingMethod allows assignable params, potentially finding annotations on unrelated overloads.
+ * </p>
+ */
+public class MethodUtilsAnnotationsTest {
+
+    /** Interface with a method taking Number, annotated @Deprecated */
+    public interface Processor {
+
+        @SuppressWarnings("javadoc")
+        @Deprecated
+        void process(Number n);
+    }
+
+    /** Implementation that does NOT annotate process(Integer) */
+    public static class ProcessorImpl implements Processor {
+
+        // Overload with Integer — NOT annotated
+        @SuppressWarnings("javadoc")
+        public void process(final Integer i) {
+            // intentionally no @Deprecated
+        }
+
+        @SuppressWarnings("deprecation")
+        @Override
+        public void process(final Number n) {
+            // inherited, annotated on interface
+        }
+    }
+
+    /**
+     * getAnnotation() for process(Integer) should return null because the Integer overload is NOT an override of process(Number).
+     * <ul>
+     * <li>Pre-patch: getMatchingMethod finds process(Number) (since Integer is assignable to Number) and returns the {@code @Deprecated} annotation
+     * incorrectly.</li>
+     * <li>Post-patch: uses getDeclaredMethod with exact types, finds nothing, returns null.</li>
+     * </ul>
+     */
+    @SuppressWarnings("javadoc")
+    @Test
+    public void testAnnotationLookupDoesNotMatchAssignableOverload() throws NoSuchMethodException {
+        final Method integerMethod = ProcessorImpl.class.getDeclaredMethod("process", Integer.class);
+        final Deprecated ann = MethodUtils.getAnnotation(integerMethod, Deprecated.class, true, true);
+        assertNull(ann, "process(Integer) is NOT an override of process(Number); its annotation lookup must return null");
+        final Method numberMethod = ProcessorImpl.class.getDeclaredMethod("process", Number.class);
+        assertNotNull(MethodUtils.getAnnotation(numberMethod, Deprecated.class, true, true));
+    }
+}