Skip to content

Commit

Permalink
Defensively copy array returned from forAnnotations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
philwebb committed Mar 25, 2019
1 parent 78fd882 commit cb7f997
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -864,12 +864,12 @@ public <T extends Annotation> T getAnnotation(Class<T> annotationClass) {

@Override
public Annotation[] getAnnotations() {
return this.annotations;
return this.annotations.clone();
}

@Override
public Annotation[] getDeclaredAnnotations() {
return this.annotations;
return this.annotations.clone();
}

};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
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;
Expand All @@ -43,6 +41,7 @@

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.*;
Expand Down Expand Up @@ -736,6 +735,15 @@ public void findMethodAnnotationFromGenericSuperclass() throws Exception {
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());
}

// -------------------------------------------------------------------------

Expand Down Expand Up @@ -1301,4 +1309,10 @@ public void doIt() {
}
}

@Deprecated
@ComponentScan
class ForAnnotationsClass {

}

}

0 comments on commit cb7f997

Please sign in to comment.