Unit test for circular reference in default profile property, plus related polishing

Issue: SPR-12078

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -181,8 +181,8 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
 	 * @see #setIgnoreUnresolvableNestedPlaceholders
 	 */
 	protected String resolveNestedPlaceholders(String value) {
-		return this.ignoreUnresolvableNestedPlaceholders ?
+		return (this.ignoreUnresolvableNestedPlaceholders ?
-				resolvePlaceholders(value) : resolveRequiredPlaceholders(value);
+				resolvePlaceholders(value) : resolveRequiredPlaceholders(value));
 	}
 
 	private PropertyPlaceholderHelper createPlaceholderHelper(boolean ignoreUnresolvablePlaceholders) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -62,8 +62,8 @@ public class PropertyPlaceholderHelper {
 	/**
 	 * Creates a new {@code PropertyPlaceholderHelper} that uses the supplied prefix and suffix.
 	 * Unresolvable placeholders are ignored.
-	 * @param placeholderPrefix the prefix that denotes the start of a placeholder.
+	 * @param placeholderPrefix the prefix that denotes the start of a placeholder
-	 * @param placeholderSuffix the suffix that denotes the end of a placeholder.
+	 * @param placeholderSuffix the suffix that denotes the end of a placeholder
 	 */
 	public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix) {
 		this(placeholderPrefix, placeholderSuffix, null, true);
@@ -75,14 +75,14 @@ public class PropertyPlaceholderHelper {
 	 * @param placeholderSuffix the suffix that denotes the end of a placeholder
 	 * @param valueSeparator the separating character between the placeholder variable
 	 * and the associated default value, if any
-	 * @param ignoreUnresolvablePlaceholders indicates whether unresolvable placeholders should be ignored
+	 * @param ignoreUnresolvablePlaceholders indicates whether unresolvable placeholders should
-	 * ({@code true}) or cause an exception ({@code false}).
+	 * be ignored ({@code true}) or cause an exception ({@code false})
 	 */
 	public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix,
 			String valueSeparator, boolean ignoreUnresolvablePlaceholders) {
 
-		Assert.notNull(placeholderPrefix, "placeholderPrefix must not be null");
+		Assert.notNull(placeholderPrefix, "'placeholderPrefix' must not be null");
-		Assert.notNull(placeholderSuffix, "placeholderSuffix must not be null");
+		Assert.notNull(placeholderSuffix, "'placeholderSuffix' must not be null");
 		this.placeholderPrefix = placeholderPrefix;
 		this.placeholderSuffix = placeholderSuffix;
 		String simplePrefixForSuffix = wellKnownSimplePrefixes.get(this.placeholderSuffix);
@@ -100,12 +100,12 @@ public class PropertyPlaceholderHelper {
 	/**
 	 * Replaces all placeholders of format {@code ${name}} with the corresponding
 	 * property from the supplied {@link Properties}.
-	 * @param value the value containing the placeholders to be replaced.
+	 * @param value the value containing the placeholders to be replaced
-	 * @param properties the {@code Properties} to use for replacement.
+	 * @param properties the {@code Properties} to use for replacement
-	 * @return the supplied value with placeholders replaced inline.
+	 * @return the supplied value with placeholders replaced inline
 	 */
 	public String replacePlaceholders(String value, final Properties properties) {
-		Assert.notNull(properties, "Argument 'properties' must not be null.");
+		Assert.notNull(properties, "'properties' must not be null");
 		return replacePlaceholders(value, new PlaceholderResolver() {
 			@Override
 			public String resolvePlaceholder(String placeholderName) {
@@ -117,25 +117,25 @@ public class PropertyPlaceholderHelper {
 	/**
 	 * Replaces all placeholders of format {@code ${name}} with the value returned
 	 * from the supplied {@link PlaceholderResolver}.
-	 * @param value the value containing the placeholders to be replaced.
+	 * @param value the value containing the placeholders to be replaced
-	 * @param placeholderResolver the {@code PlaceholderResolver} to use for replacement.
+	 * @param placeholderResolver the {@code PlaceholderResolver} to use for replacement
-	 * @return the supplied value with placeholders replaced inline.
+	 * @return the supplied value with placeholders replaced inline
 	 */
 	public String replacePlaceholders(String value, PlaceholderResolver placeholderResolver) {
-		Assert.notNull(value, "Argument 'value' must not be null.");
+		Assert.notNull(value, "'value' must not be null");
 		return parseStringValue(value, placeholderResolver, new HashSet<String>());
 	}
 
 	protected String parseStringValue(
 			String strVal, PlaceholderResolver placeholderResolver, Set<String> visitedPlaceholders) {
 
-		StringBuilder buf = new StringBuilder(strVal);
+		StringBuilder result = new StringBuilder(strVal);
 
 		int startIndex = strVal.indexOf(this.placeholderPrefix);
 		while (startIndex != -1) {
-			int endIndex = findPlaceholderEndIndex(buf, startIndex);
+			int endIndex = findPlaceholderEndIndex(result, startIndex);
 			if (endIndex != -1) {
-				String placeholder = buf.substring(startIndex + this.placeholderPrefix.length(), endIndex);
+				String placeholder = result.substring(startIndex + this.placeholderPrefix.length(), endIndex);
 				String originalPlaceholder = placeholder;
 				if (!visitedPlaceholders.add(originalPlaceholder)) {
 					throw new IllegalArgumentException(
@@ -160,15 +160,15 @@ public class PropertyPlaceholderHelper {
 					// Recursive invocation, parsing placeholders contained in the
 					// previously resolved placeholder value.
 					propVal = parseStringValue(propVal, placeholderResolver, visitedPlaceholders);
-					buf.replace(startIndex, endIndex + this.placeholderSuffix.length(), propVal);
+					result.replace(startIndex, endIndex + this.placeholderSuffix.length(), propVal);
 					if (logger.isTraceEnabled()) {
 						logger.trace("Resolved placeholder '" + placeholder + "'");
 					}
-					startIndex = buf.indexOf(this.placeholderPrefix, startIndex + propVal.length());
+					startIndex = result.indexOf(this.placeholderPrefix, startIndex + propVal.length());
 				}
 				else if (this.ignoreUnresolvablePlaceholders) {
 					// Proceed with unprocessed value.
-					startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
+					startIndex = result.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
 				}
 				else {
 					throw new IllegalArgumentException("Could not resolve placeholder '" +
@@ -181,7 +181,7 @@ public class PropertyPlaceholderHelper {
 			}
 		}
 
-		return buf.toString();
+		return result.toString();
 	}
 
 	private int findPlaceholderEndIndex(CharSequence buf, int startIndex) {
@@ -211,14 +211,13 @@ public class PropertyPlaceholderHelper {
 
 	/**
 	 * Strategy interface used to resolve replacement values for placeholders contained in Strings.
-	 * @see PropertyPlaceholderHelper
 	 */
 	public static interface PlaceholderResolver {
 
 		/**
-		 * Resolves the supplied placeholder name into the replacement value.
+		 * Resolve the supplied placeholder name to the replacement value.
 		 * @param placeholderName the name of the placeholder to resolve
-		 * @return the replacement value or {@code null} if no replacement is to be made
+		 * @return the replacement value, or {@code null} if no replacement is to be made
 		 */
 		String resolvePlaceholder(String placeholderName);
 	}

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

@@ -204,6 +204,17 @@ public class StandardEnvironmentTests {
 		System.getProperties().remove(DEFAULT_PROFILES_PROPERTY_NAME);
 	}
 
+	@Test(expected=IllegalArgumentException.class)
+	public void defaultProfileWithCircularPlaceholder() {
+		System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "${spring.profiles.default}");
+		try {
+			environment.getDefaultProfiles();
+		}
+		finally {
+			System.getProperties().remove(DEFAULT_PROFILES_PROPERTY_NAME);
+		}
+	}
+
 	@Test
 	public void getActiveProfiles_systemPropertiesEmpty() {
 		assertThat(environment.getActiveProfiles().length, is(0));