From 8dbfa80b132d583fe7a26555f9a2842ced6a50cf Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 12 Aug 2014 19:23:36 +0200 Subject: [PATCH] Unit test for circular reference in default profile property, plus related polishing Issue: SPR-12078 --- .../core/env/AbstractPropertyResolver.java | 6 +-- .../util/PropertyPlaceholderHelper.java | 49 +++++++++---------- .../core/env/StandardEnvironmentTests.java | 11 +++++ 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/env/AbstractPropertyResolver.java b/spring-core/src/main/java/org/springframework/core/env/AbstractPropertyResolver.java index d8bbd20d96..2d1bcf022b 100644 --- a/spring-core/src/main/java/org/springframework/core/env/AbstractPropertyResolver.java +++ b/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 ? - resolvePlaceholders(value) : resolveRequiredPlaceholders(value); + return (this.ignoreUnresolvableNestedPlaceholders ? + resolvePlaceholders(value) : resolveRequiredPlaceholders(value)); } private PropertyPlaceholderHelper createPlaceholderHelper(boolean ignoreUnresolvablePlaceholders) { diff --git a/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java b/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java index 65b600ac15..b5c749fbba 100644 --- a/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java +++ b/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 placeholderSuffix the suffix that denotes the end 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 */ 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 - * ({@code true}) or cause an exception ({@code false}). + * @param ignoreUnresolvablePlaceholders indicates whether unresolvable placeholders should + * 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(placeholderSuffix, "placeholderSuffix must not be null"); + Assert.notNull(placeholderPrefix, "'placeholderPrefix' 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 properties the {@code Properties} to use for replacement. - * @return the supplied value with placeholders replaced inline. + * @param value the value containing the placeholders to be replaced + * @param properties the {@code Properties} to use for replacement + * @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 placeholderResolver the {@code PlaceholderResolver} to use for replacement. - * @return the supplied value with placeholders replaced inline. + * @param value the value containing the placeholders to be replaced + * @param placeholderResolver the {@code PlaceholderResolver} to use for replacement + * @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()); } protected String parseStringValue( String strVal, PlaceholderResolver placeholderResolver, Set 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); } diff --git a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java b/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java index 01daf449dc..e094e342d2 100644 --- a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java +++ b/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));