Skip to content

Commit 38f1383

Browse files
author
David Syer
committed
RESOLVED - issue SPR-6321: Regression: ResourceEditor in 3.0 does not ignore unresolvable placeholders, but it did in 2.5.6
1 parent 7685b51 commit 38f1383

File tree

4 files changed

+124
-28
lines changed

4 files changed

+124
-28
lines changed

‎org.springframework.core/src/main/java/org/springframework/core/io/ResourceEditor.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,16 @@
3131
* properties instead of using a <code>String</code> location property.
3232
*
3333
* <p>The path may contain <code>${...}</code> placeholders, to be resolved
34-
* as system properties: e.g. <code>${user.dir}</code>.
34+
* as system properties: e.g. <code>${user.dir}</code>. By default unresolvable
35+
* placeholders are ignored, but if an exception is preferred set the
36+
* {@link #setIgnoreUnresolvablePlaceholders(boolean) ignoreUnresolvablePlaceholders}
37+
* flag to false.
3538
*
3639
* <p>Delegates to a {@link ResourceLoader} to do the heavy lifting,
3740
* by default using a {@link DefaultResourceLoader}.
3841
*
3942
* @author Juergen Hoeller
43+
* @author Dave Syer
4044
* @since 28.12.2003
4145
* @see Resource
4246
* @see ResourceLoader
@@ -47,7 +51,8 @@
4751
public class ResourceEditor extends PropertyEditorSupport {
4852

4953
private final ResourceLoader resourceLoader;
50-
54+
55+
private boolean ignoreUnresolvablePlaceholders = true;
5156

5257
/**
5358
* Create a new instance of the {@link ResourceEditor} class
@@ -66,7 +71,14 @@ public ResourceEditor(ResourceLoader resourceLoader) {
6671
Assert.notNull(resourceLoader, "ResourceLoader must not be null");
6772
this.resourceLoader = resourceLoader;
6873
}
69-
74+
75+
/**
76+
* Flag to determine if unresolvable placeholders in System properties
77+
* @param ignoreUnresolvablePlaceholders
78+
*/
79+
public void setIgnoreUnresolvablePlaceholders(boolean ignoreUnresolvablePlaceholders) {
80+
this.ignoreUnresolvablePlaceholders = ignoreUnresolvablePlaceholders;
81+
}
7082

7183
@Override
7284
public void setAsText(String text) {
@@ -87,7 +99,7 @@ public void setAsText(String text) {
8799
* @see org.springframework.util.SystemPropertyUtils#resolvePlaceholders
88100
*/
89101
protected String resolvePath(String path) {
90-
return SystemPropertyUtils.resolvePlaceholders(path);
102+
return SystemPropertyUtils.resolvePlaceholders(path, ignoreUnresolvablePlaceholders);
91103
}
92104

93105

‎org.springframework.core/src/main/java/org/springframework/util/SystemPropertyUtils.java

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
* Helper class for resolving placeholders in texts. Usually applied to file paths.
2323
*
2424
* <p>A text may contain <code>${...}</code> placeholders, to be resolved as system properties: e.g.
25-
* <code>${user.dir}</code>.
25+
* <code>${user.dir}</code>. Default values can be supplied using the ":" separator between key
26+
* and value.
2627
*
2728
* @author Juergen Hoeller
29+
* @author Dave Syer
2830
* @see #PLACEHOLDER_PREFIX
2931
* @see #PLACEHOLDER_SUFFIX
3032
* @see System#getProperty(String)
@@ -38,44 +40,76 @@ public abstract class SystemPropertyUtils {
3840
/** Suffix for system property placeholders: "}" */
3941
public static final String PLACEHOLDER_SUFFIX = "}";
4042

41-
/** Value separator for system property placeholders: "}" */
43+
/** Value separator for system property placeholders: ":" */
4244
public static final String VALUE_SEPARATOR = ":";
4345

44-
private static final PropertyPlaceholderHelper helper =
45-
new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, false);
46+
private static final PropertyPlaceholderHelper strictHelper = new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX,
47+
PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, false);
4648

49+
private static final PropertyPlaceholderHelper nonStrictHelper = new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX,
50+
PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, true);
4751

4852
/**
4953
* Resolve ${...} placeholders in the given text, replacing them with corresponding system property values.
5054
* @param text the String to resolve
5155
* @return the resolved String
5256
* @see #PLACEHOLDER_PREFIX
5357
* @see #PLACEHOLDER_SUFFIX
58+
*
59+
* @throws IllegalArgumentException if there is an unresolvable placeholder
5460
*/
5561
public static String resolvePlaceholders(final String text) {
56-
return helper.replacePlaceholders(text, new PlaceholderResolver() {
57-
public String resolvePlaceholder(String placeholderName) {
58-
String propVal = null;
59-
try {
60-
propVal = System.getProperty(placeholderName);
61-
if (propVal == null) {
62-
// Fall back to searching the system environment.
63-
propVal = System.getenv(placeholderName);
64-
}
65-
66-
if (propVal == null) {
67-
System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text +
68-
"] as system property: neither system property nor environment variable found");
69-
}
62+
return resolvePlaceholders(text, false);
63+
}
64+
65+
/**
66+
* Resolve ${...} placeholders in the given text, replacing them with corresponding system property values.
67+
* Unresolvable placeholders with no default value are ignored and passed through unchanged if the
68+
* flag is set to true.
69+
*
70+
* @param text the String to resolve
71+
* @param ignoreUnresolvablePlaceholders flag to determine is unresolved placeholders are ignored
72+
* @return the resolved String
73+
* @see #PLACEHOLDER_PREFIX
74+
* @see #PLACEHOLDER_SUFFIX
75+
*
76+
* @throws IllegalArgumentException if there is an unresolvable placeholder and the flag is false
77+
*
78+
*/
79+
public static String resolvePlaceholders(final String text, boolean ignoreUnresolvablePlaceholders) {
80+
if (ignoreUnresolvablePlaceholders) {
81+
return nonStrictHelper.replacePlaceholders(text, new PlaceholderResolverImplementation(text));
82+
}
83+
return strictHelper.replacePlaceholders(text, new PlaceholderResolverImplementation(text));
84+
}
85+
86+
private static final class PlaceholderResolverImplementation implements PlaceholderResolver {
87+
private final String text;
88+
89+
private PlaceholderResolverImplementation(String text) {
90+
this.text = text;
91+
}
92+
93+
public String resolvePlaceholder(String placeholderName) {
94+
String propVal = null;
95+
try {
96+
propVal = System.getProperty(placeholderName);
97+
if (propVal == null) {
98+
// Fall back to searching the system environment.
99+
propVal = System.getenv(placeholderName);
70100
}
71-
catch (Throwable ex) {
72-
System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text +
73-
"] as system property: " + ex);
74101

102+
if (propVal == null) {
103+
System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text
104+
+ "] as system property: neither system property nor environment variable found");
75105
}
76-
return propVal;
106+
} catch (Throwable ex) {
107+
System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text
108+
+ "] as system property: " + ex);
109+
77110
}
78-
});
111+
return propVal;
112+
}
79113
}
80114

81115
}

