Skip to content

Commit 00a6939

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 45f1438 commit 00a6939

File tree

5 files changed

+49
-12
lines changed

5 files changed

+49
-12
lines changed

org.springframework.core/.classpath

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
<classpathentry kind="var" path="IVY_CACHE/org.aspectj/com.springsource.org.aspectj.weaver/1.6.8.RELEASE/com.springsource.org.aspectj.weaver-1.6.8.RELEASE.jar"/>
1212
<classpathentry kind="var" path="IVY_CACHE/org.custommonkey.xmlunit/com.springsource.org.custommonkey.xmlunit/1.2.0/com.springsource.org.custommonkey.xmlunit-1.2.0.jar" sourcepath="/IVY_CACHE/org.custommonkey.xmlunit/com.springsource.org.custommonkey.xmlunit/1.2.0/com.springsource.org.custommonkey.xmlunit-sources-1.2.0.jar"/>
1313
<classpathentry kind="var" path="IVY_CACHE/org.easymock/com.springsource.org.easymock/2.5.1/com.springsource.org.easymock-2.5.1.jar" sourcepath="/IVY_CACHE/org.easymock/com.springsource.org.easymock/2.5.1/com.springsource.org.easymock-sources-2.5.1.jar"/>
14+
<classpathentry kind="var" path="IVY_CACHE/org.hamcrest/com.springsource.org.hamcrest/1.1.0/com.springsource.org.hamcrest-1.1.0.jar" sourcepath="IVY_CACHE/org.hamcrest/com.springsource.org.hamcrest/1.1.0/com.springsource.org.hamcrest-sources-1.1.0.jar"/>
1415
<classpathentry kind="var" path="IVY_CACHE/org.codehaus.woodstox/com.springsource.com.ctc.wstx/3.2.7/com.springsource.com.ctc.wstx-3.2.7.jar" sourcepath="/IVY_CACHE/org.codehaus.woodstox/com.springsource.com.ctc.wstx/3.2.7/com.springsource.com.ctc.wstx-sources-3.2.7.jar"/>
1516
<classpathentry kind="var" path="IVY_CACHE/net.sourceforge.jopt-simple/com.springsource.joptsimple/3.0.0/com.springsource.joptsimple-3.0.0.jar" sourcepath="IVY_CACHE/net.sourceforge.jopt-simple/com.springsource.joptsimple/3.0.0/com.springsource.joptsimple-sources-3.0.0.jar"/>
1617
<classpathentry kind="lib" path="/org.springframework.asm/target/artifacts/org.springframework.asm.jar" sourcepath="/org.springframework.asm/target/artifacts/org.springframework.asm-sources.jar"/>

org.springframework.core/ivy.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
<dependency org="net.sourceforge.jopt-simple" name="com.springsource.joptsimple" rev="3.0.0" conf="optional->compile"/>
3333
<!-- test dependencies -->
3434
<dependency org="org.junit" name="com.springsource.org.junit" rev="${junit.version}" conf="test->runtime"/>
35+
<dependency org="org.hamcrest" name="com.springsource.org.hamcrest" rev="1.1.0" conf="test->compile"/>
3536
<dependency org="org.easymock" name="com.springsource.org.easymock" rev="2.5.1" conf="test->compile"/>
3637
<dependency org="org.custommonkey.xmlunit" name="com.springsource.org.custommonkey.xmlunit" rev="1.2.0" conf="test->compile"/>
3738
<dependency org="org.codehaus.woodstox" name="com.springsource.com.ctc.wstx" rev="3.2.7" conf="test->compile"/>

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
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.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* http://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -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'",

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2010 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.
@@ -170,7 +170,8 @@ else if (this.ignoreUnresolvablePlaceholders) {
170170
startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
171171
}
172172
else {
173-
throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'");
173+
throw new IllegalArgumentException("Could not resolve placeholder '" +
174+
placeholder + "'" + " in string value [" + strVal + "]");
174175
}
175176

176177
visitedPlaceholders.remove(placeholder);

org.springframework.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)