Skip to content

Commit

Permalink
Refine exception handling for type not present versus access exception
Browse files Browse the repository at this point in the history 
Includes TypeVariable bypass for reflection-free annotation retrieval.
Includes info log message for annotation attribute retrieval failure.

Closes gh-27182
  • Loading branch information
@jhoeller
jhoeller committed Jan 6, 2024
1 parent 87b35e7 commit 70247c4
Show file tree
Hide file tree
Showing 7 changed files with 368 additions and 418 deletions.
9 changes: 7 additions & 2 deletions spring-core/src/main/java/org/springframework/core/SerializableTypeWrapper.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -217,7 +217,12 @@ else if (Type[].class == method.getReturnType() && ObjectUtils.isEmpty(args)) {
return result;
}

return ReflectionUtils.invokeMethod(method, this.provider.getType(), args);
Type type = this.provider.getType();
if (type instanceof TypeVariable<?> tv && method.getName().equals("getName")) {
// Avoid reflection for common comparison of type variables
return tv.getName();
}
return ReflectionUtils.invokeMethod(method, type, args);
}
}

Expand Down
39 changes: 27 additions & 12 deletions spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -757,7 +757,7 @@ public static boolean isInJavaLangAnnotationPackage(@Nullable String annotationT
* Google App Engine's late arrival of {@code TypeNotPresentExceptionProxy} for
* {@code Class} values (instead of early {@code Class.getAnnotations() failure}).
* <p>This method not failing indicates that {@link #getAnnotationAttributes(Annotation)}
* won't failure either (when attempted later on).
* won't fail either (when attempted later on).
* @param annotation the annotation to validate
* @throws IllegalStateException if a declared {@code Class} attribute could not be read
* @since 4.3.15
Expand Down Expand Up @@ -1056,8 +1056,7 @@ public static Object getValue(@Nullable Annotation annotation, @Nullable String
return null;
}
catch (Throwable ex) {
rethrowAnnotationConfigurationException(ex);
handleIntrospectionFailure(annotation.getClass(), ex);
handleValueRetrievalFailure(annotation, ex);
return null;
}
}
Expand All @@ -1073,14 +1072,18 @@ public static Object getValue(@Nullable Annotation annotation, @Nullable String
* @return the value returned from the method invocation
* @since 5.3.24
*/
static Object invokeAnnotationMethod(Method method, Object annotation) {
@Nullable
static Object invokeAnnotationMethod(Method method, @Nullable Object annotation) {
if (annotation == null) {
return null;
}
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
// Ignore and fall back to reflection below
}
}
return ReflectionUtils.invokeMethod(method, annotation);
Expand Down Expand Up @@ -1114,20 +1117,32 @@ static void rethrowAnnotationConfigurationException(Throwable ex) {
* @see #rethrowAnnotationConfigurationException
* @see IntrospectionFailureLogger
*/
static void handleIntrospectionFailure(@Nullable AnnotatedElement element, Throwable ex) {
static void handleIntrospectionFailure(AnnotatedElement element, Throwable ex) {
rethrowAnnotationConfigurationException(ex);
IntrospectionFailureLogger logger = IntrospectionFailureLogger.INFO;
boolean meta = false;
if (element instanceof Class<?> clazz && Annotation.class.isAssignableFrom(clazz)) {
// Meta-annotation or (default) value lookup on an annotation type
// Meta-annotation introspection failure
logger = IntrospectionFailureLogger.DEBUG;
meta = true;
}
if (logger.isEnabled()) {
String message = meta ?
"Failed to meta-introspect annotation " :
"Failed to introspect annotations on ";
logger.log(message + element + ": " + ex);
logger.log("Failed to " + (meta ? "meta-introspect annotation " : "introspect annotations on ") +
element + ": " + ex);
}
}

/**
* Handle the supplied value retrieval exception.
* @param annotation the annotation instance from which to retrieve the value
* @param ex the exception that we encountered
* @see #handleIntrospectionFailure
*/
private static void handleValueRetrievalFailure(Annotation annotation, Throwable ex) {
rethrowAnnotationConfigurationException(ex);
IntrospectionFailureLogger logger = IntrospectionFailureLogger.INFO;
if (logger.isEnabled()) {
logger.log("Failed to retrieve value from " + annotation + ": " + ex);
}
}

Expand Down
4 changes: 2 additions & 2 deletions spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -451,7 +451,7 @@ static Annotation[] getDeclaredAnnotations(AnnotatedElement source, boolean defe
for (int i = 0; i < annotations.length; i++) {
Annotation annotation = annotations[i];
if (isIgnorable(annotation.annotationType()) ||
!AttributeMethods.forAnnotationType(annotation.annotationType()).isValid(annotation)) {
!AttributeMethods.forAnnotationType(annotation.annotationType()).canLoad(annotation)) {
annotations[i] = null;
}
else {
Expand Down
23 changes: 17 additions & 6 deletions spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -87,18 +87,26 @@ private AttributeMethods(@Nullable Class<? extends Annotation> annotationType, M
/**
* Determine if values from the given annotation can be safely accessed without
* causing any {@link TypeNotPresentException TypeNotPresentExceptions}.
* <p>This method is designed to cover Google App Engine's late arrival of such
* exceptions for {@code Class} values (instead of the more typical early
* {@code Class.getAnnotations() failure} on a regular JVM).
* @param annotation the annotation to check
* @return {@code true} if all values are present
* @see #validate(Annotation)
*/
boolean isValid(Annotation annotation) {
boolean canLoad(Annotation annotation) {
assertAnnotation(annotation);
for (int i = 0; i < size(); i++) {
if (canThrowTypeNotPresentException(i)) {
try {
AnnotationUtils.invokeAnnotationMethod(get(i), annotation);
}
catch (IllegalStateException ex) {
// Plain invocation failure to expose -> leave up to attribute retrieval
// (if any) where such invocation failure will be logged eventually.
}
catch (Throwable ex) {
// TypeNotPresentException etc. -> annotation type not actually loadable.
return false;
}
}
Expand All @@ -108,13 +116,13 @@ boolean isValid(Annotation annotation) {

/**
* Check if values from the given annotation can be safely accessed without causing
* any {@link TypeNotPresentException TypeNotPresentExceptions}. In particular,
* this method is designed to cover Google App Engine's late arrival of such
* any {@link TypeNotPresentException TypeNotPresentExceptions}.
* <p>This method is designed to cover Google App Engine's late arrival of such
* exceptions for {@code Class} values (instead of the more typical early
* {@code Class.getAnnotations() failure}).
* {@code Class.getAnnotations() failure} on a regular JVM).
* @param annotation the annotation to validate
* @throws IllegalStateException if a declared {@code Class} attribute could not be read
* @see #isValid(Annotation)
* @see #canLoad(Annotation)
*/
void validate(Annotation annotation) {
assertAnnotation(annotation);
Expand All @@ -123,6 +131,9 @@ void validate(Annotation annotation) {
try {
AnnotationUtils.invokeAnnotationMethod(get(i), annotation);
}
catch (IllegalStateException ex) {
throw ex;
}
catch (Throwable ex) {
throw new IllegalStateException("Could not obtain annotation attribute value for " +
get(i).getName() + " declared on " + annotation.annotationType(), ex);
Expand Down
77 changes: 33 additions & 44 deletions ...rc/test/java/org/springframework/core/annotation/AnnotationIntrospectionFailureTests.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,55 +43,45 @@ class AnnotationIntrospectionFailureTests {

@Test
void filteredTypeThrowsTypeNotPresentException() throws Exception {
FilteringClassLoader classLoader = new FilteringClassLoader(
getClass().getClassLoader());
Class<?> withExampleAnnotation = ClassUtils.forName(
WithExampleAnnotation.class.getName(), classLoader);
Annotation annotation = withExampleAnnotation.getAnnotations()[0];
FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader());
Class<?> withAnnotation = ClassUtils.forName(WithExampleAnnotation.class.getName(), classLoader);
Annotation annotation = withAnnotation.getAnnotations()[0];
Method method = annotation.annotationType().getMethod("value");
method.setAccessible(true);
assertThatExceptionOfType(TypeNotPresentException.class).isThrownBy(() ->
ReflectionUtils.invokeMethod(method, annotation))
.withCauseInstanceOf(ClassNotFoundException.class);
assertThatExceptionOfType(TypeNotPresentException.class)
.isThrownBy(() -> ReflectionUtils.invokeMethod(method, annotation))
.withCauseInstanceOf(ClassNotFoundException.class);
}

@Test
@SuppressWarnings("unchecked")
void filteredTypeInMetaAnnotationWhenUsingAnnotatedElementUtilsHandlesException() throws Exception {
FilteringClassLoader classLoader = new FilteringClassLoader(
getClass().getClassLoader());
Class<?> withExampleMetaAnnotation = ClassUtils.forName(
WithExampleMetaAnnotation.class.getName(), classLoader);
Class<Annotation> exampleAnnotationClass = (Class<Annotation>) ClassUtils.forName(
ExampleAnnotation.class.getName(), classLoader);
Class<Annotation> exampleMetaAnnotationClass = (Class<Annotation>) ClassUtils.forName(
ExampleMetaAnnotation.class.getName(), classLoader);
assertThat(AnnotatedElementUtils.getMergedAnnotationAttributes(
withExampleMetaAnnotation, exampleAnnotationClass)).isNull();
assertThat(AnnotatedElementUtils.getMergedAnnotationAttributes(
withExampleMetaAnnotation, exampleMetaAnnotationClass)).isNull();
assertThat(AnnotatedElementUtils.hasAnnotation(withExampleMetaAnnotation,
exampleAnnotationClass)).isFalse();
assertThat(AnnotatedElementUtils.hasAnnotation(withExampleMetaAnnotation,
exampleMetaAnnotationClass)).isFalse();
FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader());
Class<?> withAnnotation = ClassUtils.forName(WithExampleMetaAnnotation.class.getName(), classLoader);
Class<Annotation> annotationClass = (Class<Annotation>)
ClassUtils.forName(ExampleAnnotation.class.getName(), classLoader);
Class<Annotation> metaAnnotationClass = (Class<Annotation>)
ClassUtils.forName(ExampleMetaAnnotation.class.getName(), classLoader);
assertThat(AnnotatedElementUtils.getMergedAnnotationAttributes(withAnnotation, annotationClass)).isNull();
assertThat(AnnotatedElementUtils.getMergedAnnotationAttributes(withAnnotation, metaAnnotationClass)).isNull();
assertThat(AnnotatedElementUtils.hasAnnotation(withAnnotation, annotationClass)).isFalse();
assertThat(AnnotatedElementUtils.hasAnnotation(withAnnotation, metaAnnotationClass)).isFalse();
}

@Test
@SuppressWarnings("unchecked")
void filteredTypeInMetaAnnotationWhenUsingMergedAnnotationsHandlesException() throws Exception {
FilteringClassLoader classLoader = new FilteringClassLoader(
getClass().getClassLoader());
Class<?> withExampleMetaAnnotation = ClassUtils.forName(
WithExampleMetaAnnotation.class.getName(), classLoader);
Class<Annotation> exampleAnnotationClass = (Class<Annotation>) ClassUtils.forName(
ExampleAnnotation.class.getName(), classLoader);
Class<Annotation> exampleMetaAnnotationClass = (Class<Annotation>) ClassUtils.forName(
ExampleMetaAnnotation.class.getName(), classLoader);
MergedAnnotations annotations = MergedAnnotations.from(withExampleMetaAnnotation);
assertThat(annotations.get(exampleAnnotationClass).isPresent()).isFalse();
assertThat(annotations.get(exampleMetaAnnotationClass).isPresent()).isFalse();
assertThat(annotations.isPresent(exampleMetaAnnotationClass)).isFalse();
assertThat(annotations.isPresent(exampleAnnotationClass)).isFalse();
FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader());
Class<?> withAnnotation = ClassUtils.forName(WithExampleMetaAnnotation.class.getName(), classLoader);
Class<Annotation> annotationClass = (Class<Annotation>)
ClassUtils.forName(ExampleAnnotation.class.getName(), classLoader);
Class<Annotation> metaAnnotationClass = (Class<Annotation>)
ClassUtils.forName(ExampleMetaAnnotation.class.getName(), classLoader);
MergedAnnotations annotations = MergedAnnotations.from(withAnnotation);
assertThat(annotations.get(annotationClass).isPresent()).isFalse();
assertThat(annotations.get(metaAnnotationClass).isPresent()).isFalse();
assertThat(annotations.isPresent(metaAnnotationClass)).isFalse();
assertThat(annotations.isPresent(annotationClass)).isFalse();
}


Expand All @@ -103,17 +93,16 @@ static class FilteringClassLoader extends OverridingClassLoader {

@Override
protected boolean isEligibleForOverriding(String className) {
return className.startsWith(
AnnotationIntrospectionFailureTests.class.getName());
return className.startsWith(AnnotationIntrospectionFailureTests.class.getName()) ||
className.startsWith("jdk.internal");
}

@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
if (name.startsWith(AnnotationIntrospectionFailureTests.class.getName()) &&
name.contains("Filtered")) {
protected Class<?> loadClassForOverriding(String name) throws ClassNotFoundException {
if (name.contains("Filtered") || name.startsWith("jdk.internal")) {
throw new ClassNotFoundException(name);
}
return super.loadClass(name, resolve);
return super.loadClassForOverriding(name);
}
}

Expand Down
6 changes: 3 additions & 3 deletions spring-core/src/test/java/org/springframework/core/annotation/AttributeMethodsTests.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -112,7 +112,7 @@ void isValidWhenHasTypeNotPresentExceptionReturnsFalse() {
ClassValue annotation = mockAnnotation(ClassValue.class);
given(annotation.value()).willThrow(TypeNotPresentException.class);
AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType());
assertThat(attributes.isValid(annotation)).isFalse();
assertThat(attributes.canLoad(annotation)).isFalse();
}

@Test
Expand All @@ -121,7 +121,7 @@ void isValidWhenDoesNotHaveTypeNotPresentExceptionReturnsTrue() {
ClassValue annotation = mock();
given(annotation.value()).willReturn((Class) InputStream.class);
AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType());
assertThat(attributes.isValid(annotation)).isTrue();
assertThat(attributes.canLoad(annotation)).isTrue();
}

@Test
Expand Down

0 comments on commit 70247c4

Please sign in to comment.