From 00e05e603d4423d33c99dadeb52fef26be71dfb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Fri, 29 Dec 2023 17:40:25 +0100 Subject: [PATCH 1/2] Add new property placeholder implementation This commit provides a rewrite of the parser for properties containing potentially placeholders. Assuming a source where `firstName` = `John` and `lastName` = `Smith`, the "${firstName}-${lastName}" property is evaluated as "John-Smith". Compared with the existing implementation in PropertyPlaceholderHelper, the new implementation offers the following extra features: 1. Placeholder can be escaped using a configurable escape character. When a placeholder is escaped it is rendered as is. This does apply to any nested placeholder that wouldn't be escaped. For instance, "\${firstName}" is evaluated as "${firstName}". 2. The default separator can also be escaped the same way. When the separator is escaped, the left and right parts are not considered as the key and the default value respectively. Rather the two parts combined, including the separator (but not the escape separator) are used for resolution. For instance, ${java\:comp/env/test} is looking for a "java:comp/env/test" property. 3. Placeholders are resolved lazily. Previously, all nested placeholders were resolved before considering if a separator was present. This implementation only attempts the resolution of the default value if the key does not provide a value. 4. Failure to resolve a placeholder are more rich, with a dedicated PlaceholderResolutionException that contains the resolution chain. See gh-9628 See gh-26268 --- .../util/PlaceholderParser.java | 503 ++++++++++++++++++ .../util/PlaceholderResolutionException.java | 100 ++++ .../util/PlaceholderParserTests.java | 355 ++++++++++++ 3 files changed, 958 insertions(+) create mode 100644 spring-core/src/main/java/org/springframework/util/PlaceholderParser.java create mode 100644 spring-core/src/main/java/org/springframework/util/PlaceholderResolutionException.java create mode 100644 spring-core/src/test/java/org/springframework/util/PlaceholderParserTests.java diff --git a/spring-core/src/main/java/org/springframework/util/PlaceholderParser.java b/spring-core/src/main/java/org/springframework/util/PlaceholderParser.java new file mode 100644 index 0000000000..6836d10d12 --- /dev/null +++ b/spring-core/src/main/java/org/springframework/util/PlaceholderParser.java @@ -0,0 +1,503 @@ +/* + * Copyright 2002-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.util; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.lang.Nullable; +import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver; + +/** + * Parser for Strings that have placeholder values in them. In its simplest form, + * a placeholder takes the form of {@code ${name}}, where {@code name} is the key + * that can be resolved using a {@link PlaceholderResolver PlaceholderResolver}, + * ${ the prefix, and } the suffix. + * + *

A placeholder can also have a default value if its key does not represent a + * known property. The default value is separated from the key using a + * {@code separator}. For instance {@code ${name:John}} resolves to {@code John} if + * the placeholder resolver does not provide a value for the {@code name} + * property. + * + *

Placeholders can also have a more complex structure, and the resolution of + * a given key can involve the resolution of nested placeholders. Default values + * can also have placeholders. + * + *

For situations where the syntax of a valid placeholder match a String that + * must be rendered as is, the placeholder can be escaped using an {@code escape} + * character. For instance {@code \${name}} resolves as {@code ${name}}. + * + *

The prefix, suffix, separator, and escape characters are configurable. Only + * the prefix and suffix are mandatory and the support of default values or + * escaping are conditional on providing a non-null value for them. + * + *

