Skip to content

Commit c75481e

Browse files
committed
Reject binding non-uniform property key as map key
Given: ```yaml my: map: "/key": "value" ``` --- Before this commit: It's equivalent to ```yaml my: map: "[key]": "value" # "[/key]" is expected ``` Such counter-intuitive behavior will confuse developers, there are several reported issues, incomplete list: GH-41099 GH-29582 GH-24548 --- After this commit: ``` *************************** APPLICATION FAILED TO START *************************** Description: Failed to bind properties under 'my.map' to java.util.Map<java.lang.String, java.lang.String>: Reason: java.lang.IllegalArgumentException: Please rewrite key '/key' to '[/key]' and surround it with quotes if YAML is using Action: Update your application's configuration ``` --- See GH-42802
1 parent 24202a0 commit c75481e

File tree

3 files changed

+54
-0
lines changed

3 files changed

+54
-0
lines changed

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
*
3838
* @author Phillip Webb
3939
* @author Madhura Bhave
40+
* @author Yanming Zhou
4041
*/
4142
class MapBinder extends AggregateBinder<Map<Object, Object>> {
4243

@@ -166,6 +167,16 @@ private class EntryBinder {
166167
void bindEntries(ConfigurationPropertySource source, Map<Object, Object> map) {
167168
if (source instanceof IterableConfigurationPropertySource iterableSource) {
168169
for (ConfigurationPropertyName name : iterableSource) {
170+
if (name.isLastElementNonUniform()) {
171+
String sourceName = name.getSource();
172+
int index = sourceName.lastIndexOf('.');
173+
String key = sourceName.substring(index + 1);
174+
if (!key.equalsIgnoreCase(name.getLastElement(Form.UNIFORM))) {
175+
// ignore non-uniform caused by case-insensitive
176+
throw new IllegalArgumentException("Please rewrite key '" + key + "' to '[" + key
177+
+ "]' and surround it with quotes if YAML is using");
178+
}
179+
}
169180
Bindable<?> valueBindable = getValueBindable(name);
170181
ConfigurationPropertyName entryName = getEntryName(source, name);
171182
Object key = getContext().getConverter().convert(getKeyName(entryName), this.keyType);

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
*
4747
* @author Phillip Webb
4848
* @author Madhura Bhave
49+
* @author Yanming Zhou
4950
* @since 2.0.0
5051
* @see #of(CharSequence)
5152
* @see ConfigurationPropertySource
@@ -121,6 +122,23 @@ public boolean isNumericIndex(int elementIndex) {
121122
return this.elements.getType(elementIndex) == ElementType.NUMERICALLY_INDEXED;
122123
}
123124

125+
/**
126+
* Return whether the last element is non-uniform.
127+
* @return whether the last element is non-uniform
128+
*/
129+
public boolean isLastElementNonUniform() {
130+
int size = getNumberOfElements();
131+
return size != 0 && this.elements.getType(size - 1) == ElementType.NON_UNIFORM;
132+
}
133+
134+
/**
135+
* Return the source.
136+
* @return the source
137+
*/
138+
public String getSource() {
139+
return this.elements.source.toString();
140+
}
141+
124142
/**
125143
* Return the last element in the name in the given form.
126144
* @param form the form to return

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
*
6565
* @author Phillip Webb
6666
* @author Madhura Bhave
67+
* @author Yanming Zhou
6768
*/
6869
class MapBinderTests {
6970

@@ -651,6 +652,30 @@ void bindToEnumMapShouldBind() {
651652
assertThat(result).hasSize(1).containsEntry(ExampleEnum.FOO_BAR, "value");
652653
}
653654

655+
@Test
656+
void rejectNonUniformKey() {
657+
MapConfigurationPropertySource source = new MapConfigurationPropertySource(
658+
Collections.singletonMap("props./foobar", "value"));
659+
this.sources.add(source);
660+
Binder binder = new Binder(this.sources, null, null, null);
661+
Bindable<Map<String, String>> bindable = Bindable
662+
.of(ResolvableType.forClassWithGenerics(Map.class, String.class, String.class));
663+
assertThatExceptionOfType(BindException.class).isThrownBy(() -> binder.bind("props", bindable))
664+
.withCauseInstanceOf(IllegalArgumentException.class);
665+
}
666+
667+
@Test
668+
void doNotRejectCaseInsensitiveNonUniformKey() {
669+
MapConfigurationPropertySource source = new MapConfigurationPropertySource(
670+
Collections.singletonMap("props.FOOBAR", "value"));
671+
this.sources.add(source);
672+
Binder binder = new Binder(this.sources, null, null, null);
673+
Bindable<Map<String, String>> bindable = Bindable
674+
.of(ResolvableType.forClassWithGenerics(Map.class, String.class, String.class));
675+
Map<String, String> result = binder.bind("props", bindable).get();
676+
assertThat(result).hasSize(1).containsEntry("FOOBAR", "value");
677+
}
678+
654679
private <K, V> Bindable<Map<K, V>> getMapBindable(Class<K> keyGeneric, ResolvableType valueType) {
655680
ResolvableType keyType = ResolvableType.forClass(keyGeneric);
656681
return Bindable.of(ResolvableType.forClassWithGenerics(Map.class, keyType, valueType));

0 commit comments

Comments
 (0)