From 59bc3c56021dc7ca0255a0e23880605e0ba51d1a Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 5 Nov 2019 14:46:19 +0000 Subject: [PATCH] Prevent recursive config props from causing a stack overflow Previously, when the configuration properties annotation processor encountered a property that was the same as an outer type that had already been processed, it would fail with a stack overflow error. This commit introduces the use of a stack to track the types that have been processed. Types that have been seen before are skipped, thereby preventing a failure from occurring. We do not fail upon encountering a recursive type to allow metadata generation to complete. At runtime, the recursive property will not cause a problem if it is not bound. Fixes gh-18365 --- ...figurationMetadataAnnotationProcessor.java | 24 ++++++++----- ...ationMetadataAnnotationProcessorTests.java | 6 ++++ .../recursive/RecursiveProperties.java | 34 +++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/recursive/RecursiveProperties.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java index 2550ffd9cae..d4ab4adc06a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java @@ -26,6 +26,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.Stack; import javax.annotation.processing.AbstractProcessor; import javax.annotation.processing.ProcessingEnvironment; @@ -258,8 +259,10 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor Map fieldValues = members.getFieldValues(); processSimpleTypes(prefix, element, source, members, fieldValues); processSimpleLombokTypes(prefix, element, source, members, fieldValues); - processNestedTypes(prefix, element, source, members); - processNestedLombokTypes(prefix, element, source, members); + Stack seen = new Stack<>(); + seen.push(element); + processNestedTypes(prefix, element, source, members, seen); + processNestedLombokTypes(prefix, element, source, members, seen); } private void processSimpleTypes(String prefix, TypeElement element, ExecutableElement source, @@ -324,19 +327,19 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor } private void processNestedTypes(String prefix, TypeElement element, ExecutableElement source, - TypeElementMembers members) { + TypeElementMembers members, Stack seen) { members.getPublicGetters().forEach((name, getter) -> { VariableElement field = members.getFields().get(name); - processNestedType(prefix, element, source, name, getter, field, getter.getReturnType()); + processNestedType(prefix, element, source, name, getter, field, getter.getReturnType(), seen); }); } private void processNestedLombokTypes(String prefix, TypeElement element, ExecutableElement source, - TypeElementMembers members) { + TypeElementMembers members, Stack seen) { members.getFields().forEach((name, field) -> { if (isLombokField(field, element)) { ExecutableElement getter = members.getPublicGetter(name, field.asType()); - processNestedType(prefix, element, source, name, getter, field, field.asType()); + processNestedType(prefix, element, source, name, getter, field, field.asType(), seen); } }); } @@ -378,7 +381,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor } private void processNestedType(String prefix, TypeElement element, ExecutableElement source, String name, - ExecutableElement getter, VariableElement field, TypeMirror returnType) { + ExecutableElement getter, VariableElement field, TypeMirror returnType, Stack seen) { Element returnElement = this.processingEnv.getTypeUtils().asElement(returnType); boolean isNested = isNested(returnElement, field, element); AnnotationMirror annotation = getAnnotation(getter, configurationPropertiesAnnotation()); @@ -387,7 +390,12 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor this.metadataCollector .add(ItemMetadata.newGroup(nestedPrefix, this.typeUtils.getQualifiedName(returnElement), this.typeUtils.getQualifiedName(element), (getter != null) ? getter.toString() : null)); - processTypeElement(nestedPrefix, (TypeElement) returnElement, source); + TypeElement nestedElement = (TypeElement) returnElement; + if (!seen.contains(nestedElement)) { + seen.push(nestedElement); + processTypeElement(nestedPrefix, nestedElement, source); + seen.pop(); + } } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java index abca207714c..425552345f1 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java @@ -68,6 +68,7 @@ import org.springframework.boot.configurationsample.method.EmptyTypeMethodConfig import org.springframework.boot.configurationsample.method.InvalidMethodConfig; import org.springframework.boot.configurationsample.method.MethodAndClassConfig; import org.springframework.boot.configurationsample.method.SimpleMethodConfig; +import org.springframework.boot.configurationsample.recursive.RecursiveProperties; import org.springframework.boot.configurationsample.simple.ClassWithNestedProperties; import org.springframework.boot.configurationsample.simple.DeprecatedFieldSingleProperty; import org.springframework.boot.configurationsample.simple.DeprecatedSingleProperty; @@ -970,6 +971,11 @@ public class ConfigurationMetadataAnnotationProcessorTests { .has(Metadata.withProperty("bar.counter").withDefaultValue(0).fromSource(RenamedBarProperties.class)); } + @Test + public void recursivePropertiesDoNotCauseAStackOverflow() { + compile(RecursiveProperties.class); + } + private void assertSimpleLombokProperties(ConfigurationMetadata metadata, Class source, String prefix) { assertThat(metadata).has(Metadata.withGroup(prefix).fromSource(source)); assertThat(metadata).doesNotHave(Metadata.withProperty(prefix + ".id")); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/recursive/RecursiveProperties.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/recursive/RecursiveProperties.java new file mode 100644 index 00000000000..2d2d63c97e4 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/recursive/RecursiveProperties.java @@ -0,0 +1,34 @@ +/* + * Copyright 2012-2018 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.boot.configurationsample.recursive; + +import org.springframework.boot.configurationsample.ConfigurationProperties; + +@ConfigurationProperties("prefix") +public class RecursiveProperties { + + private RecursiveProperties recursive; + + public RecursiveProperties getRecursive() { + return this.recursive; + } + + public void setRecursive(RecursiveProperties recursive) { + this.recursive = recursive; + } + +}