This parser makes sure to resolves placeholders as lazily as possible. + * + * @author Stephane Nicoll + * @since 6.2 + */ +final class PlaceholderParser { + + private static final Log logger = LogFactory.getLog(PlaceholderParser.class); + + private static final Map wellKnownSimplePrefixes = Map.of( + "}", "{", "]", "[", ")", "("); + + private final String prefix; + + private final String suffix; + + private final String simplePrefix; + + @Nullable + private final String separator; + + private final boolean ignoreUnresolvablePlaceholders; + + @Nullable + private final Character escape; + + /** + * Create an instance using the specified input for the parser. + * + * @param prefix the prefix that denotes the start of a placeholder + * @param suffix the suffix that denotes the end of a placeholder + * @param ignoreUnresolvablePlaceholders whether unresolvable placeholders + * should be ignored ({@code true}) or cause an exception ({@code false}) + * @param separator the separating character between the placeholder + * variable and the associated default value, if any + * @param escape the character to use at the beginning of a placeholder + * to escape it and render it as is + */ + PlaceholderParser(String prefix, String suffix, boolean ignoreUnresolvablePlaceholders, + @Nullable String separator, @Nullable Character escape) { + this.prefix = prefix; + this.suffix = suffix; + String simplePrefixForSuffix = wellKnownSimplePrefixes.get(this.suffix); + if (simplePrefixForSuffix != null && this.prefix.endsWith(simplePrefixForSuffix)) { + this.simplePrefix = simplePrefixForSuffix; + } + else { + this.simplePrefix = this.prefix; + } + this.separator = separator; + this.ignoreUnresolvablePlaceholders = ignoreUnresolvablePlaceholders; + this.escape = escape; + } + + /** + * 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 + */ + public String replacePlaceholders(String value, PlaceholderResolver placeholderResolver) { + Assert.notNull(value, "'value' must not be null"); + ParsedValue parsedValue = parse(value); + PartResolutionContext resolutionContext = new PartResolutionContext(placeholderResolver, + this.prefix, this.suffix, this.ignoreUnresolvablePlaceholders, + candidate -> parse(candidate, false)); + return parsedValue.resolve(resolutionContext); + } + + /** + * Parse the specified value. + * @param value the value containing the placeholders to be replaced + * @return the different parts that have been identified + */ + ParsedValue parse(String value) { + List parts = parse(value, false); + return new ParsedValue(value, parts); + } + + private List parse(String value, boolean inPlaceholder) { + LinkedList parts = new LinkedList<>(); + int startIndex = nextStartPrefix(value, 0); + if (startIndex == -1) { + Part part = inPlaceholder ? createSimplePlaceholderPart(value) + : new TextPart(value); + parts.add(part); + return parts; + } + int position = 0; + while (startIndex != -1) { + int endIndex = nextValidEndPrefix(value, startIndex); + if (endIndex == -1) { // Not a valid placeholder, consume the prefix and continue + addText(value, position, startIndex + this.prefix.length(), parts); + position = startIndex + this.prefix.length(); + startIndex = nextStartPrefix(value, position); + } + else if (isEscaped(value, startIndex)) { // Not a valid index, accumulate and skip the escape character + addText(value, position, startIndex - 1, parts); + addText(value, startIndex, startIndex + this.prefix.length(), parts); + position = startIndex + this.prefix.length(); + startIndex = nextStartPrefix(value, position); + } + else { // Found valid placeholder, recursive parsing + addText(value, position, startIndex, parts); + String placeholder = value.substring(startIndex + this.prefix.length(), endIndex); + List placeholderParts = parse(placeholder, true); + parts.addAll(placeholderParts); + startIndex = nextStartPrefix(value, endIndex + this.suffix.length()); + position = endIndex + this.suffix.length(); + } + } + // Add rest of text if necessary + addText(value, position, value.length(), parts); + return inPlaceholder ? List.of(createNestedPlaceholderPart(value, parts)) : parts; + } + + private SimplePlaceholderPart createSimplePlaceholderPart(String text) { + String[] keyAndDefault = splitKeyAndDefault(text); + return (keyAndDefault != null) ? new SimplePlaceholderPart(text, keyAndDefault[0], keyAndDefault[1]) + : new SimplePlaceholderPart(text, text, null); + } + + private NestedPlaceholderPart createNestedPlaceholderPart(String text, List parts) { + if (this.separator == null) { + return new NestedPlaceholderPart(text, parts, null); + } + List keyParts = new ArrayList<>(); + List defaultParts = new ArrayList<>(); + for (int i = 0; i < parts.size(); i++) { + Part part = parts.get(i); + if (!(part instanceof TextPart)) { + keyParts.add(part); + } + else { + String candidate = part.text(); + String[] keyAndDefault = splitKeyAndDefault(candidate); + if (keyAndDefault != null) { + keyParts.add(new TextPart(keyAndDefault[0])); + if (keyAndDefault[1] != null) { + defaultParts.add(new TextPart(keyAndDefault[1])); + } + defaultParts.addAll(parts.subList(i + 1, parts.size())); + return new NestedPlaceholderPart(text, keyParts, defaultParts); + } + else { + keyParts.add(part); + } + } + } + // No separator found + return new NestedPlaceholderPart(text, parts, null); + } + + @Nullable + private String[] splitKeyAndDefault(String value) { + if (this.separator == null || !value.contains(this.separator)) { + return null; + } + int position = 0; + int index = value.indexOf(this.separator, position); + StringBuilder buffer = new StringBuilder(); + while (index != -1) { + if (isEscaped(value, index)) { + // Accumulate, without the escape character. + buffer.append(value, position, index - 1); + buffer.append(value, index, index + this.separator.length()); + position = index + this.separator.length(); + index = value.indexOf(this.separator, position); + } + else { + buffer.append(value, position, index); + String key = buffer.toString(); + String fallback = value.substring(index + this.separator.length()); + return new String[] { key, fallback }; + } + } + buffer.append(value, position, value.length()); + return new String[] { buffer.toString(), null }; + } + + private static void addText(String value, int start, int end, LinkedList parts) { + if (start > end) { + return; + } + String text = value.substring(start, end); + if (!text.isEmpty()) { + if (!parts.isEmpty()) { + Part current = parts.removeLast(); + if (current instanceof TextPart textPart) { + parts.add(new TextPart(textPart.text + text)); + } + else { + parts.add(current); + parts.add(new TextPart(text)); + } + } + else { + parts.add(new TextPart(text)); + } + } + } + + + private int nextStartPrefix(String value, int index) { + return value.indexOf(this.prefix, index); + } + + private int nextValidEndPrefix(String value, int startIndex) { + int index = startIndex + this.prefix.length(); + int withinNestedPlaceholder = 0; + while (index < value.length()) { + if (StringUtils.substringMatch(value, index, this.suffix)) { + if (withinNestedPlaceholder > 0) { + withinNestedPlaceholder--; + index = index + this.suffix.length(); + } + else { + return index; + } + } + else if (StringUtils.substringMatch(value, index, this.simplePrefix)) { + withinNestedPlaceholder++; + index = index + this.simplePrefix.length(); + } + else { + index++; + } + } + return -1; + } + + private boolean isEscaped(String value, int index) { + return (this.escape != null && index > 0 && value.charAt(index - 1) == this.escape); + } + + /** + * Provide the necessary to handle and resolve underlying placeholders. + */ + static class PartResolutionContext implements PlaceholderResolver { + + private final String prefix; + + private final String suffix; + + private final boolean ignoreUnresolvablePlaceholders; + + private final Function> parser; + + private final PlaceholderResolver resolver; + + @Nullable + private Set visitedPlaceholders; + + PartResolutionContext(PlaceholderResolver resolver, String prefix, String suffix, + boolean ignoreUnresolvablePlaceholders, Function> parser) { + this.prefix = prefix; + this.suffix = suffix; + this.ignoreUnresolvablePlaceholders = ignoreUnresolvablePlaceholders; + this.parser = parser; + this.resolver = resolver; + } + + @Override + public String resolvePlaceholder(String placeholderName) { + String value = this.resolver.resolvePlaceholder(placeholderName); + if (value != null && logger.isTraceEnabled()) { + logger.trace("Resolved placeholder '" + placeholderName + "'"); + } + return value; + } + + public String handleUnresolvablePlaceholder(String key, String text) { + if (this.ignoreUnresolvablePlaceholders) { + return toPlaceholderText(key); + } + String originalValue = (!key.equals(text) ? toPlaceholderText(text) : null); + throw new PlaceholderResolutionException( + "Could not resolve placeholder '%s'".formatted(key), key, originalValue); + } + + private String toPlaceholderText(String text) { + return this.prefix + text + this.suffix; + } + + public List parse(String text) { + return this.parser.apply(text); + } + + public void flagPlaceholderAsVisited(String placeholder) { + if (this.visitedPlaceholders == null) { + this.visitedPlaceholders = new HashSet<>(4); + } + if (!this.visitedPlaceholders.add(placeholder)) { + throw new PlaceholderResolutionException( + "Circular placeholder reference '%s'".formatted(placeholder), placeholder, null); + } + } + + public void removePlaceholder(String placeholder) { + this.visitedPlaceholders.remove(placeholder); + } + + } + + + /** + * A part is a section of a String containing placeholders to replace. + */ + interface Part { + + /** + * Resolve this part using the specified {@link PartResolutionContext}. + * @param resolutionContext the context to use + * @return the resolved part + */ + String resolve(PartResolutionContext resolutionContext); + + /** + * Provide a textual representation of this part. + * @return the raw text that this part defines + */ + String text(); + + /** + * Return a String that appends the resolution of the specified parts. + * @param parts the parts to resolve + * @param resolutionContext the context to use for the resolution + * @return a concatenation of the supplied parts with placeholders replaced inline + */ + static String resolveAll(Iterable parts, PartResolutionContext resolutionContext) { + StringBuilder sb = new StringBuilder(); + for (Part part : parts) { + sb.append(part.resolve(resolutionContext)); + } + return sb.toString(); + } + + } + + /** + * A representation of the parsing of an input string. + * @param text the raw input string + * @param parts the parts that appear in the string, in order + */ + record ParsedValue(String text, List parts) { + + public String resolve(PartResolutionContext resolutionContext) { + try { + return Part.resolveAll(this.parts, resolutionContext); + } + catch (PlaceholderResolutionException ex) { + throw ex.withValue(this.text); + } + } + + } + + /** + * A {@link Part} implementation that does not contain a valid placeholder. + * @param text the raw (and resolved) text + */ + record TextPart(String text) implements Part { + + @Override + public String resolve(PartResolutionContext resolutionContext) { + return this.text; + } + + } + + /** + * A {@link Part} implementation that represents a single placeholder with + * a hard-coded fallback. + * @param text the raw text + * @param key the key of the placeholder + * @param fallback the fallback to use, if any + */ + record SimplePlaceholderPart(String text, String key, @Nullable String fallback) implements Part { + + @Override + public String resolve(PartResolutionContext resolutionContext) { + String resolvedValue = resolveToText(resolutionContext, this.key); + if (resolvedValue != null) { + return resolvedValue; + } + else if (this.fallback != null) { + return this.fallback; + } + return resolutionContext.handleUnresolvablePlaceholder(this.key, this.text); + } + + @Nullable + private String resolveToText(PartResolutionContext resolutionContext, String text) { + String resolvedValue = resolutionContext.resolvePlaceholder(text); + if (resolvedValue != null) { + resolutionContext.flagPlaceholderAsVisited(text); + // Let's check if we need to recursively resolve that value + List nestedParts = resolutionContext.parse(resolvedValue); + String value = toText(nestedParts); + if (!isTextOnly(nestedParts)) { + value = new ParsedValue(resolvedValue, nestedParts).resolve(resolutionContext); + } + resolutionContext.removePlaceholder(text); + return value; + } + // Not found + return null; + } + + private boolean isTextOnly(List parts) { + return parts.stream().allMatch(part -> part instanceof TextPart); + } + + private String toText(List parts) { + StringBuilder sb = new StringBuilder(); + parts.forEach(part -> sb.append(part.text())); + return sb.toString(); + } + + } + + /** + * A {@link Part} implementation that represents a single placeholder + * containing nested placeholders. + * @param text the raw text of the root placeholder + * @param keyParts the parts of the key + * @param defaultParts the parts of the fallback, if any + */ + record NestedPlaceholderPart(String text, List keyParts, @Nullable List defaultParts) implements Part { + + @Override + public String resolve(PartResolutionContext resolutionContext) { + String resolvedKey = Part.resolveAll(this.keyParts, resolutionContext); + String value = resolutionContext.resolvePlaceholder(resolvedKey); + if (value != null) { + return value; + } + else if (this.defaultParts != null) { + return Part.resolveAll(this.defaultParts, resolutionContext); + } + return resolutionContext.handleUnresolvablePlaceholder(resolvedKey, this.text); + } + + } + +} diff --git a/spring-core/src/main/java/org/springframework/util/PlaceholderResolutionException.java b/spring-core/src/main/java/org/springframework/util/PlaceholderResolutionException.java new file mode 100644 index 0000000000..bc789a9bba --- /dev/null +++ b/spring-core/src/main/java/org/springframework/util/PlaceholderResolutionException.java @@ -0,0 +1,100 @@ +/* + * Copyright 2002-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.util; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.springframework.lang.Nullable; + +/** + * Thrown when the resolution of placeholder failed. This exception provides + * the placeholder as well as the hierarchy of values that led to the issue. + * + * @author Stephane Nicoll + * @since 6.2 + */ +@SuppressWarnings("serial") +public class PlaceholderResolutionException extends RuntimeException { + + private final String reason; + + private final String placeholder; + + private final List values; + + /** + * Create an exception using the specified reason for its message. + * @param reason the reason for the exception, should contain the placeholder + * @param placeholder the placeholder + * @param value the original expression that led to the issue if available + */ + PlaceholderResolutionException(String reason, String placeholder, @Nullable String value) { + this(reason, placeholder, (value != null ? List.of(value) : Collections.emptyList())); + } + + private PlaceholderResolutionException(String reason, String placeholder, List values) { + super(buildMessage(reason, values)); + this.reason = reason; + this.placeholder = placeholder; + this.values = values; + } + + private static String buildMessage(String reason, List values) { + StringBuilder sb = new StringBuilder(); + sb.append(reason); + if (!CollectionUtils.isEmpty(values)) { + String valuesChain = values.stream().map(value -> "\"" + value + "\"") + .collect(Collectors.joining(" <-- ")); + sb.append(" in value %s".formatted(valuesChain)); + } + return sb.toString(); + } + + /** + * Return a {@link PlaceholderResolutionException} that provides + * an additional parent value. + * @param value the parent value to add + * @return a new exception with the parent value added + */ + PlaceholderResolutionException withValue(String value) { + List allValues = new ArrayList<>(this.values); + allValues.add(value); + return new PlaceholderResolutionException(this.reason, this.placeholder, allValues); + } + + /** + * Return the placeholder that could not be resolved. + * @return the unresolvable placeholder + */ + public String getPlaceholder() { + return this.placeholder; + } + + /** + * Return a contextualized list of the resolution attempts that led to this + * exception, where the first element is the value that generated this + * exception. + * @return the stack of values that led to this exception + */ + public List getValues() { + return this.values; + } + +} diff --git a/spring-core/src/test/java/org/springframework/util/PlaceholderParserTests.java b/spring-core/src/test/java/org/springframework/util/PlaceholderParserTests.java new file mode 100644 index 0000000000..110383eb68 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/util/PlaceholderParserTests.java @@ -0,0 +1,355 @@ +/* + * Copyright 2002-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.util; + +import java.util.Properties; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import org.springframework.util.PlaceholderParser.ParsedValue; +import org.springframework.util.PlaceholderParser.TextPart; +import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +/** + * Tests for {@link PlaceholderParser}. + * + * @author Stephane Nicoll + */ +class PlaceholderParserTests { + + @Nested // Tests with only the basic placeholder feature enabled + class OnlyPlaceholderTests { + + private final PlaceholderParser parser = new PlaceholderParser("${", "}", true, null, null); + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("placeholders") + void placeholderIsReplaced(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("firstName", "John"); + properties.setProperty("nested0", "first"); + properties.setProperty("nested1", "Name"); + assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected); + } + + static Stream placeholders() { + return Stream.of( + Arguments.of("${firstName}", "John"), + Arguments.of("$${firstName}", "$John"), + Arguments.of("}${firstName}", "}John"), + Arguments.of("${firstName}$", "John$"), + Arguments.of("${firstName}}", "John}"), + Arguments.of("${firstName} ${firstName}", "John John"), + Arguments.of("First name: ${firstName}", "First name: John"), + Arguments.of("${firstName} is the first name", "John is the first name"), + Arguments.of("${first${nested1}}", "John"), + Arguments.of("${${nested0}${nested1}}", "John") + ); + } + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("nestedPlaceholders") + void nestedPlaceholdersAreReplaced(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("p1", "v1"); + properties.setProperty("p2", "v2"); + properties.setProperty("p3", "${p1}:${p2}"); // nested placeholders + properties.setProperty("p4", "${p3}"); // deeply nested placeholders + properties.setProperty("p5", "${p1}:${p2}:${bogus}"); // unresolvable placeholder + assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected); + } + + static Stream nestedPlaceholders() { + return Stream.of( + Arguments.of("${p1}:${p2}", "v1:v2"), + Arguments.of("${p3}", "v1:v2"), + Arguments.of("${p4}", "v1:v2"), + Arguments.of("${p5}", "v1:v2:${bogus}"), + Arguments.of("${p0${p0}}", "${p0${p0}}") + ); + } + + @Test + void parseWithSinglePlaceholder() { + PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John"); + assertThat(this.parser.replacePlaceholders("${firstName}", resolver)) + .isEqualTo("John"); + verify(resolver).resolvePlaceholder("firstName"); + verifyNoMoreInteractions(resolver); + } + + @Test + void parseWithPlaceholderAndPrefixText() { + PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John"); + assertThat(this.parser.replacePlaceholders("This is ${firstName}", resolver)) + .isEqualTo("This is John"); + verify(resolver).resolvePlaceholder("firstName"); + verifyNoMoreInteractions(resolver); + } + + @Test + void parseWithMultiplePlaceholdersAndText() { + PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John", "lastName", "Smith"); + assertThat(this.parser.replacePlaceholders("User: ${firstName} - ${lastName}.", resolver)) + .isEqualTo("User: John - Smith."); + verify(resolver).resolvePlaceholder("firstName"); + verify(resolver).resolvePlaceholder("lastName"); + verifyNoMoreInteractions(resolver); + } + + @Test + void parseWithNestedPlaceholderInKey() { + PlaceholderResolver resolver = mockPlaceholderResolver( + "nested", "Name", "firstName", "John"); + assertThat(this.parser.replacePlaceholders("${first${nested}}", resolver)) + .isEqualTo("John"); + verifyPlaceholderResolutions(resolver, "nested", "firstName"); + } + + @Test + void parseWithMultipleNestedPlaceholdersInKey() { + PlaceholderResolver resolver = mockPlaceholderResolver( + "nested0", "first", "nested1", "Name", "firstName", "John"); + assertThat(this.parser.replacePlaceholders("${${nested0}${nested1}}", resolver)) + .isEqualTo("John"); + verifyPlaceholderResolutions(resolver, "nested0", "nested1", "firstName"); + } + + @Test + void placeholdersWithSeparatorAreHandledAsIs() { + PlaceholderResolver resolver = mockPlaceholderResolver("my:test", "value"); + assertThat(this.parser.replacePlaceholders("${my:test}", resolver)).isEqualTo("value"); + verifyPlaceholderResolutions(resolver, "my:test"); + } + + @Test + void placeholdersWithoutEscapeCharAreNotEscaped() { + PlaceholderResolver resolver = mockPlaceholderResolver("test", "value"); + assertThat(this.parser.replacePlaceholders("\\${test}", resolver)).isEqualTo("\\value"); + verifyPlaceholderResolutions(resolver, "test"); + } + + @Test + void textWithInvalidPlaceholderIsMerged() { + String text = "test${of${with${and${"; + ParsedValue parsedValue = this.parser.parse(text); + assertThat(parsedValue.parts()).singleElement().isInstanceOfSatisfying( + TextPart.class, textPart -> assertThat(textPart.text()).isEqualTo(text)); + } + + } + + @Nested // Tests with the use of a separator + class DefaultValueTests { + + private final PlaceholderParser parser = new PlaceholderParser("${", "}", true, ":", null); + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("placeholders") + void placeholderIsReplaced(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("firstName", "John"); + properties.setProperty("nested0", "first"); + properties.setProperty("nested1", "Name"); + assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected); + } + + static Stream placeholders() { + return Stream.of( + Arguments.of("${invalid:John}", "John"), + Arguments.of("${first${invalid:Name}}", "John"), + Arguments.of("${invalid:${firstName}}", "John"), + Arguments.of("${invalid:${${nested0}${nested1}}}", "John"), + Arguments.of("${invalid:$${firstName}}", "$John"), + Arguments.of("${invalid: }${firstName}", " John"), + Arguments.of("${invalid:}", ""), + Arguments.of("${:}", "") + ); + } + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("nestedPlaceholders") + void nestedPlaceholdersAreReplaced(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("p1", "v1"); + properties.setProperty("p2", "v2"); + properties.setProperty("p3", "${p1}:${p2}"); // nested placeholders + properties.setProperty("p4", "${p3}"); // deeply nested placeholders + properties.setProperty("p5", "${p1}:${p2}:${bogus}"); // unresolvable placeholder + properties.setProperty("p6", "${p1}:${p2}:${bogus:def}"); // unresolvable w/ default + assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected); + } + + static Stream nestedPlaceholders() { + return Stream.of( + Arguments.of("${p6}", "v1:v2:def"), + Arguments.of("${invalid:${p1}:${p2}}", "v1:v2"), + Arguments.of("${invalid:${p3}}", "v1:v2"), + Arguments.of("${invalid:${p4}}", "v1:v2"), + Arguments.of("${invalid:${p5}}", "v1:v2:${bogus}"), + Arguments.of("${invalid:${p6}}", "v1:v2:def") + ); + } + + @Test + void parseWithHardcodedFallback() { + PlaceholderResolver resolver = mockPlaceholderResolver(); + assertThat(this.parser.replacePlaceholders("${firstName:Steve}", resolver)) + .isEqualTo("Steve"); + verifyPlaceholderResolutions(resolver, "firstName"); + } + + @Test + void parseWithNestedPlaceholderInKeyUsingFallback() { + PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John"); + assertThat(this.parser.replacePlaceholders("${first${invalid:Name}}", resolver)) + .isEqualTo("John"); + verifyPlaceholderResolutions(resolver, "invalid", "firstName"); + } + + @Test + void parseWithFallbackUsingPlaceholder() { + PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John"); + assertThat(this.parser.replacePlaceholders("${invalid:${firstName}}", resolver)) + .isEqualTo("John"); + verifyPlaceholderResolutions(resolver, "invalid", "firstName"); + } + + } + + @Nested // Tests with the use of the escape character + class EscapedTests { + + private final PlaceholderParser parser = new PlaceholderParser("${", "}", true, ":", '\\'); + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("escapedPlaceholders") + void escapedPlaceholderIsNotReplaced(String text, String expected) { + PlaceholderResolver resolver = mockPlaceholderResolver( + "firstName", "John", "nested0", "first", "nested1", "Name", + "${test}", "John", + "p1", "v1", "p2", "\\${p1:default}", "p3", "${p2}", + "p4", "adc${p0:\\${p1}}", + "p5", "adc${\\${p0}:${p1}}", + "p6", "adc${p0:def\\${p1}}", + "p7", "adc\\${"); + assertThat(this.parser.replacePlaceholders(text, resolver)).isEqualTo(expected); + } + + static Stream escapedPlaceholders() { + return Stream.of( + Arguments.of("\\${firstName}", "${firstName}"), + Arguments.of("First name: \\${firstName}", "First name: ${firstName}"), + Arguments.of("$\\${firstName}", "$${firstName}"), + Arguments.of("\\}${firstName}", "\\}John"), + Arguments.of("${\\${test}}", "John"), + Arguments.of("${p2}", "${p1:default}"), + Arguments.of("${p3}", "${p1:default}"), + Arguments.of("${p4}", "adc${p1}"), + Arguments.of("${p5}", "adcv1"), + Arguments.of("${p6}", "adcdef${p1}"), + Arguments.of("${p7}", "adc\\${")); + + } + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("escapedSeparators") + void escapedSeparatorIsNotReplaced(String text, String expected) { + Properties properties = new Properties(); + properties.setProperty("first:Name", "John"); + properties.setProperty("nested0", "first"); + properties.setProperty("nested1", "Name"); + assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected); + } + + static Stream escapedSeparators() { + return Stream.of( + Arguments.of("${first\\:Name}", "John"), + Arguments.of("${last\\:Name}", "${last:Name}") + ); + } + + } + + @Nested + class ExceptionTests { + + private final PlaceholderParser parser = new PlaceholderParser("${", "}", false, ":", null); + + @Test + void textWithCircularReference() { + PlaceholderResolver resolver = mockPlaceholderResolver("pL", "${pR}", "pR", "${pL}"); + assertThatThrownBy(() -> this.parser.replacePlaceholders("${pL}", resolver)) + .isInstanceOf(PlaceholderResolutionException.class) + .hasMessage("Circular placeholder reference 'pL' in value \"${pL}\" <-- \"${pR}\" <-- \"${pL}\""); + } + + @Test + void unresolvablePlaceholderIsReported() { + PlaceholderResolver resolver = mockPlaceholderResolver(); + assertThatExceptionOfType(PlaceholderResolutionException.class) + .isThrownBy(() -> this.parser.replacePlaceholders("${bogus}", resolver)) + .withMessage("Could not resolve placeholder 'bogus' in value \"${bogus}\"") + .withNoCause(); + } + + @Test + void unresolvablePlaceholderInNestedPlaceholderIsReportedWithChain() { + PlaceholderResolver resolver = mockPlaceholderResolver("p1", "v1", "p2", "v2", + "p3", "${p1}:${p2}:${bogus}"); + assertThatExceptionOfType(PlaceholderResolutionException.class) + .isThrownBy(() -> this.parser.replacePlaceholders("${p3}", resolver)) + .withMessage("Could not resolve placeholder 'bogus' in value \"${p1}:${p2}:${bogus}\" <-- \"${p3}\"") + .withNoCause(); + } + + } + + PlaceholderResolver mockPlaceholderResolver(String... pairs) { + if (pairs.length % 2 == 1) { + throw new IllegalArgumentException("size must be even, it is a set of key=value pairs"); + } + PlaceholderResolver resolver = mock(PlaceholderResolver.class); + for (int i = 0; i < pairs.length; i += 2) { + String key = pairs[i]; + String value = pairs[i + 1]; + given(resolver.resolvePlaceholder(key)).willReturn(value); + } + return resolver; + } + + void verifyPlaceholderResolutions(PlaceholderResolver mock, String... placeholders) { + for (String placeholder : placeholders) { + verify(mock).resolvePlaceholder(placeholder); + } + verifyNoMoreInteractions(mock); + } + +} From e3aa5b6b1154345c231acc7950d74cd56d7420c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Fri, 29 Dec 2023 17:41:26 +0100 Subject: [PATCH 2/2] Use new implementation in PropertyPlaceholderHelper This commit removes the previous implementation in favor of the new PlaceholderParser. The only noticeable side effect is that the exception is no longer an IllegalArgumentException, but rather the dedicated PlaceholderResolutionException. See gh-9628 --- .../annotation-config/value-annotations.adoc | 4 +- .../config/PlaceholderConfigurerSupport.java | 19 ++- .../config/PropertyPlaceholderConfigurer.java | 5 +- .../PropertySourcesPlaceholderConfigurer.java | 3 +- .../PropertySourceAnnotationTests.java | 5 +- .../config/ContextNamespaceHandlerTests.java | 5 +- ...ertySourcesPlaceholderConfigurerTests.java | 7 +- spring-core/spring-core.gradle | 1 + .../core/env/AbstractEnvironment.java | 7 +- .../core/env/AbstractPropertyResolver.java | 20 ++- .../env/ConfigurablePropertyResolver.java | 10 +- .../io/support/PropertySourceProcessor.java | 7 +- .../util/PropertyPlaceholderHelper.java | 157 ++++-------------- .../util/SystemPropertyUtils.java | 11 +- .../PropertySourcesPropertyResolverTests.java | 13 +- .../core/env/StandardEnvironmentTests.java | 6 +- .../core/io/ResourceEditorTests.java | 4 +- .../support/PropertySourceProcessorTests.java | 12 +- .../ResourceArrayPropertyEditorTests.java | 7 +- .../util/PropertyPlaceholderHelperTests.java | 19 +-- .../util/SystemPropertyUtilsTests.java | 6 +- .../SendToMethodReturnValueHandler.java | 4 +- .../setup/StandaloneMockMvcBuilder.java | 4 +- .../web/util/ServletContextPropertyUtils.java | 8 +- 24 files changed, 156 insertions(+), 188 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/core/beans/annotation-config/value-annotations.adoc b/framework-docs/modules/ROOT/pages/core/beans/annotation-config/value-annotations.adoc index 967e04af4e..4f5fd95cad 100644 --- a/framework-docs/modules/ROOT/pages/core/beans/annotation-config/value-annotations.adoc +++ b/framework-docs/modules/ROOT/pages/core/beans/annotation-config/value-annotations.adoc @@ -101,8 +101,8 @@ NOTE: When configuring a `PropertySourcesPlaceholderConfigurer` using JavaConfig Using the above configuration ensures Spring initialization failure if any `${}` placeholder could not be resolved. It is also possible to use methods like -`setPlaceholderPrefix`, `setPlaceholderSuffix`, or `setValueSeparator` to customize -placeholders. +`setPlaceholderPrefix`, `setPlaceholderSuffix`, `setValueSeparator`, or +`setEscapeCharacter` to customize placeholders. NOTE: Spring Boot configures by default a `PropertySourcesPlaceholderConfigurer` bean that will get properties from `application.properties` and `application.yml` files. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/PlaceholderConfigurerSupport.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/PlaceholderConfigurerSupport.java index 91910a2f4a..e357ec061c 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/PlaceholderConfigurerSupport.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/PlaceholderConfigurerSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -100,6 +100,8 @@ public abstract class PlaceholderConfigurerSupport extends PropertyResourceConfi /** Default value separator: {@value}. */ public static final String DEFAULT_VALUE_SEPARATOR = ":"; + /** Default escape character: {@value}. */ + public static final Character DEFAULT_ESCAPE_CHARACTER = '\\'; /** Defaults to {@value #DEFAULT_PLACEHOLDER_PREFIX}. */ protected String placeholderPrefix = DEFAULT_PLACEHOLDER_PREFIX; @@ -111,6 +113,10 @@ public abstract class PlaceholderConfigurerSupport extends PropertyResourceConfi @Nullable protected String valueSeparator = DEFAULT_VALUE_SEPARATOR; + /** Defaults to {@value #DEFAULT_ESCAPE_CHARACTER}. */ + @Nullable + protected Character escapeCharacter = DEFAULT_ESCAPE_CHARACTER; + protected boolean trimValues = false; @Nullable @@ -151,6 +157,17 @@ public abstract class PlaceholderConfigurerSupport extends PropertyResourceConfi this.valueSeparator = valueSeparator; } + /** + * Specify the escape character to use to ignore placeholder prefix + * or value separator, or {@code null} if no escaping should take + * place. + *

Default is {@value #DEFAULT_ESCAPE_CHARACTER}. + * @since 6.2 + */ + public void setEscapeCharacter(@Nullable Character escsEscapeCharacter) { + this.escapeCharacter = escsEscapeCharacter; + } + /** * Specify whether to trim resolved values before applying them, * removing superfluous whitespace from the beginning and end. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurer.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurer.java index 0fba4f79c2..d5fe3bf607 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurer.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -234,7 +234,8 @@ public class PropertyPlaceholderConfigurer extends PlaceholderConfigurerSupport public PlaceholderResolvingStringValueResolver(Properties props) { this.helper = new PropertyPlaceholderHelper( - placeholderPrefix, placeholderSuffix, valueSeparator, ignoreUnresolvablePlaceholders); + placeholderPrefix, placeholderSuffix, valueSeparator, + ignoreUnresolvablePlaceholders, escapeCharacter); this.resolver = new PropertyPlaceholderConfigurerResolver(props); } diff --git a/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java b/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java index 1fd84402c1..1035e175e6 100644 --- a/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java +++ b/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -193,6 +193,7 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS propertyResolver.setPlaceholderPrefix(this.placeholderPrefix); propertyResolver.setPlaceholderSuffix(this.placeholderSuffix); propertyResolver.setValueSeparator(this.valueSeparator); + propertyResolver.setEscapeCharacter(this.escapeCharacter); StringValueResolver valueResolver = strVal -> { String resolved = (this.ignoreUnresolvablePlaceholders ? diff --git a/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java b/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java index 4ec292abd3..df6a0723be 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -40,6 +40,7 @@ import org.springframework.core.env.MutablePropertySources; import org.springframework.core.io.support.EncodedResource; import org.springframework.core.io.support.PropertiesLoaderUtils; import org.springframework.core.io.support.PropertySourceFactory; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -132,7 +133,7 @@ class PropertySourceAnnotationTests { void withUnresolvablePlaceholder() { assertThatExceptionOfType(BeanDefinitionStoreException.class) .isThrownBy(() -> new AnnotationConfigApplicationContext(ConfigWithUnresolvablePlaceholder.class)) - .withCauseInstanceOf(IllegalArgumentException.class); + .withCauseInstanceOf(PlaceholderResolutionException.class); } @Test diff --git a/spring-context/src/test/java/org/springframework/context/config/ContextNamespaceHandlerTests.java b/spring-context/src/test/java/org/springframework/context/config/ContextNamespaceHandlerTests.java index 21e6db8d94..532135b383 100644 --- a/spring-context/src/test/java/org/springframework/context/config/ContextNamespaceHandlerTests.java +++ b/spring-context/src/test/java/org/springframework/context/config/ContextNamespaceHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -28,6 +28,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericXmlApplicationContext; import org.springframework.core.io.ClassPathResource; import org.springframework.mock.env.MockEnvironment; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -136,7 +137,7 @@ class ContextNamespaceHandlerTests { assertThatExceptionOfType(FatalBeanException.class).isThrownBy(() -> new ClassPathXmlApplicationContext("contextNamespaceHandlerTests-location-placeholder.xml", getClass())) .havingRootCause() - .isInstanceOf(IllegalArgumentException.class) + .isInstanceOf(PlaceholderResolutionException.class) .withMessage("Could not resolve placeholder 'foo' in value \"${foo}\""); } diff --git a/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java b/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java index 33e27414c4..c8bb5d5ef7 100644 --- a/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java @@ -37,6 +37,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.testfixture.env.MockPropertySource; import org.springframework.mock.env.MockEnvironment; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -170,7 +171,7 @@ class PropertySourcesPlaceholderConfigurerTests { assertThatExceptionOfType(BeanDefinitionStoreException.class) .isThrownBy(() -> ppc.postProcessBeanFactory(bf)) .havingCause() - .isExactlyInstanceOf(IllegalArgumentException.class) + .isExactlyInstanceOf(PlaceholderResolutionException.class) .withMessage("Could not resolve placeholder 'my.name' in value \"${my.name}\""); } @@ -201,8 +202,8 @@ class PropertySourcesPlaceholderConfigurerTests { assertThatExceptionOfType(BeanCreationException.class) .isThrownBy(context::refresh) .havingCause() - .isExactlyInstanceOf(IllegalArgumentException.class) - .withMessage("Could not resolve placeholder 'enigma' in value \"${enigma}\""); + .isExactlyInstanceOf(PlaceholderResolutionException.class) + .withMessage("Could not resolve placeholder 'enigma' in value \"${enigma}\" <-- \"${my.key}\""); } @Test diff --git a/spring-core/spring-core.gradle b/spring-core/spring-core.gradle index a947f95c1a..fedd203d55 100644 --- a/spring-core/spring-core.gradle +++ b/spring-core/spring-core.gradle @@ -102,6 +102,7 @@ dependencies { testImplementation("jakarta.xml.bind:jakarta.xml.bind-api") testImplementation("org.jetbrains.kotlinx:kotlinx-serialization-json") testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor") + testImplementation("org.mockito:mockito-core") testImplementation("org.skyscreamer:jsonassert") testImplementation("org.xmlunit:xmlunit-assertj") testImplementation("org.xmlunit:xmlunit-matchers") diff --git a/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java b/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java index e2cc3355c1..ec93302a7b 100644 --- a/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java +++ b/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -521,6 +521,11 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment { this.propertyResolver.setValueSeparator(valueSeparator); } + @Override + public void setEscapeCharacter(@Nullable Character escapeCharacter) { + this.propertyResolver.setEscapeCharacter(escapeCharacter); + } + @Override public void setIgnoreUnresolvableNestedPlaceholders(boolean ignoreUnresolvableNestedPlaceholders) { this.propertyResolver.setIgnoreUnresolvableNestedPlaceholders(ignoreUnresolvableNestedPlaceholders); 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 c3f29e106a..890cfb0894 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-2020 the original author or authors. + * Copyright 2002-2024 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. @@ -61,6 +61,9 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe @Nullable private String valueSeparator = SystemPropertyUtils.VALUE_SEPARATOR; + @Nullable + private Character escapeCharacter = SystemPropertyUtils.ESCAPE_CHARACTER; + private final Set requiredProperties = new LinkedHashSet<>(); @@ -121,6 +124,19 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe this.valueSeparator = valueSeparator; } + /** + * Specify the escape character to use to ignore placeholder prefix + * or value separator, or {@code null} if no escaping should take + * place. + *

The default is "\". + * @since 6.2 + * @see org.springframework.util.SystemPropertyUtils#ESCAPE_CHARACTER + */ + @Override + public void setEscapeCharacter(@Nullable Character escapeCharacter) { + this.escapeCharacter = escapeCharacter; + } + /** * Set whether to throw an exception when encountering an unresolvable placeholder * nested within the value of a given property. A {@code false} value indicates strict @@ -232,7 +248,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe private PropertyPlaceholderHelper createPlaceholderHelper(boolean ignoreUnresolvablePlaceholders) { return new PropertyPlaceholderHelper(this.placeholderPrefix, this.placeholderSuffix, - this.valueSeparator, ignoreUnresolvablePlaceholders); + this.valueSeparator, ignoreUnresolvablePlaceholders, this.escapeCharacter); } private String doResolvePlaceholders(String text, PropertyPlaceholderHelper helper) { diff --git a/spring-core/src/main/java/org/springframework/core/env/ConfigurablePropertyResolver.java b/spring-core/src/main/java/org/springframework/core/env/ConfigurablePropertyResolver.java index bb2f9bc79d..362ca6c15e 100644 --- a/spring-core/src/main/java/org/springframework/core/env/ConfigurablePropertyResolver.java +++ b/spring-core/src/main/java/org/springframework/core/env/ConfigurablePropertyResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2024 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. @@ -74,6 +74,14 @@ public interface ConfigurablePropertyResolver extends PropertyResolver { */ void setValueSeparator(@Nullable String valueSeparator); + /** + * Specify the escape character to use to ignore placeholder prefix + * or value separator, or {@code null} if no escaping should take + * place. + * @since 6.2 + */ + void setEscapeCharacter(@Nullable Character escapeCharacter); + /** * Set whether to throw an exception when encountering an unresolvable placeholder * nested within the value of a given property. A {@code false} value indicates strict diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java index 7559ce8864..02f0370b22 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -36,6 +36,7 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.PlaceholderResolutionException; import org.springframework.util.ReflectionUtils; /** @@ -93,8 +94,8 @@ public class PropertySourceProcessor { } } catch (RuntimeException | IOException ex) { - // Placeholders not resolvable (IllegalArgumentException) or resource not found when trying to open it - if (ignoreResourceNotFound && (ex instanceof IllegalArgumentException || isIgnorableException(ex) || + // Placeholders not resolvable or resource not found when trying to open it + if (ignoreResourceNotFound && (ex instanceof PlaceholderResolutionException || isIgnorableException(ex) || isIgnorableException(ex.getCause()))) { if (logger.isInfoEnabled()) { logger.info("Properties location [" + location + "] not resolvable: " + ex.getMessage()); 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 c35c048602..00b2791b9c 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-2021 the original author or authors. + * Copyright 2002-2024 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. @@ -16,14 +16,7 @@ package org.springframework.util; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; import java.util.Properties; -import java.util.Set; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.lang.Nullable; @@ -37,31 +30,12 @@ import org.springframework.lang.Nullable; * * @author Juergen Hoeller * @author Rob Harrop + * @author Stephane Nicoll * @since 3.0 */ public class PropertyPlaceholderHelper { - private static final Log logger = LogFactory.getLog(PropertyPlaceholderHelper.class); - - private static final Map wellKnownSimplePrefixes = new HashMap<>(4); - - static { - wellKnownSimplePrefixes.put("}", "{"); - wellKnownSimplePrefixes.put("]", "["); - wellKnownSimplePrefixes.put(")", "("); - } - - - private final String placeholderPrefix; - - private final String placeholderSuffix; - - private final String simplePrefix; - - @Nullable - private final String valueSeparator; - - private final boolean ignoreUnresolvablePlaceholders; + private final PlaceholderParser parser; /** @@ -71,7 +45,7 @@ public class PropertyPlaceholderHelper { * @param placeholderSuffix the suffix that denotes the end of a placeholder */ public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix) { - this(placeholderPrefix, placeholderSuffix, null, true); + this(placeholderPrefix, placeholderSuffix, null, true, null); } /** @@ -82,23 +56,35 @@ public class PropertyPlaceholderHelper { * and the associated default value, if any * @param ignoreUnresolvablePlaceholders indicates whether unresolvable placeholders should * be ignored ({@code true}) or cause an exception ({@code false}) + * @deprecated in favor of {@link PropertyPlaceholderHelper#PropertyPlaceholderHelper(String, String, String, boolean, Character)} */ + @Deprecated(since = "6.2", forRemoval = true) public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix, @Nullable String valueSeparator, boolean ignoreUnresolvablePlaceholders) { + this(placeholderPrefix, placeholderSuffix, valueSeparator, ignoreUnresolvablePlaceholders, null); + } + + /** + * Creates a new {@code PropertyPlaceholderHelper} that uses the supplied prefix and suffix. + * @param placeholderPrefix the prefix that denotes the start of a placeholder + * @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 escapeCharacter the escape character to use to ignore placeholder prefix + * or value separator, if any + * @since 6.2 + */ + public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuffix, + @Nullable String valueSeparator, boolean ignoreUnresolvablePlaceholders, + @Nullable Character escapeCharacter) { + 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); - if (simplePrefixForSuffix != null && this.placeholderPrefix.endsWith(simplePrefixForSuffix)) { - this.simplePrefix = simplePrefixForSuffix; - } - else { - this.simplePrefix = this.placeholderPrefix; - } - this.valueSeparator = valueSeparator; - this.ignoreUnresolvablePlaceholders = ignoreUnresolvablePlaceholders; + this.parser = new PlaceholderParser(placeholderPrefix, placeholderSuffix, + ignoreUnresolvablePlaceholders, valueSeparator, escapeCharacter); } @@ -123,94 +109,11 @@ public class PropertyPlaceholderHelper { */ public String replacePlaceholders(String value, PlaceholderResolver placeholderResolver) { Assert.notNull(value, "'value' must not be null"); - return parseStringValue(value, placeholderResolver, null); + return parseStringValue(value, placeholderResolver); } - protected String parseStringValue( - String value, PlaceholderResolver placeholderResolver, @Nullable Set visitedPlaceholders) { - - int startIndex = value.indexOf(this.placeholderPrefix); - if (startIndex == -1) { - return value; - } - - StringBuilder result = new StringBuilder(value); - while (startIndex != -1) { - int endIndex = findPlaceholderEndIndex(result, startIndex); - if (endIndex != -1) { - String placeholder = result.substring(startIndex + this.placeholderPrefix.length(), endIndex); - String originalPlaceholder = placeholder; - if (visitedPlaceholders == null) { - visitedPlaceholders = new HashSet<>(4); - } - if (!visitedPlaceholders.add(originalPlaceholder)) { - throw new IllegalArgumentException( - "Circular placeholder reference '" + originalPlaceholder + "' in property definitions"); - } - // Recursive invocation, parsing placeholders contained in the placeholder key. - placeholder = parseStringValue(placeholder, placeholderResolver, visitedPlaceholders); - // Now obtain the value for the fully resolved key... - String propVal = placeholderResolver.resolvePlaceholder(placeholder); - if (propVal == null && this.valueSeparator != null) { - int separatorIndex = placeholder.indexOf(this.valueSeparator); - if (separatorIndex != -1) { - String actualPlaceholder = placeholder.substring(0, separatorIndex); - String defaultValue = placeholder.substring(separatorIndex + this.valueSeparator.length()); - propVal = placeholderResolver.resolvePlaceholder(actualPlaceholder); - if (propVal == null) { - propVal = defaultValue; - } - } - } - if (propVal != null) { - // Recursive invocation, parsing placeholders contained in the - // previously resolved placeholder value. - propVal = parseStringValue(propVal, placeholderResolver, visitedPlaceholders); - result.replace(startIndex, endIndex + this.placeholderSuffix.length(), propVal); - if (logger.isTraceEnabled()) { - logger.trace("Resolved placeholder '" + placeholder + "'"); - } - startIndex = result.indexOf(this.placeholderPrefix, startIndex + propVal.length()); - } - else if (this.ignoreUnresolvablePlaceholders) { - // Proceed with unprocessed value. - startIndex = result.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length()); - } - else { - throw new IllegalArgumentException("Could not resolve placeholder '" + - placeholder + "'" + " in value \"" + value + "\""); - } - visitedPlaceholders.remove(originalPlaceholder); - } - else { - startIndex = -1; - } - } - return result.toString(); - } - - private int findPlaceholderEndIndex(CharSequence buf, int startIndex) { - int index = startIndex + this.placeholderPrefix.length(); - int withinNestedPlaceholder = 0; - while (index < buf.length()) { - if (StringUtils.substringMatch(buf, index, this.placeholderSuffix)) { - if (withinNestedPlaceholder > 0) { - withinNestedPlaceholder--; - index = index + this.placeholderSuffix.length(); - } - else { - return index; - } - } - else if (StringUtils.substringMatch(buf, index, this.simplePrefix)) { - withinNestedPlaceholder++; - index = index + this.simplePrefix.length(); - } - else { - index++; - } - } - return -1; + protected String parseStringValue(String value, PlaceholderResolver placeholderResolver) { + return this.parser.replacePlaceholders(value, placeholderResolver); } diff --git a/spring-core/src/main/java/org/springframework/util/SystemPropertyUtils.java b/spring-core/src/main/java/org/springframework/util/SystemPropertyUtils.java index 21b58ca18e..8fa95ebff8 100644 --- a/spring-core/src/main/java/org/springframework/util/SystemPropertyUtils.java +++ b/spring-core/src/main/java/org/springframework/util/SystemPropertyUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -44,12 +44,17 @@ public abstract class SystemPropertyUtils { /** Value separator for system property placeholders: {@value}. */ public static final String VALUE_SEPARATOR = ":"; + /** Default escape character: {@value}. */ + public static final Character ESCAPE_CHARACTER = '\\'; + private static final PropertyPlaceholderHelper strictHelper = - new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, false); + new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, + false, ESCAPE_CHARACTER); private static final PropertyPlaceholderHelper nonStrictHelper = - new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, true); + new PropertyPlaceholderHelper(PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, VALUE_SEPARATOR, + true, ESCAPE_CHARACTER); /** diff --git a/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java b/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java index f51fc97e9a..d442f6a0cb 100644 --- a/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java +++ b/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2024 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. @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.testfixture.env.MockPropertySource; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -227,7 +228,7 @@ class PropertySourcesPropertyResolverTests { MutablePropertySources propertySources = new MutablePropertySources(); propertySources.addFirst(new MockPropertySource().withProperty("key", "value")); PropertyResolver resolver = new PropertySourcesPropertyResolver(propertySources); - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> resolver.resolveRequiredPlaceholders("Replace this ${key} plus ${unknown}")); } @@ -290,11 +291,11 @@ class PropertySourcesPropertyResolverTests { assertThat(pr.getProperty("p2")).isEqualTo("v2"); assertThat(pr.getProperty("p3")).isEqualTo("v1:v2"); assertThat(pr.getProperty("p4")).isEqualTo("v1:v2"); - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> pr.getProperty("p5")) .withMessageContaining("Could not resolve placeholder 'bogus' in value \"${p1}:${p2}:${bogus}\""); assertThat(pr.getProperty("p6")).isEqualTo("v1:v2:def"); - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> pr.getProperty("pL")) .withMessageContaining("Circular"); } @@ -315,7 +316,7 @@ class PropertySourcesPropertyResolverTests { // placeholders nested within the value of "p4" are unresolvable and cause an // exception by default - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> pr.getProperty("p4")) .withMessageContaining("Could not resolve placeholder 'bogus' in value \"${p1}:${p2}:${bogus}\""); @@ -327,7 +328,7 @@ class PropertySourcesPropertyResolverTests { // resolve[Nested]Placeholders methods behave as usual regardless the value of // ignoreUnresolvableNestedPlaceholders assertThat(pr.resolvePlaceholders("${p1}:${p2}:${bogus}")).isEqualTo("v1:v2:${bogus}"); - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> pr.resolveRequiredPlaceholders("${p1}:${p2}:${bogus}")) .withMessageContaining("Could not resolve placeholder 'bogus' in value \"${p1}:${p2}:${bogus}\""); } 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 cd881a4bd3..f3e05ec8bf 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 @@ -23,8 +23,10 @@ import org.junit.jupiter.api.Test; import org.springframework.core.SpringProperties; import org.springframework.core.testfixture.env.MockPropertySource; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.springframework.core.env.AbstractEnvironment.ACTIVE_PROFILES_PROPERTY_NAME; import static org.springframework.core.env.AbstractEnvironment.DEFAULT_PROFILES_PROPERTY_NAME; @@ -207,9 +209,9 @@ class StandardEnvironmentTests { void defaultProfileWithCircularPlaceholder() { try { System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "${spring.profiles.default}"); - assertThatIllegalArgumentException() + assertThatExceptionOfType(PlaceholderResolutionException.class) .isThrownBy(environment::getDefaultProfiles) - .withMessage("Circular placeholder reference 'spring.profiles.default' in property definitions"); + .withMessageContaining("Circular placeholder reference 'spring.profiles.default'"); } finally { System.clearProperty(DEFAULT_PROFILES_PROPERTY_NAME); diff --git a/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java b/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java index 128d7a44ca..1708749fee 100644 --- a/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/ResourceEditorTests.java @@ -21,8 +21,10 @@ import java.beans.PropertyEditor; import org.junit.jupiter.api.Test; import org.springframework.core.env.StandardEnvironment; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** @@ -96,7 +98,7 @@ class ResourceEditorTests { PropertyEditor editor = new ResourceEditor(new DefaultResourceLoader(), new StandardEnvironment(), false); System.setProperty("test.prop", "foo"); try { - assertThatIllegalArgumentException().isThrownBy(() -> { + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> { editor.setAsText("${test.prop}-${bar}"); editor.getValue(); }); diff --git a/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java b/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java index 221736ce14..0f3ea2ba62 100644 --- a/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java @@ -32,10 +32,12 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.ResourceLoader; import org.springframework.util.ClassUtils; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.mockito.Mockito.mock; /** * Tests for {@link PropertySourceProcessor}. @@ -73,8 +75,8 @@ class PropertySourceProcessorTests { class FailOnErrorTests { @Test - void processorFailsOnIllegalArgumentException() { - assertProcessorFailsOnError(IllegalArgumentExceptionPropertySourceFactory.class, IllegalArgumentException.class); + void processorFailsOnPlaceholderResolutionException() { + assertProcessorFailsOnError(PlaceholderResolutionExceptionPropertySourceFactory.class, PlaceholderResolutionException.class); } @Test @@ -98,7 +100,7 @@ class PropertySourceProcessorTests { @Test void processorIgnoresIllegalArgumentException() { - assertProcessorIgnoresFailure(IllegalArgumentExceptionPropertySourceFactory.class); + assertProcessorIgnoresFailure(PlaceholderResolutionExceptionPropertySourceFactory.class); } @Test @@ -134,11 +136,11 @@ class PropertySourceProcessorTests { } - private static class IllegalArgumentExceptionPropertySourceFactory implements PropertySourceFactory { + private static class PlaceholderResolutionExceptionPropertySourceFactory implements PropertySourceFactory { @Override public PropertySource createPropertySource(String name, EncodedResource resource) { - throw new IllegalArgumentException("bogus"); + throw mock(PlaceholderResolutionException.class); } } diff --git a/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java b/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java index eabd4e46bf..f747d0cbec 100644 --- a/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/support/ResourceArrayPropertyEditorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -24,9 +24,10 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.FileUrlResource; import org.springframework.core.io.Resource; +import org.springframework.util.PlaceholderResolutionException; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests for {@link ResourceArrayPropertyEditor}. @@ -81,7 +82,7 @@ class ResourceArrayPropertyEditorTests { false); System.setProperty("test.prop", "foo"); try { - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> editor.setAsText("${test.prop}-${bar}")); } finally { diff --git a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java index 429df4d0a4..2b97330046 100644 --- a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java +++ b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -19,7 +19,6 @@ package org.springframework.util; import java.util.Properties; import java.util.stream.Stream; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -29,11 +28,11 @@ import org.junit.jupiter.params.provider.MethodSource; import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; /** * Tests for {@link PropertyPlaceholderHelper}. @@ -116,16 +115,15 @@ class PropertyPlaceholderHelperTests { Properties props = new Properties(); props.setProperty("foo", "bar"); - PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", null, false); - assertThatIllegalArgumentException().isThrownBy(() -> + PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", null, false, null); + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> helper.replacePlaceholders(text, props)); - } @Nested class DefaultValueTests { - private final PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", ":", true); + private final PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("${", "}", ":", true, null); @ParameterizedTest(name = "{0} -> {1}") @MethodSource("defaultValues") @@ -137,12 +135,11 @@ class PropertyPlaceholderHelperTests { } @Test - @Disabled("gh-26268") void defaultValueIsNotEvaluatedEarly() { PlaceholderResolver resolver = mockPlaceholderResolver("one", "1"); - assertThat(this.helper.replacePlaceholders("This is ${one:or${two}}",resolver)).isEqualTo("This is 1"); + assertThat(this.helper.replacePlaceholders("This is ${one:or${two}}", resolver)).isEqualTo("This is 1"); verify(resolver).resolvePlaceholder("one"); - verifyNoMoreInteractions(resolver); + verify(resolver, never()).resolvePlaceholder("two"); } static Stream defaultValues() { diff --git a/spring-core/src/test/java/org/springframework/util/SystemPropertyUtilsTests.java b/spring-core/src/test/java/org/springframework/util/SystemPropertyUtilsTests.java index 6761a94d66..b3170f7ee1 100644 --- a/spring-core/src/test/java/org/springframework/util/SystemPropertyUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/util/SystemPropertyUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -21,7 +21,7 @@ import java.util.Map; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Rob Harrop @@ -97,7 +97,7 @@ class SystemPropertyUtilsTests { @Test void replaceWithNoDefault() { - assertThatIllegalArgumentException().isThrownBy(() -> + assertThatExceptionOfType(PlaceholderResolutionException.class).isThrownBy(() -> SystemPropertyUtils.resolvePlaceholders("${test.prop}")); } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SendToMethodReturnValueHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SendToMethodReturnValueHandler.java index 319297e18c..88a65e9c22 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SendToMethodReturnValueHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SendToMethodReturnValueHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -68,7 +68,7 @@ public class SendToMethodReturnValueHandler implements HandlerMethodReturnValueH private String defaultUserDestinationPrefix = "/queue"; - private final PropertyPlaceholderHelper placeholderHelper = new PropertyPlaceholderHelper("{", "}", null, false); + private final PropertyPlaceholderHelper placeholderHelper = new PropertyPlaceholderHelper("{", "}", null, false, null); @Nullable private MessageHeaderInitializer headerInitializer; diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java index d47c3bb94e..40b3d74a2d 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -586,7 +586,7 @@ public class StandaloneMockMvcBuilder extends AbstractMockMvcBuilder values) { - this.helper = new PropertyPlaceholderHelper("${", "}", ":", false); + this.helper = new PropertyPlaceholderHelper("${", "}", ":", false, null); this.resolver = values::get; } diff --git a/spring-web/src/main/java/org/springframework/web/util/ServletContextPropertyUtils.java b/spring-web/src/main/java/org/springframework/web/util/ServletContextPropertyUtils.java index 6ce2c6e1a3..3c8f9cce2b 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ServletContextPropertyUtils.java +++ b/spring-web/src/main/java/org/springframework/web/util/ServletContextPropertyUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2024 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. @@ -39,11 +39,13 @@ public abstract class ServletContextPropertyUtils { private static final PropertyPlaceholderHelper strictHelper = new PropertyPlaceholderHelper(SystemPropertyUtils.PLACEHOLDER_PREFIX, - SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, false); + SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, + false, SystemPropertyUtils.ESCAPE_CHARACTER); private static final PropertyPlaceholderHelper nonStrictHelper = new PropertyPlaceholderHelper(SystemPropertyUtils.PLACEHOLDER_PREFIX, - SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, true); + SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, + true, SystemPropertyUtils.ESCAPE_CHARACTER); /**