Skip to content

Commit

Permalink
Support recursive annotations in merged annotations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Feb 12, 2022
1 parent 4eaee1e commit 3ec612a
Show file tree
Hide file tree
Showing 7 changed files with 279 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -82,8 +82,8 @@ final class AnnotationTypeMapping {
private final Set<Method> claimedAliases = new HashSet<>();


AnnotationTypeMapping(@Nullable AnnotationTypeMapping source,
Class<? extends Annotation> annotationType, @Nullable Annotation annotation) {
AnnotationTypeMapping(@Nullable AnnotationTypeMapping source, Class<? extends Annotation> annotationType,
@Nullable Annotation annotation, Set<Class<? extends Annotation>> visitedAnnotationTypes) {

this.source = source;
this.root = (source != null ? source.getRoot() : this);
Expand All @@ -103,7 +103,7 @@ final class AnnotationTypeMapping {
processAliases();
addConventionMappings();
addConventionAnnotationValues();
this.synthesizable = computeSynthesizableFlag();
this.synthesizable = computeSynthesizableFlag(visitedAnnotationTypes);
}


Expand Down Expand Up @@ -311,7 +311,10 @@ private boolean isBetterConventionAnnotationValue(int index, boolean isValueAttr
}

@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) {
Expand Down Expand Up @@ -340,9 +343,15 @@ private boolean computeSynthesizableFlag() {
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);
if (mapping.isSynthesizable()) {
return true;
// Ensure we have not yet visited the current nested annotation type, in order
// to avoid infinite recursion for JVM languages other than Java that support
// recursive annotation definitions.
if (visitedAnnotationTypes.add(annotationType)) {
AnnotationTypeMapping mapping =
AnnotationTypeMappings.forAnnotationType(annotationType, visitedAnnotationTypes).get(0);
if (mapping.isSynthesizable()) {
return true;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -20,8 +20,10 @@
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;
Expand All @@ -40,6 +42,7 @@
* be searched once, regardless of how many times they are actually used.
*
* @author Phillip Webb
* @author Sam Brannen
* @since 5.2
* @see AnnotationTypeMapping
*/
Expand All @@ -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);
Expand Down Expand Up @@ -102,14 +108,15 @@ private void addMetaAnnotationsToQueue(Deque<AnnotationTypeMapping> queue, Annot
}

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);
Expand Down Expand Up @@ -166,20 +173,22 @@ AnnotationTypeMapping get(int index) {
* @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
* 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
*/
static AnnotationTypeMappings forAnnotationType(
Class<? extends Annotation> annotationType, AnnotationFilter annotationFilter) {
static AnnotationTypeMappings forAnnotationType(Class<? extends Annotation> annotationType,
Set<Class<? extends Annotation>> visitedAnnotationTypes) {

return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(), annotationFilter);
return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(),
AnnotationFilter.PLAIN, visitedAnnotationTypes);
}

/**
Expand All @@ -194,15 +203,34 @@ static AnnotationTypeMappings forAnnotationType(
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() {
Expand Down Expand Up @@ -235,14 +263,21 @@ private static class Cache {
/**
* 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) {
return this.mappings.computeIfAbsent(annotationType, this::createMappings);
AnnotationTypeMappings get(Class<? extends Annotation> annotationType,
Set<Class<? extends Annotation>> visitedAnnotationTypes) {

return this.mappings.computeIfAbsent(annotationType, key -> createMappings(key, visitedAnnotationTypes));
}

AnnotationTypeMappings createMappings(Class<? extends Annotation> annotationType) {
return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType);
private AnnotationTypeMappings createMappings(Class<? extends Annotation> annotationType,
Set<Class<? extends Annotation>> visitedAnnotationTypes) {

return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType, visitedAnnotationTypes);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -88,7 +88,7 @@ void forAnnotationTypeWhenHasRepeatingMetaAnnotationReturnsMapping() {
@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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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()

)
Original file line number Diff line number Diff line change
@@ -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

)
Loading

0 comments on commit 3ec612a

Please sign in to comment.