Protect against stack overflow when searching meta annotations

It is legal for an annotation to be annotated with itself. Previously,
when searching for meta annotations this could lead to a stack overflow.
This was likely to occur when using Kotlin as, like Java, its Target
annotation is annotated with itself. A stack overflow doesn’t occur
with Java’s Target annotation due to some short-circuiting logic for
annotations in java.lang.

This commit updates the logic for finding meta-annotations to
short-circuit when an annotation that has already been seen is
encountered.

Closes gh-5902

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelector.java

@@ -20,6 +20,7 @@ import java.lang.annotation.Annotation;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -34,6 +35,7 @@ import org.springframework.util.ClassUtils;
  * {@link ImportAutoConfiguration}.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  */
 class ImportAutoConfigurationImportSelector
 		extends EnableAutoConfigurationImportSelector {
@@ -66,29 +68,30 @@ class ImportAutoConfigurationImportSelector
 
 	private List<String> getCandidateConfigurations(Class<?> source) {
 		Set<String> candidates = new LinkedHashSet<String>();
-		collectCandidateConfigurations(source, candidates);
+		collectCandidateConfigurations(source, candidates, new HashSet<Class<?>>());
 		return new ArrayList<String>(candidates);
 	}
 
-	private void collectCandidateConfigurations(Class<?> source, Set<String> candidates) {
-		if (source != null) {
+	private void collectCandidateConfigurations(Class<?> source, Set<String> candidates,
+			Set<Class<?>> seen) {
+		if (source != null && seen.add(source)) {
 			for (Annotation annotation : source.getDeclaredAnnotations()) {
 				if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) {
-					collectCandidateConfigurations(annotation, candidates);
+					collectCandidateConfigurations(annotation, candidates, seen);
 				}
 			}
-			collectCandidateConfigurations(source.getSuperclass(), candidates);
+			collectCandidateConfigurations(source.getSuperclass(), candidates, seen);
 		}
 	}
 
 	private void collectCandidateConfigurations(Annotation annotation,
-			Set<String> candidates) {
+			Set<String> candidates, Set<Class<?>> seen) {
 		if (ANNOTATION_NAMES.contains(annotation.annotationType().getName())) {
 			String[] value = (String[]) AnnotationUtils
 					.getAnnotationAttributes(annotation, true).get("value");
 			candidates.addAll(Arrays.asList(value));
 		}
-		collectCandidateConfigurations(annotation.annotationType(), candidates);
+		collectCandidateConfigurations(annotation.annotationType(), candidates, seen);
 	}
 
 	@Override

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelectorTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.autoconfigure;
 
+import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
@@ -41,6 +42,7 @@ import static org.mockito.Mockito.verifyZeroInteractions;
  * Tests for {@link ImportAutoConfigurationImportSelector}.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  */
 @RunWith(MockitoJUnitRunner.class)
 public class ImportAutoConfigurationImportSelectorTests {
@@ -87,6 +89,15 @@ public class ImportAutoConfigurationImportSelectorTests {
 				ThymeleafAutoConfiguration.class.getName());
 	}
 
+	@Test
+	public void selfAnnotatingAnnotationDoesNotCauseStackOverflow() throws IOException {
+		AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory()
+				.getMetadataReader(ImportWithSelfAnnotatingAnnotation.class.getName())
+				.getAnnotationMetadata();
+		String[] imports = this.importSelector.selectImports(annotationMetadata);
+		assertThat(imports).containsOnly(ThymeleafAutoConfiguration.class.getName());
+	}
+
 	@ImportAutoConfiguration(FreeMarkerAutoConfiguration.class)
 	static class ImportFreemarker {
 
@@ -98,6 +109,11 @@ public class ImportAutoConfigurationImportSelectorTests {
 
 	}
 
+	@SelfAnnotating
+	static class ImportWithSelfAnnotatingAnnotation {
+
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@ImportAutoConfiguration(FreeMarkerAutoConfiguration.class)
 	static @interface ImportOne {
@@ -110,4 +126,11 @@ public class ImportAutoConfigurationImportSelectorTests {
 
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@ImportAutoConfiguration(ThymeleafAutoConfiguration.class)
+	@SelfAnnotating
+	static @interface SelfAnnotating {
+
+	}
+
 }

spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/properties/AnnotationsPropertySource.java

@@ -20,9 +20,11 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -58,13 +60,13 @@ public class AnnotationsPropertySource extends EnumerablePropertySource<Class<?>
 
 	private Map<String, Object> getProperties(Class<?> source) {
 		Map<String, Object> properties = new LinkedHashMap<String, Object>();
-		collectProperties(source, source, properties);
+		collectProperties(source, source, properties, new HashSet<Class<?>>());
 		return Collections.unmodifiableMap(properties);
 	}
 
 	private void collectProperties(Class<?> root, Class<?> source,
-			Map<String, Object> properties) {
-		if (source != null) {
+			Map<String, Object> properties, Set<Class<?>> seen) {
+		if (source != null && seen.add(source)) {
 			for (Annotation annotation : getMergedAnnotations(root, source)) {
 				if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) {
 					PropertyMapping typeMapping = annotation.annotationType()
@@ -73,10 +75,11 @@ public class AnnotationsPropertySource extends EnumerablePropertySource<Class<?>
 							.getDeclaredMethods()) {
 						collectProperties(annotation, attribute, typeMapping, properties);
 					}
-					collectProperties(root, annotation.annotationType(), properties);
+					collectProperties(root, annotation.annotationType(), properties,
+							seen);
 				}
 			}
-			collectProperties(root, source.getSuperclass(), properties);
+			collectProperties(root, source.getSuperclass(), properties, seen);
 		}
 	}
 

spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/properties/AnnotationsPropertySourceTests.java

@@ -164,6 +164,11 @@ public class AnnotationsPropertySourceTests {
 		assertThat(source.getProperty("aliasing.value")).isEqualTo("baz");
 	}
 
+	@Test
+	public void selfAnnotatingAnnotationDoesNotCauseStackOverflow() {
+		new AnnotationsPropertySource(PropertyMappedWithSelfAnnotatingAnnotation.class);
+	}
+
 	static class NoAnnotation {
 
 	}
@@ -327,7 +332,9 @@ public class AnnotationsPropertySourceTests {
 	static @interface AttributeWithAliasAnnotation {
 
 		@AliasFor(annotation = AliasedAttributeAnnotation.class, attribute = "value")
-		String value() default "foo";
+		String value()
+
+		default "foo";
 
 		String someOtherAttribute() default "shouldNotBeMapped";
 
@@ -341,4 +348,15 @@ public class AnnotationsPropertySourceTests {
 
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@SelfAnnotating
+	static @interface SelfAnnotating {
+
+	}
+
+	@SelfAnnotating
+	static class PropertyMappedWithSelfAnnotatingAnnotation {
+
+	}
+
 }

spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java

@@ -49,6 +49,7 @@ import org.springframework.test.context.MergedContextConfiguration;
  * test classes.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  * @see ImportsContextCustomizerFactory
  */
 class ImportsContextCustomizer implements ContextCustomizer {
@@ -217,27 +218,31 @@ class ImportsContextCustomizer implements ContextCustomizer {
 
 		ContextCustomizerKey(Class<?> testClass) {
 			Set<Annotation> annotations = new HashSet<Annotation>();
-			collectClassAnnotations(testClass, annotations);
+			Set<Class<?>> seen = new HashSet<Class<?>>();
+			collectClassAnnotations(testClass, annotations, seen);
 			this.annotations = Collections.unmodifiableSet(annotations);
 		}
 
 		private void collectClassAnnotations(Class<?> classType,
-				Set<Annotation> annotations) {
-			collectElementAnnotations(classType, annotations);
-			for (Class<?> interfaceType : classType.getInterfaces()) {
-				collectClassAnnotations(interfaceType, annotations);
-			}
-			if (classType.getSuperclass() != null) {
-				collectClassAnnotations(classType.getSuperclass(), annotations);
+				Set<Annotation> annotations, Set<Class<?>> seen) {
+			if (seen.add(classType)) {
+				collectElementAnnotations(classType, annotations, seen);
+				for (Class<?> interfaceType : classType.getInterfaces()) {
+					collectClassAnnotations(interfaceType, annotations, seen);
+				}
+				if (classType.getSuperclass() != null) {
+					collectClassAnnotations(classType.getSuperclass(), annotations, seen);
+				}
 			}
 		}
 
 		private void collectElementAnnotations(AnnotatedElement element,
-				Set<Annotation> annotations) {
+				Set<Annotation> annotations, Set<Class<?>> seen) {
 			for (Annotation annotation : element.getDeclaredAnnotations()) {
 				if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) {
 					annotations.add(annotation);
-					collectClassAnnotations(annotation.annotationType(), annotations);
+					collectClassAnnotations(annotation.annotationType(), annotations,
+							seen);
 				}
 			}
 		}

spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerFactoryTests.java

@@ -37,6 +37,7 @@ import static org.mockito.Mockito.mock;
  * Tests for {@link ImportsContextCustomizerFactory} and {@link ImportsContextCustomizer}.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  */
 public class ImportsContextCustomizerFactoryTests {
 
@@ -101,6 +102,12 @@ public class ImportsContextCustomizerFactoryTests {
 		assertThat(context.getBean(ImportedBean.class)).isNotNull();
 	}
 
+	@Test
+	public void selfAnnotatingAnnotationDoesNotCauseStackOverflow() {
+		assertThat(this.factory.createContextCustomizer(
+				TestWithImportAndSelfAnnotatingAnnotation.class, null)).isNotNull();
+	}
+
 	static class TestWithNoImport {
 
 	}
@@ -137,6 +144,12 @@ public class ImportsContextCustomizerFactoryTests {
 
 	}
 
+	@SelfAnnotating
+	@Import(ImportedBean.class)
+	static class TestWithImportAndSelfAnnotatingAnnotation {
+
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Import(ImportedBean.class)
 	@interface MetaImport {
@@ -153,4 +166,10 @@ public class ImportsContextCustomizerFactoryTests {
 
 	}
 
+	@Retention(RetentionPolicy.RUNTIME)
+	@SelfAnnotating
+	static @interface SelfAnnotating {
+
+	}
+
 }