From 588dcf76c64da8856df82e9d76f86c6b919a6f02 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 4 Nov 2017 22:26:11 -0700 Subject: [PATCH] Only allow binder recursion with Maps Refine the binder recursion rules introduced in commit f337323819 so they only apply to Maps. Recursive List and Array binding is now no longer allowed. Prior to this commit, binding a List that contained a reference to itself would fail with a `StackOverflowException` if the underlying property source was not iterable. Fixes gh-10702 --- .../properties/bind/AggregateBinder.java | 9 ++++++++- .../boot/context/properties/bind/Binder.java | 17 +++++++++-------- .../properties/bind/IndexedElementsBinder.java | 2 +- .../boot/context/properties/bind/MapBinder.java | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/AggregateBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/AggregateBinder.java index 90aeab57f7..3eb97bfd1d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/AggregateBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/AggregateBinder.java @@ -31,8 +31,15 @@ abstract class AggregateBinder { private final BindContext context; - AggregateBinder(BindContext context) { + private final boolean allowRecursiveBinding; + + AggregateBinder(BindContext context, boolean allowRecursiveBinding) { this.context = context; + this.allowRecursiveBinding = allowRecursiveBinding; + } + + boolean isAllowRecursiveBinding() { + return this.allowRecursiveBinding; } /** diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index cf35bff252..26b7c4dad6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -188,13 +188,14 @@ public class Binder { } protected final T bind(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context, boolean skipIfHasBoundBean) { + BindHandler handler, Context context, boolean allowRecursiveBinding) { context.clearConfigurationProperty(); try { if (!handler.onStart(name, target, context)) { return null; } - Object bound = bindObject(name, target, handler, context, skipIfHasBoundBean); + Object bound = bindObject(name, target, handler, context, + allowRecursiveBinding); return handleBindResult(name, target, handler, context, bound); } catch (Exception ex) { @@ -232,7 +233,7 @@ public class Binder { } private Object bindObject(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context, boolean skipIfHasBoundBean) + BindHandler handler, Context context, boolean allowRecursiveBinding) throws Exception { ConfigurationProperty property = findProperty(name, context); if (property == null && containsNoDescendantOf(context.streamSources(), name)) { @@ -245,7 +246,7 @@ public class Binder { if (property != null) { return bindProperty(name, target, handler, context, property); } - return bindBean(name, target, handler, context, skipIfHasBoundBean); + return bindBean(name, target, handler, context, allowRecursiveBinding); } private AggregateBinder getAggregateBinder(Bindable target, Context context) { @@ -266,7 +267,7 @@ public class Binder { BindHandler handler, Context context, AggregateBinder aggregateBinder) { AggregateElementBinder elementBinder = (itemName, itemTarget, source) -> { Supplier supplier = () -> bind(itemName, itemTarget, handler, context, - false); + aggregateBinder.isAllowRecursiveBinding()); return context.withSource(source, supplier); }; return context.withIncreasedDepth( @@ -290,15 +291,15 @@ public class Binder { } private Object bindBean(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context, boolean skipIfHasBoundBean) { + BindHandler handler, Context context, boolean allowRecursiveBinding) { if (containsNoDescendantOf(context.streamSources(), name) || isUnbindableBean(name, target, context)) { return null; } BeanPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind( - name.append(propertyName), propertyTarget, handler, context, true); + name.append(propertyName), propertyTarget, handler, context, false); Class type = target.getType().resolve(); - if (skipIfHasBoundBean && context.hasBoundBean(type)) { + if (!allowRecursiveBinding && context.hasBoundBean(type)) { return null; } return context.withBean(type, () -> { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java index d9e09fe87e..123190f71e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java @@ -45,7 +45,7 @@ abstract class IndexedElementsBinder extends AggregateBinder { private static final String INDEX_ZERO = "[0]"; IndexedElementsBinder(BindContext context) { - super(context); + super(context, false); } protected final void bindIndexed(ConfigurationPropertyName name, Bindable target, diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java index c7bd816070..358d3db96e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java @@ -41,7 +41,7 @@ class MapBinder extends AggregateBinder> { .mapOf(String.class, String.class); MapBinder(BindContext context) { - super(context); + super(context, true); } @Override