From 18006c72b014246946fd487159de7e133d173a17 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 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 --- .../util/PropertyPlaceholderHelper.java | 9 +++++---- .../util/PropertyPlaceholderHelperTests.java | 11 ++++++++++- 2 files changed, 15 insertions(+), 5 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 4b1b2c37b1..5c769fae5c 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-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -135,9 +135,10 @@ public class PropertyPlaceholderHelper { 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 @@ public class PropertyPlaceholderHelper { 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 91f06b12f9..58f8f14af3 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-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -65,6 +65,15 @@ public class PropertyPlaceholderHelperTests { props.setProperty("inner", "ar"); assertEquals("foo=bar", this.helper.replacePlaceholders(text, props)); + + 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