From 650cbeee1433aa38619691946c316d9d03a6512d Mon Sep 17 00:00:00 2001 From: yilianhuaixiao <85142297@qq.com> Date: Mon, 20 Jul 2020 17:30:25 +0800 Subject: [PATCH 1/2] Avoid infinite loop in AnnotationScanner Prior to this commit, scanning for annotations resulted in an infinite loop when using the INHERITED_ANNOTATIONS search strategy and a class filter that filters out visited classes. This commit avoids an infinite loop in AnnotationsScanner's processClassInheritedAnnotations(...) method by skipping the current level of the class hierarchy when the current source class has been filtered out. Closes gh-25429 --- .../core/annotation/AnnotationsScanner.java | 2 ++ .../core/annotation/AnnotationsScannerTests.java | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java index 67ccd303244..795c7a036c0 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java @@ -151,6 +151,8 @@ abstract class AnnotationsScanner { return result; } if (isFiltered(source, context, classFilter)) { + source = source.getSuperclass(); + aggregateIndex++; continue; } Annotation[] declaredAnnotations = diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java index 635452343b2..80a621930cc 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java @@ -499,6 +499,22 @@ class AnnotationsScannerTests { assertThat(result).isEqualTo("OK"); } + @Test + void scanWithFilteredAll() { + List indexes = new ArrayList<>(); + String result = AnnotationsScanner.scan(this, WithSingleSuperclass.class, + SearchStrategy.INHERITED_ANNOTATIONS, + (context, aggregateIndex, source, annotations) -> { + indexes.add(aggregateIndex); + return ""; + }, + (context,cls)->{ + return true; + } + ); + assertThat(result).isNull(); + } + private Method methodFrom(Class type) { return ReflectionUtils.findMethod(type, "method"); From 5442d8779aa6ae3d5495255853ef7be8a14ca6da Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 25 Jul 2020 12:27:21 +0200 Subject: [PATCH 2/2] Polish contribution See gh-25429 --- .../core/annotation/AnnotationsScanner.java | 3 +- .../annotation/AnnotationsScannerTests.java | 101 ++++++++++++------ 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java index 795c7a036c0..b4841e3cf19 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -151,6 +151,7 @@ abstract class AnnotationsScanner { return result; } if (isFiltered(source, context, classFilter)) { + // Skip the current level in the class hierarchy. source = source.getSuperclass(); aggregateIndex++; continue; diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java index 80a621930cc..fd929d63bf2 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java @@ -25,13 +25,13 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.lang.Nullable; -import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link AnnotationsScanner}. * * @author Phillip Webb + * @author Sam Brannen */ class AnnotationsScannerTests { @@ -115,7 +116,7 @@ class AnnotationsScannerTests { void inheritedAnnotationsStrategyOnClassHierarchyScansInCorrectOrder() { Class source = WithHierarchy.class; assertThat(scan(source, SearchStrategy.INHERITED_ANNOTATIONS)).containsExactly( - "0:TestAnnotation1", "1:TestInheritedAnnotation2"); + "0:TestAnnotation1", "1:TestInheritedAnnotation2", "2:TestInheritedAnnotation3"); } @Test @@ -124,7 +125,52 @@ class AnnotationsScannerTests { assertThat(Arrays.stream(source.getAnnotations()).map( Annotation::annotationType).map(Class::getName)).containsExactly( TestInheritedAnnotation2.class.getName()); - assertThat(scan(source, SearchStrategy.INHERITED_ANNOTATIONS)).containsOnly("0:TestInheritedAnnotation2"); + assertThat(scan(source, SearchStrategy.INHERITED_ANNOTATIONS)).containsExactly("0:TestInheritedAnnotation2"); + } + + @Test + void inheritedAnnotationsStrategyOnClassWithAllClassesFilteredOut() { + List annotationsFound = new ArrayList<>(); + String scanResult = AnnotationsScanner.scan(this, WithSingleSuperclass.class, + SearchStrategy.INHERITED_ANNOTATIONS, + (context, aggregateIndex, source, annotations) -> { + trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); + return null; // continue searching + }, + (context, clazz) -> true // filter out all classes + ); + assertThat(annotationsFound).isEmpty(); + assertThat(scanResult).isNull(); + } + + @Test + void inheritedAnnotationsStrategyOnClassWithSourceClassFilteredOut() { + List annotationsFound = new ArrayList<>(); + String scanResult = AnnotationsScanner.scan(this, WithSingleSuperclass.class, + SearchStrategy.INHERITED_ANNOTATIONS, + (context, aggregateIndex, source, annotations) -> { + trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); + return null; // continue searching + }, + (context, clazz) -> clazz == WithSingleSuperclass.class + ); + assertThat(annotationsFound).containsExactly(/* "0:TestAnnotation1", */ "1:TestInheritedAnnotation2"); + assertThat(scanResult).isNull(); + } + + @Test + void inheritedAnnotationsStrategyInClassHierarchyWithSuperSuperclassFilteredOut() { + List annotationsFound = new ArrayList<>(); + String scanResult = AnnotationsScanner.scan(this, WithHierarchy.class, + SearchStrategy.INHERITED_ANNOTATIONS, + (context, aggregateIndex, source, annotations) -> { + trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); + return null; // continue searching + }, + (context, clazz) -> clazz == HierarchySuperSuperclass.class + ); + assertThat(annotationsFound).containsExactly("0:TestAnnotation1", "1:TestInheritedAnnotation2"); + assertThat(scanResult).isNull(); } @Test @@ -163,7 +209,7 @@ class AnnotationsScannerTests { Class source = WithHierarchy.class; assertThat(scan(source, SearchStrategy.SUPERCLASS)).containsExactly( "0:TestAnnotation1", "1:TestAnnotation2", "1:TestInheritedAnnotation2", - "2:TestAnnotation3"); + "2:TestAnnotation3", "2:TestInheritedAnnotation3"); } @Test @@ -205,7 +251,7 @@ class AnnotationsScannerTests { assertThat(scan(source, SearchStrategy.TYPE_HIERARCHY)).containsExactly( "0:TestAnnotation1", "1:TestAnnotation5", "1:TestInheritedAnnotation5", "2:TestAnnotation6", "3:TestAnnotation2", "3:TestInheritedAnnotation2", - "4:TestAnnotation3", "5:TestAnnotation4"); + "4:TestAnnotation3", "4:TestInheritedAnnotation3", "5:TestAnnotation4"); } @Test @@ -474,7 +520,7 @@ class AnnotationsScannerTests { return ""; }); assertThat(result).isEmpty(); - assertThat(indexes).containsOnly(0); + assertThat(indexes).containsExactly(0); } @Test @@ -499,42 +545,30 @@ class AnnotationsScannerTests { assertThat(result).isEqualTo("OK"); } - @Test - void scanWithFilteredAll() { - List indexes = new ArrayList<>(); - String result = AnnotationsScanner.scan(this, WithSingleSuperclass.class, - SearchStrategy.INHERITED_ANNOTATIONS, - (context, aggregateIndex, source, annotations) -> { - indexes.add(aggregateIndex); - return ""; - }, - (context,cls)->{ - return true; - } - ); - assertThat(result).isNull(); - } - private Method methodFrom(Class type) { return ReflectionUtils.findMethod(type, "method"); } private Stream scan(AnnotatedElement element, SearchStrategy searchStrategy) { - List result = new ArrayList<>(); + List results = new ArrayList<>(); AnnotationsScanner.scan(this, element, searchStrategy, (criteria, aggregateIndex, source, annotations) -> { - for (Annotation annotation : annotations) { - if (annotation != null) { - String name = ClassUtils.getShortName( - annotation.annotationType()); - name = name.substring(name.lastIndexOf(".") + 1); - result.add(aggregateIndex + ":" + name); - } - } - return null; + trackIndexedAnnotations(aggregateIndex, annotations, results); + return null; // continue searching }); - return result.stream(); + return results.stream(); + } + + private void trackIndexedAnnotations(int aggregateIndex, Annotation[] annotations, List results) { + Arrays.stream(annotations) + .filter(Objects::nonNull) + .map(annotation -> indexedName(aggregateIndex, annotation)) + .forEach(results::add); + } + + private String indexedName(int aggregateIndex, Annotation annotation) { + return aggregateIndex + ":" + annotation.annotationType().getSimpleName(); } @@ -686,6 +720,7 @@ class AnnotationsScannerTests { } @TestAnnotation3 + @TestInheritedAnnotation3 static class HierarchySuperSuperclass implements HierarchySuperSuperclassInterface { @Override