From 122734c247bf11551680265980f1b3c788cfe754 Mon Sep 17 00:00:00 2001 From: Stevo Slavic Date: Sat, 18 Feb 2012 17:44:04 +0100 Subject: [PATCH] Fix circular placeholder prevention 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 set in invalid state and mechanism broken. This fix makes sure value that is put in set is same one that is removed from it, and by this avoid false positives in reporting circular placeholder. Issue: SPR-5369 --- .../util/PropertyPlaceholderHelper.java | 7 ++++--- .../util/PropertyPlaceholderHelperTests.java | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) 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 4b1b2c37b12a..06a2406ae507 100644 --- a/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java +++ b/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java @@ -135,9 +135,10 @@ protected String parseStringValue( int endIndex = findPlaceholderEndIndex(buf, startIndex); if (endIndex != -1) { String placeholder = buf.substring(startIndex + this.placeholderPrefix.length(), endIndex); - if (!visitedPlaceholders.add(placeholder)) { + String originalPlaceholder = placeholder; + if (!visitedPlaceholders.add(originalPlaceholder)) { throw new IllegalArgumentException( - "Circular placeholder reference '" + placeholder + "' in property definitions"); + "Circular placeholder reference '" + originalPlaceholder + "' in property definitions"); } // Recursive invocation, parsing placeholders contained in the placeholder key. placeholder = parseStringValue(placeholder, placeholderResolver, visitedPlaceholders); @@ -173,7 +174,7 @@ else if (this.ignoreUnresolvablePlaceholders) { throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'"); } - visitedPlaceholders.remove(placeholder); + visitedPlaceholders.remove(originalPlaceholder); } else { startIndex = -1; 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 91f06b12f975..b19a0a178fed 100644 --- a/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java +++ b/spring-core/src/test/java/org/springframework/util/PropertyPlaceholderHelperTests.java @@ -65,6 +65,16 @@ public void testRecurseInPlaceholder() { props.setProperty("inner", "ar"); assertEquals("foo=bar", this.helper.replacePlaceholders(text, props)); + + // SPR-5369 + text = "${top}"; + props = new Properties(); + props.setProperty("top", "${child}+${child}"); + props.setProperty("child", "${${differentiator}.grandchild}"); + props.setProperty("differentiator", "first"); + props.setProperty("first.grandchild", "actualValue"); + + assertEquals("actualValue+actualValue", this.helper.replacePlaceholders(text, props)); } @Test