From cb7f997962661a9a1b502123f778b67ad8cf4853 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 25 Mar 2019 07:51:01 -0700 Subject: [PATCH] Defensively copy array returned from forAnnotations Update the proxy used in `AnnotatedElementUtils.forAnnotations` so that it returns a cloned array for calls to `getDeclaredAnnotations` or `getAnnotations`. This matches the behavior of standard JDK `AnnotatedElement` implementations. Closes gh-22655 --- .../core/annotation/AnnotatedElementUtils.java | 4 ++-- .../annotation/AnnotatedElementUtilsTests.java | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index 216cf1630b..4d5312a638 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -864,12 +864,12 @@ public abstract class AnnotatedElementUtils { @Override public Annotation[] getAnnotations() { - return this.annotations; + return this.annotations.clone(); } @Override public Annotation[] getDeclaredAnnotations() { - return this.annotations; + return this.annotations.clone(); } }; diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java index 8774e6ae2d..4aaf682b36 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java @@ -23,9 +23,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; -import java.util.Date; import java.util.List; import java.util.Set; import javax.annotation.Resource; @@ -43,6 +41,7 @@ import org.springframework.util.MultiValueMap; import static java.util.Arrays.*; import static java.util.stream.Collectors.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.springframework.core.annotation.AnnotatedElementUtils.*; @@ -736,6 +735,15 @@ public class AnnotatedElementUtilsTests { assertNotNull(order); } + @Test // gh-22655 + public void forAnnotationsCreatesCopyOfArrayOnEachCall() { + AnnotatedElement element = AnnotatedElementUtils.forAnnotations(ForAnnotationsClass.class.getDeclaredAnnotations()); + // Trigger the NPE as originally reported in the bug + AnnotationsScanner.getDeclaredAnnotations(element, false); + AnnotationsScanner.getDeclaredAnnotations(element, false); + // Also specifically test we get different instances + assertThat(element.getDeclaredAnnotations()).isNotSameAs(element.getDeclaredAnnotations()); + } // ------------------------------------------------------------------------- @@ -1301,4 +1309,10 @@ public class AnnotatedElementUtilsTests { } } + @Deprecated + @ComponentScan + class ForAnnotationsClass { + + } + }