Support recursive annotations in merged annotations

Although Java does not allow the definition of recursive annotations,
Kotlin does, and prior to this commit an attempt to synthesize a
merged annotation using the MergedAnnotation API resulted in a
StackOverflowError if there was a recursive cycle in the annotation
definitions.

This commit addresses this issue by tracking which annotations have
already been visited and short circuits the recursive algorithm if a
cycle is detected.

Closes gh-28012

spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -82,8 +82,8 @@ final class AnnotationTypeMapping {
 	private final Set<Method> claimedAliases = new HashSet<>();
 
 
-	AnnotationTypeMapping(@Nullable AnnotationTypeMapping source,
+	AnnotationTypeMapping(@Nullable AnnotationTypeMapping source, Class<? extends Annotation> annotationType,
-			Class<? extends Annotation> annotationType, @Nullable Annotation annotation) {
+			@Nullable Annotation annotation, Set<Class<? extends Annotation>> visitedAnnotationTypes) {
 
 		this.source = source;
 		this.root = (source != null ? source.getRoot() : this);
@@ -103,7 +103,7 @@ final class AnnotationTypeMapping {
 		processAliases();
 		addConventionMappings();
 		addConventionAnnotationValues();
-		this.synthesizable = computeSynthesizableFlag();
+		this.synthesizable = computeSynthesizableFlag(visitedAnnotationTypes);
 	}
 
 
@@ -311,7 +311,10 @@ final class AnnotationTypeMapping {
 	}
 
 	@SuppressWarnings("unchecked")
-	private boolean computeSynthesizableFlag() {
+	private boolean computeSynthesizableFlag(Set<Class<? extends Annotation>> visitedAnnotationTypes) {
+		// Track that we have visited the current annotation type.
+		visitedAnnotationTypes.add(this.annotationType);
+
 		// Uses @AliasFor for local aliases?
 		for (int index : this.aliasMappings) {
 			if (index != -1) {
@@ -340,9 +343,15 @@ final class AnnotationTypeMapping {
 				if (type.isAnnotation() || (type.isArray() && type.getComponentType().isAnnotation())) {
 					Class<? extends Annotation> annotationType =
 							(Class<? extends Annotation>) (type.isAnnotation() ? type : type.getComponentType());
-					AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0);
+					// Ensure we have not yet visited the current nested annotation type, in order
-					if (mapping.isSynthesizable()) {
+					// to avoid infinite recursion for JVM languages other than Java that support
-						return true;
+					// recursive annotation definitions.
+					if (visitedAnnotationTypes.add(annotationType)) {
+						AnnotationTypeMapping mapping =
+								AnnotationTypeMappings.forAnnotationType(annotationType, visitedAnnotationTypes).get(0);
+						if (mapping.isSynthesizable()) {
+							return true;
+						}
 					}
 				}
 			}

spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -20,8 +20,10 @@ import java.lang.annotation.Annotation;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.springframework.lang.Nullable;
 import org.springframework.util.ConcurrentReferenceHashMap;
@@ -40,6 +42,7 @@ import org.springframework.util.ConcurrentReferenceHashMap;
  * be searched once, regardless of how many times they are actually used.
  *
  * @author Phillip Webb
+ * @author Sam Brannen
  * @since 5.2
  * @see AnnotationTypeMapping
  */
@@ -60,19 +63,22 @@ final class AnnotationTypeMappings {
 
 
 	private AnnotationTypeMappings(RepeatableContainers repeatableContainers,
-			AnnotationFilter filter, Class<? extends Annotation> annotationType) {
+			AnnotationFilter filter, Class<? extends Annotation> annotationType,
+			Set<Class<? extends Annotation>> visitedAnnotationTypes) {
 
 		this.repeatableContainers = repeatableContainers;
 		this.filter = filter;
 		this.mappings = new ArrayList<>();
-		addAllMappings(annotationType);
+		addAllMappings(annotationType, visitedAnnotationTypes);
 		this.mappings.forEach(AnnotationTypeMapping::afterAllMappingsSet);
 	}
 
 
-	private void addAllMappings(Class<? extends Annotation> annotationType) {
+	private void addAllMappings(Class<? extends Annotation> annotationType,
+			Set<Class<? extends Annotation>> visitedAnnotationTypes) {
+
 		Deque<AnnotationTypeMapping> queue = new ArrayDeque<>();
-		addIfPossible(queue, null, annotationType, null);
+		addIfPossible(queue, null, annotationType, null, visitedAnnotationTypes);
 		while (!queue.isEmpty()) {
 			AnnotationTypeMapping mapping = queue.removeFirst();
 			this.mappings.add(mapping);
@@ -102,14 +108,15 @@ final class AnnotationTypeMappings {
 	}
 
 	private void addIfPossible(Deque<AnnotationTypeMapping> queue, AnnotationTypeMapping source, Annotation ann) {
-		addIfPossible(queue, source, ann.annotationType(), ann);
+		addIfPossible(queue, source, ann.annotationType(), ann, new HashSet<>());
 	}
 
 	private void addIfPossible(Deque<AnnotationTypeMapping> queue, @Nullable AnnotationTypeMapping source,
-			Class<? extends Annotation> annotationType, @Nullable Annotation ann) {
+			Class<? extends Annotation> annotationType, @Nullable Annotation ann,
+			Set<Class<? extends Annotation>> visitedAnnotationTypes) {
 
 		try {
-			queue.addLast(new AnnotationTypeMapping(source, annotationType, ann));
+			queue.addLast(new AnnotationTypeMapping(source, annotationType, ann, visitedAnnotationTypes));
 		}
 		catch (Exception ex) {
 			AnnotationUtils.rethrowAnnotationConfigurationException(ex);
@@ -166,20 +173,22 @@ final class AnnotationTypeMappings {
 	 * @return type mappings for the annotation type
 	 */
 	static AnnotationTypeMappings forAnnotationType(Class<? extends Annotation> annotationType) {
-		return forAnnotationType(annotationType, AnnotationFilter.PLAIN);
+		return forAnnotationType(annotationType, new HashSet<>());
 	}
 
 	/**
 	 * Create {@link AnnotationTypeMappings} for the specified annotation type.
 	 * @param annotationType the source annotation type
-	 * @param annotationFilter the annotation filter used to limit which
+	 * @param visitedAnnotationTypes the set of annotations that we have already
-	 * annotations are considered
+	 * visited; used to avoid infinite recursion for recursive annotations which
+	 * some JVM languages support (such as Kotlin)
 	 * @return type mappings for the annotation type
 	 */
-	static AnnotationTypeMappings forAnnotationType(
+	static AnnotationTypeMappings forAnnotationType(Class<? extends Annotation> annotationType,
-			Class<? extends Annotation> annotationType, AnnotationFilter annotationFilter) {
+			Set<Class<? extends Annotation>> visitedAnnotationTypes) {
 
-		return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(), annotationFilter);
+		return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(),
+				AnnotationFilter.PLAIN, visitedAnnotationTypes);
 	}
 
 	/**
@@ -194,15 +203,34 @@ final class AnnotationTypeMappings {
 	static AnnotationTypeMappings forAnnotationType(Class<? extends Annotation> annotationType,
 			RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) {
 
+		return forAnnotationType(annotationType, repeatableContainers, annotationFilter, new HashSet<>());
+	}
+
+	/**
+	 * Create {@link AnnotationTypeMappings} for the specified annotation type.
+	 * @param annotationType the source annotation type
+	 * @param repeatableContainers the repeatable containers that may be used by
+	 * the meta-annotations
+	 * @param annotationFilter the annotation filter used to limit which
+	 * annotations are considered
+	 * @param visitedAnnotationTypes the set of annotations that we have already
+	 * visited; used to avoid infinite recursion for recursive annotations which
+	 * some JVM languages support (such as Kotlin)
+	 * @return type mappings for the annotation type
+	 */
+	private static AnnotationTypeMappings forAnnotationType(Class<? extends Annotation> annotationType,
+			RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter,
+			Set<Class<? extends Annotation>> visitedAnnotationTypes) {
+
 		if (repeatableContainers == RepeatableContainers.standardRepeatables()) {
 			return standardRepeatablesCache.computeIfAbsent(annotationFilter,
-					key -> new Cache(repeatableContainers, key)).get(annotationType);
+					key -> new Cache(repeatableContainers, key)).get(annotationType, visitedAnnotationTypes);
 		}
 		if (repeatableContainers == RepeatableContainers.none()) {
 			return noRepeatablesCache.computeIfAbsent(annotationFilter,
-					key -> new Cache(repeatableContainers, key)).get(annotationType);
+					key -> new Cache(repeatableContainers, key)).get(annotationType, visitedAnnotationTypes);
 		}
-		return new AnnotationTypeMappings(repeatableContainers, annotationFilter, annotationType);
+		return new AnnotationTypeMappings(repeatableContainers, annotationFilter, annotationType, visitedAnnotationTypes);
 	}
 
 	static void clearCache() {
@@ -235,14 +263,21 @@ final class AnnotationTypeMappings {
 		/**
 		 * Get or create {@link AnnotationTypeMappings} for the specified annotation type.
 		 * @param annotationType the annotation type
+		 * @param visitedAnnotationTypes the set of annotations that we have already
+		 * visited; used to avoid infinite recursion for recursive annotations which
+		 * some JVM languages support (such as Kotlin)
 		 * @return a new or existing {@link AnnotationTypeMappings} instance
 		 */
-		AnnotationTypeMappings get(Class<? extends Annotation> annotationType) {
+		AnnotationTypeMappings get(Class<? extends Annotation> annotationType,
-			return this.mappings.computeIfAbsent(annotationType, this::createMappings);
+				Set<Class<? extends Annotation>> visitedAnnotationTypes) {
+
+			return this.mappings.computeIfAbsent(annotationType, key -> createMappings(key, visitedAnnotationTypes));
 		}
 
-		AnnotationTypeMappings createMappings(Class<? extends Annotation> annotationType) {
+		private AnnotationTypeMappings createMappings(Class<? extends Annotation> annotationType,
-			return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType);
+				Set<Class<? extends Annotation>> visitedAnnotationTypes) {
+
+			return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType, visitedAnnotationTypes);
 		}
 	}
 

spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -88,7 +88,7 @@ class AnnotationTypeMappingsTests {
 	@Test
 	void forAnnotationTypeWhenRepeatableMetaAnnotationIsFiltered() {
 		AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(WithRepeatedMetaAnnotations.class,
-				Repeating.class.getName()::equals);
+				RepeatableContainers.standardRepeatables(), Repeating.class.getName()::equals);
 		assertThat(getAll(mappings)).flatExtracting(AnnotationTypeMapping::getAnnotationType)
 				.containsExactly(WithRepeatedMetaAnnotations.class);
 	}

spring-core/src/test/kotlin/org/springframework/core/annotation/Filter.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.annotation
+
+/**
+ * @author Sam Brannen
+ * @since 5.3.16
+ */
+@Target(AnnotationTarget.FUNCTION)
+@Retention(AnnotationRetention.RUNTIME)
+public annotation class Filter(
+
+	@get:AliasFor("name")
+	val value: String = "",
+
+	@get:AliasFor("value")
+	val name: String = "",
+
+	val and: Filters = Filters()
+
+)

spring-core/src/test/kotlin/org/springframework/core/annotation/Filters.kt

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.annotation
+
+/**
+ * @author Sam Brannen
+ * @since 5.3.16
+ */
+@Target(AnnotationTarget.FUNCTION)
+@Retention(AnnotationRetention.RUNTIME)
+public annotation class Filters(
+
+	vararg val value: Filter
+
+)

spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.annotation
+
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.springframework.core.annotation.MergedAnnotations
+import org.springframework.core.annotation.MergedAnnotation
+
+/**
+ * Tests for {@link MergedAnnotations} and {@link MergedAnnotation} in Kotlin.
+ *
+ * @author Sam Brannen
+ * @since 5.3.16
+ */
+class KotlinMergedAnnotationsTests {
+
+	@Test  // gh-28012
+	fun recursiveAnnotation() {
+		val method = javaClass.getMethod("personMethod")
+
+		// MergedAnnotations
+		val mergedAnnotations = MergedAnnotations.from(method)
+		assertThat(mergedAnnotations.isPresent(Person::class.java)).isTrue();
+
+		// MergedAnnotation
+		val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(Person::class.java))
+		assertThat(mergedAnnotation).isNotNull();
+
+		// Synthesized Annotations
+		val jane = mergedAnnotation.synthesize()
+		assertThat(jane).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(jane.value).isEqualTo("jane")
+		assertThat(jane.name).isEqualTo("jane")
+		val synthesizedFriends = jane.friends
+		assertThat(synthesizedFriends).hasSize(2)
+
+		val john = synthesizedFriends[0]
+		assertThat(john).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(john.value).isEqualTo("john")
+		assertThat(john.name).isEqualTo("john")
+
+		val sally = synthesizedFriends[1]
+		assertThat(sally).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(sally.value).isEqualTo("sally")
+		assertThat(sally.name).isEqualTo("sally")
+	}
+
+	@Test  // gh-28012
+	fun recursiveNestedAnnotation() {
+		val method = javaClass.getMethod("filterMethod")
+
+		// MergedAnnotations
+		val mergedAnnotations = MergedAnnotations.from(method)
+		assertThat(mergedAnnotations.isPresent(Filter::class.java)).isTrue();
+
+		// MergedAnnotation
+		val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(Filter::class.java))
+		assertThat(mergedAnnotation).isNotNull();
+
+		// Synthesized Annotations
+		val fooFilter = mergedAnnotation.synthesize()
+		assertThat(fooFilter).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(fooFilter.value).isEqualTo("foo")
+		assertThat(fooFilter.name).isEqualTo("foo")
+		val filters = fooFilter.and
+		assertThat(filters.value).hasSize(2)
+
+		val barFilter = filters.value[0]
+		assertThat(barFilter).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(barFilter.value).isEqualTo("bar")
+		assertThat(barFilter.name).isEqualTo("bar")
+		assertThat(barFilter.and.value).isEmpty()
+
+		val bazFilter = filters.value[1]
+		assertThat(bazFilter).isInstanceOf(SynthesizedAnnotation::class.java)
+		assertThat(bazFilter.value).isEqualTo("baz")
+		assertThat(bazFilter.name).isEqualTo("baz")
+		assertThat(bazFilter.and.value).isEmpty()
+	}
+
+
+	@Person("jane", friends = [Person("john"), Person("sally")])
+	fun personMethod() {
+	}
+
+	@Filter("foo", and = Filters(Filter("bar"), Filter("baz")))
+	fun filterMethod() {
+	}
+
+}

spring-core/src/test/kotlin/org/springframework/core/annotation/Person.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2002-2022 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.annotation
+
+/**
+ * @author Sam Brannen
+ * @since 5.3.16
+ */
+@Target(AnnotationTarget.FUNCTION)
+@Retention(AnnotationRetention.RUNTIME)
+public annotation class Person(
+
+	@get:AliasFor("name")
+	val value: String = "",
+
+	@get:AliasFor("value")
+	val name: String = "",
+
+	vararg val friends: Person = []
+
+)