Skip to content

Commit 2ec7834

Browse files
committed
Resolve nested placeholders via PropertyResolver
Prior to this change, PropertySourcesPropertyResolver (and therefore all AbstractEnvironment) implementations failed to resolve nested placeholders as in the following example: p1=v1 p2=v2 p3=${v1}:{$v2} Calls to PropertySource#getProperty for keys 'p1' and 'v1' would successfully return their respective values, but for 'p3' the return value would be the unresolved placeholders. This behavior is inconsistent with that of PropertyPlaceholderConfigurer. PropertySourcesPropertyResolver #getProperty variants now resolve any nested placeholders recursively, throwing IllegalArgumentException for any unresolvable placeholders (as is the default behavior for PropertyPlaceholderConfigurer). See SPR-9569 for an enhancement that will intoduce an 'ignoreUnresolvablePlaceholders' switch to make this behavior configurable. This commit also improves error output in PropertyPlaceholderHelper#parseStringValue by including the original string in which an unresolvable placeholder was found. Issue: SPR-9473, SPR-9569
1 parent b9786cc commit 2ec7834

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ public <T> T getProperty(String key, Class<T> targetValueType) {
7272
Object value;
7373
if ((value = propertySource.getProperty(key)) != null) {
7474
Class<?> valueType = value.getClass();
75+
if (String.class.equals(valueType)) {
76+
value = this.resolveRequiredPlaceholders((String) value);
77+
}
7578
if (debugEnabled) {
7679
logger.debug(
7780
format("Found key '%s' in [%s] with type [%s] and value '%s'",

spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ else if (this.ignoreUnresolvablePlaceholders) {
171171
startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
172172
}
173173
else {
174-
throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'");
174+
throw new IllegalArgumentException("Could not resolve placeholder '" +
175+
placeholder + "'" + " in string value [" + strVal + "]");
175176
}
176177

177178
visitedPlaceholders.remove(originalPlaceholder);

spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,22 +16,20 @@
1616

1717
package org.springframework.core.env;
1818

19-
import static org.hamcrest.CoreMatchers.equalTo;
20-
import static org.hamcrest.CoreMatchers.is;
21-
import static org.hamcrest.CoreMatchers.nullValue;
22-
import static org.junit.Assert.assertThat;
23-
import static org.junit.Assert.assertTrue;
24-
import static org.junit.Assert.fail;
25-
2619
import java.util.HashMap;
2720
import java.util.Map;
2821
import java.util.Properties;
2922

23+
import org.hamcrest.Matchers;
3024
import org.junit.Before;
3125
import org.junit.Test;
26+
3227
import org.springframework.core.convert.ConversionException;
3328
import org.springframework.mock.env.MockPropertySource;
3429

30+
import static org.hamcrest.CoreMatchers.*;
31+
import static org.junit.Assert.*;
32+
3533
/**
3634
* Unit tests for {@link PropertySourcesPropertyResolver}.
3735
*
@@ -352,6 +350,39 @@ public void setRequiredProperties_andValidateRequiredProperties() {
352350
propertyResolver.validateRequiredProperties();
353351
}
354352

353+
@Test
354+
public void resolveNestedPropertyPlaceholders() {
355+
MutablePropertySources ps = new MutablePropertySources();
356+
ps.addFirst(new MockPropertySource()
357+
.withProperty("p1", "v1")
358+
.withProperty("p2", "v2")
359+
.withProperty("p3", "${p1}:${p2}") // nested placeholders
360+
.withProperty("p4", "${p3}") // deeply nested placeholders
361+
.withProperty("p5", "${p1}:${p2}:${bogus}") // unresolvable placeholder
362+
.withProperty("p6", "${p1}:${p2}:${bogus:def}") // unresolvable w/ default
363+
.withProperty("pL", "${pR}") // cyclic reference left
364+
.withProperty("pR", "${pL}") // cyclic reference right
365+
);
366+
PropertySourcesPropertyResolver pr = new PropertySourcesPropertyResolver(ps);
367+
assertThat(pr.getProperty("p1"), equalTo("v1"));
368+
assertThat(pr.getProperty("p2"), equalTo("v2"));
369+
assertThat(pr.getProperty("p3"), equalTo("v1:v2"));
370+
assertThat(pr.getProperty("p4"), equalTo("v1:v2"));
371+
try {
372+
pr.getProperty("p5");
373+
} catch (IllegalArgumentException ex) {
374+
assertThat(ex.getMessage(), Matchers.containsString(
375+
"Could not resolve placeholder 'bogus' in string value [${p1}:${p2}:${bogus}]"));
376+
}
377+
assertThat(pr.getProperty("p6"), equalTo("v1:v2:def"));
378+
try {
379+
pr.getProperty("pL");
380+
} catch (StackOverflowError ex) {
381+
// no explicit handling for cyclic references for now
382+
}
383+
}
384+
385+
355386
static interface SomeType { }
356387
static class SpecificType implements SomeType { }
357388
}

0 commit comments

Comments
 (0)