RESOLVED - issue SPR-6321: Regression: ResourceEditor in 3.0 does not ignore unresolvable placeholders, but it did in 2.5.6

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

@@ -31,12 +31,16 @@ import org.springframework.util.SystemPropertyUtils;
  * properties instead of using a <code>String</code> location property.
  *
  * <p>The path may contain <code>${...}</code> placeholders, to be resolved
- * as system properties: e.g. <code>${user.dir}</code>.
+ * as system properties: e.g. <code>${user.dir}</code>.  By default unresolvable
+ * placeholders are ignored, but if an exception is preferred set the 
+ * {@link #setIgnoreUnresolvablePlaceholders(boolean) ignoreUnresolvablePlaceholders} 
+ * flag to false.
  *
  * <p>Delegates to a {@link ResourceLoader} to do the heavy lifting,
  * by default using a {@link DefaultResourceLoader}.
  *
  * @author Juergen Hoeller
+ * @author Dave Syer
  * @since 28.12.2003
  * @see Resource
  * @see ResourceLoader
@@ -47,7 +51,8 @@ import org.springframework.util.SystemPropertyUtils;
 public class ResourceEditor extends PropertyEditorSupport {
 
 	private final ResourceLoader resourceLoader;
-
+	
+	private boolean ignoreUnresolvablePlaceholders = true;
 
 	/**
 	 * Create a new instance of the {@link ResourceEditor} class
@@ -66,7 +71,14 @@ public class ResourceEditor extends PropertyEditorSupport {
 		Assert.notNull(resourceLoader, "ResourceLoader must not be null");
 		this.resourceLoader = resourceLoader;
 	}
-
+	
+	/**
+	 * Flag to determine if unresolvable placeholders in System properties
+	 * @param ignoreUnresolvablePlaceholders
+	 */
+	public void setIgnoreUnresolvablePlaceholders(boolean ignoreUnresolvablePlaceholders) {
+		this.ignoreUnresolvablePlaceholders = ignoreUnresolvablePlaceholders;
+	}
 
 	@Override
 	public void setAsText(String text) {
@@ -87,7 +99,7 @@ public class ResourceEditor extends PropertyEditorSupport {
 	 * @see org.springframework.util.SystemPropertyUtils#resolvePlaceholders
 	 */
 	protected String resolvePath(String path) {
-		return SystemPropertyUtils.resolvePlaceholders(path);
+		return SystemPropertyUtils.resolvePlaceholders(path, ignoreUnresolvablePlaceholders);
 	}
 
 

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

@@ -22,9 +22,11 @@ import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver;
  * Helper class for resolving placeholders in texts. Usually applied to file paths.
  *
  * <p>A text may contain <code>${...}</code> placeholders, to be resolved as system properties: e.g.
- * <code>${user.dir}</code>.
+ * <code>${user.dir}</code>.  Default values can be supplied using the ":" separator between key 
+ * and value.
  *
  * @author Juergen Hoeller
+ * @author Dave Syer
  * @see #PLACEHOLDER_PREFIX
  * @see #PLACEHOLDER_SUFFIX
  * @see System#getProperty(String)
@@ -38,12 +40,14 @@ public abstract class SystemPropertyUtils {
 	/** Suffix for system property placeholders: "}" */
 	public static final String PLACEHOLDER_SUFFIX = "}";
 
-	/** Value separator for system property placeholders: "}" */
+	/** Value separator for system property placeholders: ":" */
 	public static final String VALUE_SEPARATOR = ":";
 
-	private static final PropertyPlaceholderHelper helper =
+	private static final PropertyPlaceholderHelper strictHelper = new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX,
-			new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, false);
+			PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, false);
 
+	private static final PropertyPlaceholderHelper nonStrictHelper = new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX,
+			PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, true);
 
 	/**
 	 * Resolve ${...} placeholders in the given text, replacing them with corresponding system property values.
@@ -51,31 +55,61 @@ public abstract class SystemPropertyUtils {
 	 * @return the resolved String
 	 * @see #PLACEHOLDER_PREFIX
 	 * @see #PLACEHOLDER_SUFFIX
+	 *
+	 * @throws IllegalArgumentException if there is an unresolvable placeholder
 	 */
 	public static String resolvePlaceholders(final String text) {
-		return helper.replacePlaceholders(text, new PlaceholderResolver() {
+		return resolvePlaceholders(text, false);
-			public String resolvePlaceholder(String placeholderName) {
+	}
-				String propVal = null;
-				try {
-					propVal = System.getProperty(placeholderName);
-					if (propVal == null) {
-						// Fall back to searching the system environment.
-						propVal = System.getenv(placeholderName);
-					}
 
-					if (propVal == null) {
+	/**
-						System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text +
+	 * Resolve ${...} placeholders in the given text, replacing them with corresponding system property values.
-								"] as system property: neither system property nor environment variable found");
+	 * Unresolvable placeholders with no default value are ignored and passed through unchanged if the
-					}
+	 * flag is set to true.
-				}
+	 * 
-				catch (Throwable ex) {
+	 * @param text the String to resolve
-					System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text +
+	 * @param ignoreUnresolvablePlaceholders flag to determine is unresolved placeholders are ignored
-							"] as system property: " + ex);
+	 * @return the resolved String
+	 * @see #PLACEHOLDER_PREFIX
+	 * @see #PLACEHOLDER_SUFFIX
+	 * 
+	 * @throws IllegalArgumentException if there is an unresolvable placeholder and the flag is false
+	 * 
+	 */
+	public static String resolvePlaceholders(final String text, boolean ignoreUnresolvablePlaceholders) {
+		if (ignoreUnresolvablePlaceholders) {
+			return nonStrictHelper.replacePlaceholders(text, new PlaceholderResolverImplementation(text));
+		}
+		return strictHelper.replacePlaceholders(text, new PlaceholderResolverImplementation(text));
+	}
 
+	private static final class PlaceholderResolverImplementation implements PlaceholderResolver {
+		private final String text;
+
+		private PlaceholderResolverImplementation(String text) {
+			this.text = text;
+		}
+
+		public String resolvePlaceholder(String placeholderName) {
+			String propVal = null;
+			try {
+				propVal = System.getProperty(placeholderName);
+				if (propVal == null) {
+					// Fall back to searching the system environment.
+					propVal = System.getenv(placeholderName);
 				}
-				return propVal;
+
+				if (propVal == null) {
+					System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text
+							+ "] as system property: neither system property nor environment variable found");
+				}
+			} catch (Throwable ex) {
+				System.err.println("Could not resolve placeholder '" + placeholderName + "' in [" + text
+						+ "] as system property: " + ex);
+
 			}
-		});
+			return propVal;
+		}
 	}
 
 }

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

@@ -16,9 +16,12 @@
 
 package org.springframework.core.io;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import java.beans.PropertyEditor;
 
-import static org.junit.Assert.*;
 import org.junit.Test;
 
 /**
@@ -57,4 +60,33 @@ public final class ResourceEditorTests {
 		assertEquals("", editor.getAsText());
 	}
 
+	@Test
+	public void testSystemPropertyReplacement() {
+		PropertyEditor editor = new ResourceEditor();
+		System.setProperty("test.prop", "foo");
+		try {
+			editor.setAsText("${test.prop}-${bar}");
+			Resource resolved = (Resource) editor.getValue();
+			assertEquals("foo-${bar}", resolved.getFilename());
+		}
+		finally {
+			System.getProperties().remove("test.prop");
+		}
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void testStrictSystemPropertyReplacement() {
+		ResourceEditor editor = new ResourceEditor();
+		editor.setIgnoreUnresolvablePlaceholders(false);
+		System.setProperty("test.prop", "foo");
+		try {
+			editor.setAsText("${test.prop}-${bar}");
+			Resource resolved = (Resource) editor.getValue();
+			assertEquals("foo-${bar}", resolved.getFilename());
+		}
+		finally {
+			System.getProperties().remove("test.prop");
+		}
+	}
+
 }

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

@@ -45,6 +45,24 @@ public class SystemPropertyUtilsTests {
 		assertEquals("foo", resolved);
 	}
 
+	@Test(expected=IllegalArgumentException.class)
+	public void testReplaceWithNoDefault() {
+		String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop}");
+		assertEquals("", resolved);
+	}
+
+	@Test
+	public void testReplaceWithNoDefaultIgnored() {
+		String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop}", true);
+		assertEquals("${test.prop}", resolved);
+	}
+
+	@Test
+	public void testReplaceWithEmptyDefault() {
+		String resolved = SystemPropertyUtils.resolvePlaceholders("${test.prop:}");
+		assertEquals("", resolved);
+	}
+
 	@Test
 	public void testRecursiveFromSystemProperty() {
 		System.setProperty("test.prop", "foo=${bar}");