From 9384e5c3c1e59a7aa4777308a015adbf4e857bb4 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Thu, 15 Mar 2018 14:09:27 -0700 Subject: [PATCH] Fix binding to bean with cloned arrays Fixes gh-12478 --- .../properties/bind/AggregateBinder.java | 8 +++--- .../context/properties/bind/ArrayBinder.java | 3 ++- .../properties/bind/CollectionBinder.java | 14 +++++++--- .../context/properties/bind/MapBinder.java | 12 ++++++--- .../bind/CollectionBinderTests.java | 26 +++++++++++++++++++ 5 files changed, 51 insertions(+), 12 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 b1c50b1908..5ce680c39e 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 @@ -57,10 +57,10 @@ abstract class AggregateBinder { AggregateElementBinder elementBinder) { Object result = bindAggregate(name, target, elementBinder); Supplier value = target.getValue(); - if (result == null || value == null || value.get() == null) { + if (result == null || value == null) { return result; } - return merge((T) value.get(), (T) result); + return merge(value, (T) result); } /** @@ -75,11 +75,11 @@ abstract class AggregateBinder { /** * Merge any additional elements into the existing aggregate. - * @param existing the existing value + * @param existing the supplier for the existing value * @param additional the additional elements to merge * @return the merged result */ - protected abstract T merge(T existing, T additional); + protected abstract T merge(Supplier existing, T additional); /** * Return the context being used by this binder. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ArrayBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ArrayBinder.java index e67800a5cd..057542fde3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ArrayBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ArrayBinder.java @@ -19,6 +19,7 @@ package org.springframework.boot.context.properties.bind; import java.lang.reflect.Array; import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; import org.springframework.boot.context.properties.bind.Binder.Context; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; @@ -55,7 +56,7 @@ class ArrayBinder extends IndexedElementsBinder { } @Override - protected Object merge(Object existing, Object additional) { + protected Object merge(Supplier existing, Object additional) { return additional; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java index 102a19d8d4..ac953607dd 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.properties.bind; import java.util.Collection; import java.util.List; +import java.util.function.Supplier; import org.springframework.boot.context.properties.bind.Binder.Context; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; @@ -54,12 +55,17 @@ class CollectionBinder extends IndexedElementsBinder> { } @Override - protected Collection merge(Collection existing, + @SuppressWarnings("unchecked") + protected Collection merge(Supplier existing, Collection additional) { + Collection existingCollection = (Collection) existing.get(); + if (existingCollection == null) { + return additional; + } try { - existing.clear(); - existing.addAll(additional); - return existing; + existingCollection.clear(); + existingCollection.addAll(additional); + return existingCollection; } catch (UnsupportedOperationException ex) { return createNewCollection(additional); 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 c90a028f01..1d02a1020f 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 @@ -19,6 +19,7 @@ package org.springframework.boot.context.properties.bind; import java.util.Collection; import java.util.Map; import java.util.Properties; +import java.util.function.Supplier; import org.springframework.boot.context.properties.bind.Binder.Context; import org.springframework.boot.context.properties.source.ConfigurationProperty; @@ -82,10 +83,15 @@ class MapBinder extends AggregateBinder> { } @Override - protected Map merge(Map existing, + @SuppressWarnings("unchecked") + protected Map merge(Supplier existing, Map additional) { - existing.putAll(additional); - return existing; + Map existingMap = (Map) existing.get(); + if (existingMap == null) { + return additional; + } + existingMap.putAll(additional); + return existingMap; } private class EntryBinder { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java index fe4ffc43e7..ec1e18db7d 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java @@ -384,6 +384,17 @@ public class CollectionBinderTests { this.binder.bind("foo", target); } + @Test + public void bindToBeanWithClonedArray() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.bar[0]", "hello"); + this.sources.add(source); + Bindable target = Bindable + .of(ClonedArrayBean.class); + ClonedArrayBean bean = this.binder.bind("foo", target).get(); + assertThat(bean.getBar()).contains("hello"); + } + public static class ExampleCollectionBean { private List items = new ArrayList<>(); @@ -457,4 +468,19 @@ public class CollectionBinderTests { } } + + public static class ClonedArrayBean { + + private String[] bar; + + public String[] getBar() { + return this.bar.clone(); + } + + public void setBar(String[] bar) { + this.bar = bar; + } + + } + }