Skip to content

Commit 18006c7

Browse files
sslaviccbeams
authored andcommitted
Fix circular placeholder prevention
A set of resolved placeholder references is used for circular placeholder prevention. For complex property definitions this mechanism would put property values with unresolved inner placeholder references in the set, but would try to remove property values with placeholders resolved, leaving the set in an invalid state and the mechanism broken. This fix makes sure that the value that is put in the set is same one that is removed from it, and by doing so avoids false positives in reporting circular placeholders. Issue: SPR-5369
1 parent f667db5 commit 18006c7

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

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

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2010 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -135,9 +135,10 @@ protected String parseStringValue(
135135
int endIndex = findPlaceholderEndIndex(buf, startIndex);
136136
if (endIndex != -1) {
137137
String placeholder = buf.substring(startIndex + this.placeholderPrefix.length(), endIndex);
138-
if (!visitedPlaceholders.add(placeholder)) {
138+
String originalPlaceholder = placeholder;
139+
if (!visitedPlaceholders.add(originalPlaceholder)) {
139140
throw new IllegalArgumentException(
140-
"Circular placeholder reference '" + placeholder + "' in property definitions");
141+
"Circular placeholder reference '" + originalPlaceholder + "' in property definitions");
141142
}
142143
// Recursive invocation, parsing placeholders contained in the placeholder key.
143144
placeholder = parseStringValue(placeholder, placeholderResolver, visitedPlaceholders);
@@ -173,7 +174,7 @@ else if (this.ignoreUnresolvablePlaceholders) {
173174
throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'");
174175
}
175176

176-
visitedPlaceholders.remove(placeholder);
177+
visitedPlaceholders.remove(originalPlaceholder);
177178
}
178179
else {
179180
startIndex = -1;

spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2009 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -65,6 +65,15 @@ public void testRecurseInPlaceholder() {
6565
props.setProperty("inner", "ar");
6666

6767
assertEquals("foo=bar", this.helper.replacePlaceholders(text, props));
68+
69+
text = "${top}";
70+
props = new Properties();
71+
props.setProperty("top", "${child}+${child}");
72+
props.setProperty("child", "${${differentiator}.grandchild}");
73+
props.setProperty("differentiator", "first");
74+
props.setProperty("first.grandchild", "actualValue");
75+
76+
assertEquals("actualValue+actualValue", this.helper.replacePlaceholders(text, props));
6877
}
6978

7079
@Test

0 commit comments

Comments
 (0)