‎org.springframework.core/src/test/java/org/springframework/core/io/ResourceEditorTests.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package org.springframework.core.io;
1818

19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNotNull;
21+
import static org.junit.Assert.assertTrue;
22+
1923
import java.beans.PropertyEditor;
2024

21-
import static org.junit.Assert.*;
2225
import org.junit.Test;
2326

2427
/**
@@ -57,4 +60,33 @@ public void setAndGetAsTextWithWhitespaceResource() throws Exception {
5760
assertEquals("", editor.getAsText());
5861
}
5962

63+
@Test
64+
public void testSystemPropertyReplacement() {
65+
PropertyEditor editor = new ResourceEditor();
66+
System.setProperty("test.prop", "foo");
67+
try {
68+
editor.setAsText("${test.prop}-${bar}");
69+
Resource resolved = (Resource) editor.getValue();
70+
assertEquals("foo-${bar}", resolved.getFilename());
71+
}
72+
finally {
73+
System.getProperties().remove("test.prop");
74+
}
75+
}
76+
77+
@Test(expected=IllegalArgumentException.class)
78+
public void testStrictSystemPropertyReplacement() {
79+
ResourceEditor editor = new ResourceEditor();
80+
editor.setIgnoreUnresolvablePlaceholders(false);
81+
System.setProperty("test.prop", "foo");
82+
try {
83+
editor.setAsText("${test.prop}-${bar}");
84+
Resource resolved = (Resource) editor.getValue();
85+
assertEquals("foo-${bar}", resolved.getFilename());
86+
}
87+
finally {
88+
System.getProperties().remove("test.prop");
89+
}
90+
}
91+
6092
}

‎org.springframework.core/src/test/java/org/springframework/util/SystemPropertyUtilsTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@ public void testReplaceWithDefault() {
4545
assertEquals("foo", resolved);
4646
}
4747

48+
@Test(expected=IllegalArgumentException.class)
49+
public void testReplaceWithNoDefault() {
50+
String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop}");
51+
assertEquals("", resolved);
52+
}
53+
54+
@Test
55+
public void testReplaceWithNoDefaultIgnored() {
56+
String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop}", true);
57+
assertEquals("${test.prop}", resolved);
58+
}
59+
60+
@Test
61+
public void testReplaceWithEmptyDefault() {
62+
String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop:}");
63+
assertEquals("", resolved);
64+
}
65+
4866
@Test
4967
public void testRecursiveFromSystemProperty() {
5068
System.setProperty("test.prop", "foo=${bar}");

0 commit comments

Comments
 (0)