From f4b3333fa88e75c465786bb718af99a2e461f6e0 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 8 Nov 2022 16:23:33 +0100 Subject: [PATCH] Avoid reflection for annotation attribute method invocations As a follow up to 332b25b680, this commit consistently avoids the use of reflection for annotation attribute method invocations. See gh-29301 Closes gh-29448 --- .../annotation/AnnotationTypeMapping.java | 7 ++-- .../core/annotation/AnnotationUtils.java | 36 ++++++++++++++----- .../core/annotation/AttributeMethods.java | 4 +-- .../core/annotation/RepeatableContainers.java | 20 ++--------- ...izedMergedAnnotationInvocationHandler.java | 2 +- .../core/annotation/TypeMappedAnnotation.java | 16 ++++----- 6 files changed, 44 insertions(+), 41 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java index 4e072238765d..92bc1894ccf2 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java @@ -32,7 +32,6 @@ import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.MirrorSet; import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -241,7 +240,7 @@ private void processAliases(int attributeIndex, List aliases) { mapping.claimedAliases.addAll(aliases); if (mapping.annotation != null) { int[] resolvedMirrors = mapping.mirrorSets.resolve(null, - mapping.annotation, ReflectionUtils::invokeMethod); + mapping.annotation, AnnotationUtils::invokeAnnotationMethod); for (int i = 0; i < mapping.attributes.size(); i++) { if (aliases.contains(mapping.attributes.get(i))) { this.annotationValueMappings[attributeIndex] = resolvedMirrors[i]; @@ -505,7 +504,7 @@ Object getMappedAnnotationValue(int attributeIndex, boolean metaAnnotationsOnly) if (source == this && metaAnnotationsOnly) { return null; } - return ReflectionUtils.invokeMethod(source.attributes.get(mappedIndex), source.annotation); + return AnnotationUtils.invokeAnnotationMethod(source.attributes.get(mappedIndex), source.annotation); } /** @@ -594,7 +593,7 @@ private static boolean areEquivalent(Annotation annotation, @Nullable Object ext AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType()); for (int i = 0; i < attributes.size(); i++) { Method attribute = attributes.get(i); - Object value1 = ReflectionUtils.invokeMethod(attribute, annotation); + Object value1 = AnnotationUtils.invokeAnnotationMethod(attribute, annotation); Object value2; if (extractedValue instanceof TypeMappedAnnotation) { value2 = ((TypeMappedAnnotation) extractedValue).getValue(attribute.getName()).orElse(null); diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index 11eb589aa583..20868db2223d 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -19,9 +19,10 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Array; -import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Proxy; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -1052,23 +1053,42 @@ public static Object getValue(@Nullable Annotation annotation, @Nullable String } try { Method method = annotation.annotationType().getDeclaredMethod(attributeName); - ReflectionUtils.makeAccessible(method); - return method.invoke(annotation); + return invokeAnnotationMethod(method, annotation); } catch (NoSuchMethodException ex) { return null; } - catch (InvocationTargetException ex) { - rethrowAnnotationConfigurationException(ex.getTargetException()); - throw new IllegalStateException("Could not obtain value for annotation attribute '" + - attributeName + "' in " + annotation, ex); - } catch (Throwable ex) { + rethrowAnnotationConfigurationException(ex); handleIntrospectionFailure(annotation.getClass(), ex); return null; } } + /** + * Invoke the supplied annotation attribute {@link Method} on the supplied + * {@link Annotation}. + *

An attempt will first be made to invoke the method via the annotation's + * {@link InvocationHandler} (if the annotation instance is a JDK dynamic proxy). + * If that fails, an attempt will be made to invoke the method via reflection. + * @param method the method to invoke + * @param annotation the annotation on which to invoke the method + * @return the value returned from the method invocation + * @since 5.3.24 + */ + static Object invokeAnnotationMethod(Method method, Object annotation) { + if (Proxy.isProxyClass(annotation.getClass())) { + try { + InvocationHandler handler = Proxy.getInvocationHandler(annotation); + return handler.invoke(annotation, method, null); + } + catch (Throwable ex) { + // ignore and fall back to reflection below + } + } + return ReflectionUtils.invokeMethod(method, annotation); + } + /** * If the supplied throwable is an {@link AnnotationConfigurationException}, * it will be cast to an {@code AnnotationConfigurationException} and thrown, diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java b/spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java index d66fb66096b8..ea9b6b715d9f 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java @@ -109,7 +109,7 @@ boolean isValid(Annotation annotation) { for (int i = 0; i < size(); i++) { if (canThrowTypeNotPresentException(i)) { try { - get(i).invoke(annotation); + AnnotationUtils.invokeAnnotationMethod(get(i), annotation); } catch (Throwable ex) { return false; @@ -134,7 +134,7 @@ void validate(Annotation annotation) { for (int i = 0; i < size(); i++) { if (canThrowTypeNotPresentException(i)) { try { - get(i).invoke(annotation); + AnnotationUtils.invokeAnnotationMethod(get(i), annotation); } catch (Throwable ex) { throw new IllegalStateException("Could not obtain annotation attribute value for " + diff --git a/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java b/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java index e1f7f9f72297..52adbc034bb0 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java @@ -18,16 +18,13 @@ import java.lang.annotation.Annotation; import java.lang.annotation.Repeatable; -import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; -import java.lang.reflect.Proxy; import java.util.Map; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; /** * Strategy used to determine annotations that act as containers for other @@ -133,19 +130,6 @@ public static RepeatableContainers none() { return NoRepeatableContainers.INSTANCE; } - private static Object invokeAnnotationMethod(Annotation annotation, Method method) { - if (Proxy.isProxyClass(annotation.getClass())) { - try { - InvocationHandler handler = Proxy.getInvocationHandler(annotation); - return handler.invoke(annotation, method, null); - } - catch (Throwable ex) { - // ignore and fall back to reflection below - } - } - return ReflectionUtils.invokeMethod(method, annotation); - } - /** * Standard {@link RepeatableContainers} implementation that searches using @@ -168,7 +152,7 @@ private static class StandardRepeatableContainers extends RepeatableContainers { Annotation[] findRepeatedAnnotations(Annotation annotation) { Method method = getRepeatedAnnotationsMethod(annotation.annotationType()); if (method != null) { - return (Annotation[]) invokeAnnotationMethod(annotation, method); + return (Annotation[]) AnnotationUtils.invokeAnnotationMethod(method, annotation); } return super.findRepeatedAnnotations(annotation); } @@ -255,7 +239,7 @@ private Class deduceContainer(Class @Nullable Annotation[] findRepeatedAnnotations(Annotation annotation) { if (this.container.isAssignableFrom(annotation.annotationType())) { - return (Annotation[]) invokeAnnotationMethod(annotation, this.valueMethod); + return (Annotation[]) AnnotationUtils.invokeAnnotationMethod(this.valueMethod, annotation); } return super.findRepeatedAnnotations(annotation); } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java index 5d3f9a40949f..6d7591b9171d 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java @@ -111,7 +111,7 @@ private boolean annotationEquals(Object other) { for (int i = 0; i < this.attributes.size(); i++) { Method attribute = this.attributes.get(i); Object thisValue = getAttributeValue(attribute); - Object otherValue = ReflectionUtils.invokeMethod(attribute, other); + Object otherValue = AnnotationUtils.invokeAnnotationMethod(attribute, other); if (!ObjectUtils.nullSafeEquals(thisValue, otherValue)) { return false; } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java index 0b18e30bf07a..1ae30b9e4a81 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java @@ -33,7 +33,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; -import org.springframework.util.ReflectionUtils; /** * {@link MergedAnnotation} that adapts attributes from a root annotation by @@ -43,9 +42,9 @@ * {@code BiFunction}. This allows various different annotation models to be * supported by the same class. For example, the attributes source might be an * actual {@link Annotation} instance where methods on the annotation instance - * are {@linkplain ReflectionUtils#invokeMethod(Method, Object) invoked} to extract - * values. Equally, the source could be a simple {@link Map} with values - * extracted using {@link Map#get(Object)}. + * are {@linkplain AnnotationUtils#invokeAnnotationMethod(Method, Object) invoked} + * to extract values. Similarly, the source could be a simple {@link Map} with + * values extracted using {@link Map#get(Object)}. * *

Extracted root attribute values must be compatible with the attribute * return type, namely: @@ -434,7 +433,7 @@ private Object getValueFromMetaAnnotation(int attributeIndex, boolean forMirrorR } if (value == null) { Method attribute = this.mapping.getAttributes().get(attributeIndex); - value = ReflectionUtils.invokeMethod(attribute, this.mapping.getAnnotation()); + value = AnnotationUtils.invokeAnnotationMethod(attribute, this.mapping.getAnnotation()); } return value; } @@ -555,7 +554,7 @@ private MergedAnnotation adaptToMergedAnnotation(Object value, Class MergedAnnotation from(@Nullable Object source, A annotation) { Assert.notNull(annotation, "Annotation must not be null"); AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(annotation.annotationType()); - return new TypeMappedAnnotation<>(mappings.get(0), null, source, annotation, ReflectionUtils::invokeMethod, 0); + return new TypeMappedAnnotation<>( + mappings.get(0), null, source, annotation, AnnotationUtils::invokeAnnotationMethod, 0); } static MergedAnnotation of( @@ -649,7 +649,7 @@ static TypeMappedAnnotation createIfPossible( int aggregateIndex, IntrospectionFailureLogger logger) { return createIfPossible(mapping, source, annotation, - ReflectionUtils::invokeMethod, aggregateIndex, logger); + AnnotationUtils::invokeAnnotationMethod, aggregateIndex, logger); } @Nullable