From 36348e4501cb3b2b4e6e0ab780f5b4cb25cdd58e Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Wed, 6 May 2026 19:00:05 -0400 Subject: [PATCH] Annotation lookup matches unrelated overloads via assignable-parameter resolution MethodUtils.getAnnotation(Method, Class, boolean, boolean) --- .../commons/lang3/reflect/MethodUtils.java | 51 ++++++++++++- .../reflect/MethodUtilsAnnotationsTest.java | 76 +++++++++++++++++++ 2 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/apache/commons/lang3/reflect/MethodUtilsAnnotationsTest.java 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 df6899f5a3a..a3b73438086 100644 --- a/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java +++ b/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java @@ -272,11 +272,56 @@ public static 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> 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; diff --git a/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsAnnotationsTest.java b/src/test/java/org/apache/commons/lang3/reflect/MethodUtilsAnnotationsTest.java new file mode 100644 index 00000000000..caaa1cbf9d9 --- /dev/null +++ b/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)}. + *

+ * getMatchingMethod allows assignable params, potentially finding annotations on unrelated overloads. + *

+ */ +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). + *
    + *
  • Pre-patch: getMatchingMethod finds process(Number) (since Integer is assignable to Number) and returns the {@code @Deprecated} annotation + * incorrectly.
    • + *
  • Post-patch: uses getDeclaredMethod with exact types, finds nothing, returns null.
    • + *
+ */ + @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)); + } +}