This is an automated email from the ASF dual-hosted git repository. rzo1 pushed a commit to branch tomee-7.1.x in repository https://gitbox.apache.org/repos/asf/tomee.git
The following commit(s) were added to refs/heads/tomee-7.1.x by this push: new e12ece9 TOMEE-2968 - Fixes connection error when a password contains "}" e12ece9 is described below commit e12ece938f4cef40f19f9280e45fe95c249e5106 Author: Richard Zowalla <13417392+r...@users.noreply.github.com> AuthorDate: Tue Feb 23 09:51:14 2021 +0100 TOMEE-2968 - Fixes connection error when a password contains "}" (cherry picked from commit afab0284592ed507911be84d8512f6f49567968d) --- .../ProvisioningClassLoaderConfigurer.java | 2 +- .../openejb/util/PropertyPlaceHolderHelper.java | 88 ++++++---- .../openejb/util/PropertyPlaceHolderTest.java | 189 +++++++++++++++++++++ .../jdbc/TomcatDataSourceConfigurationTest.java | 4 +- 4 files changed, 251 insertions(+), 32 deletions(-) diff --git a/container/openejb-core/src/main/java/org/apache/openejb/classloader/ProvisioningClassLoaderConfigurer.java b/container/openejb-core/src/main/java/org/apache/openejb/classloader/ProvisioningClassLoaderConfigurer.java index 6d26919..3c4aa8b 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/classloader/ProvisioningClassLoaderConfigurer.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/classloader/ProvisioningClassLoaderConfigurer.java @@ -80,7 +80,7 @@ public class ProvisioningClassLoaderConfigurer implements ClassLoaderConfigurer String line; while ((line = reader.readLine()) != null) { - line = PropertyPlaceHolderHelper.SUBSTITUTOR.replace(line.trim()); + line = PropertyPlaceHolderHelper.replace(line.trim()); if (line.startsWith("#") || line.isEmpty()) { continue; } diff --git a/container/openejb-core/src/main/java/org/apache/openejb/util/PropertyPlaceHolderHelper.java b/container/openejb-core/src/main/java/org/apache/openejb/util/PropertyPlaceHolderHelper.java index cd6ae90..9418abb 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/util/PropertyPlaceHolderHelper.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/util/PropertyPlaceHolderHelper.java @@ -31,17 +31,30 @@ import java.util.Properties; public final class PropertyPlaceHolderHelper { private static final String PREFIX = "${"; private static final String SUFFIX = "}"; + private static final String VALUE_DELIMITER; + private static final Properties CACHE = new Properties(); - private static final PropertiesLookup RESOLVER = new PropertiesLookup(); - public static final StrSubstitutor SUBSTITUTOR = new StrSubstitutor(RESOLVER); + private static final PropertiesLookup RESOLVER_TO_NULL_IF_MISSING = new PropertiesLookup(false); + private static final PropertiesLookup RESOLVER_TO_KEY_IF_MISSING = new PropertiesLookup(true); + + private static final StrSubstitutor DEFAULT_SUBSTITUTOR = new StrSubstitutor(RESOLVER_TO_KEY_IF_MISSING); + + private static final StrSubstitutor VALUE_DELIMITER_SUBSTITUTOR = new StrSubstitutor(RESOLVER_TO_NULL_IF_MISSING); static { - SUBSTITUTOR.setEnableSubstitutionInVariables(true); - SUBSTITUTOR.setValueDelimiter(JavaSecurityManagers.getSystemProperty("openejb.placehodler.delimiter", ":-")); // default one of [lang3] + VALUE_DELIMITER = JavaSecurityManagers.getSystemProperty("openejb.placehodler.delimiter", ":-"); // default one of [lang3] + + DEFAULT_SUBSTITUTOR.setEnableSubstitutionInVariables(true); + DEFAULT_SUBSTITUTOR.setValueDelimiter(VALUE_DELIMITER); + + VALUE_DELIMITER_SUBSTITUTOR.setPreserveEscapes(true); + VALUE_DELIMITER_SUBSTITUTOR.setEnableSubstitutionInVariables(true); + VALUE_DELIMITER_SUBSTITUTOR.setValueDelimiter(VALUE_DELIMITER); } - public static final String CIPHER_PREFIX = "cipher:"; + private static final String CIPHER_PREFIX = "cipher:"; + private static final String JAVA_PREFIX = "java:"; private PropertyPlaceHolderHelper() { // no-op @@ -49,37 +62,33 @@ public final class PropertyPlaceHolderHelper { public static void reset() { CACHE.clear(); - RESOLVER.reload(); + RESOLVER_TO_NULL_IF_MISSING.reload(); + RESOLVER_TO_KEY_IF_MISSING.reload(); } public static String simpleValue(final String raw) { - if (raw == null) { - return null; - } - if (!raw.contains(PREFIX) || !raw.contains(SUFFIX)) { - return String.class.cast(decryptIfNeeded(raw.replace(PREFIX, "").replace(SUFFIX, ""), false)); - } - - String value = SUBSTITUTOR.replace(raw); - if (!value.equals(raw) && value.startsWith("java:")) { - value = value.substring(5); - } - return String.class.cast(decryptIfNeeded(value.replace(PREFIX, "").replace(SUFFIX, ""), false)); + return String.class.cast(simpleValueAsStringOrCharArray(raw, false)); } public static Object simpleValueAsStringOrCharArray(final String raw) { + return simpleValueAsStringOrCharArray(raw, true); + } + + private static Object simpleValueAsStringOrCharArray(final String raw, boolean acceptCharArray) { if (raw == null) { return null; } if (!raw.contains(PREFIX) || !raw.contains(SUFFIX)) { - return decryptIfNeeded(raw.replace(PREFIX, "").replace(SUFFIX, ""), true); + return decryptIfNeeded(raw, acceptCharArray); } - String value = SUBSTITUTOR.replace(raw); - if (!value.equals(raw) && value.startsWith("java:")) { + String value = replace(raw); + + if (!value.equals(raw) && value.startsWith(JAVA_PREFIX)) { value = value.substring(5); } - return decryptIfNeeded(value.replace(PREFIX, "").replace(SUFFIX, ""), true); + + return decryptIfNeeded(value, acceptCharArray); } private static Object decryptIfNeeded(final String replace, final boolean acceptCharArray) { @@ -104,21 +113,21 @@ public final class PropertyPlaceHolderHelper { return replace; } - public static String value(final String aw) { - if (aw == null) { + public static String value(final String raw) { + if (raw == null) { return null; } - if (!aw.contains(PREFIX) || !aw.contains(SUFFIX)) { - return String.class.cast(decryptIfNeeded(aw, false)); + if (!raw.contains(PREFIX) || !raw.contains(SUFFIX)) { + return String.class.cast(decryptIfNeeded(raw, false)); } - String value = CACHE.getProperty(aw); + String value = CACHE.getProperty(raw); if (value != null) { return value; } - value = simpleValue(aw); - CACHE.setProperty(aw, value); + value = simpleValue(raw); + CACHE.setProperty(raw, value); return value; } @@ -154,9 +163,27 @@ public final class PropertyPlaceHolderHelper { props.putAll(toUpdate); } + public static String replace(final String raw) { + if(raw == null) { + return null; + } + + if(raw.contains(VALUE_DELIMITER)) { + return DEFAULT_SUBSTITUTOR.replace(VALUE_DELIMITER_SUBSTITUTOR.replace(raw)); + } else { + return DEFAULT_SUBSTITUTOR.replace(raw); + } + } + private static class PropertiesLookup extends StrLookup<Object> { private static final Map<String, String> ENV = System.getenv(); + private final boolean resolveToKey; + + public PropertiesLookup(boolean resolveToKey) { + this.resolveToKey = resolveToKey; + } + @Override public synchronized String lookup(final String key) { String value = SystemInstance.get().getProperties().getProperty(key); @@ -169,11 +196,12 @@ public final class PropertyPlaceHolderHelper { return value; } - return null; + return this.resolveToKey ? key : null; } public synchronized void reload() { //no-op } } + } diff --git a/container/openejb-core/src/test/java/org/apache/openejb/util/PropertyPlaceHolderTest.java b/container/openejb-core/src/test/java/org/apache/openejb/util/PropertyPlaceHolderTest.java index b23ff0a..869920b 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/util/PropertyPlaceHolderTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/util/PropertyPlaceHolderTest.java @@ -76,4 +76,193 @@ public class PropertyPlaceHolderTest { final String foo = PropertyPlaceHolderHelper.simpleValue("jdbc://${PropertyPlaceHolderTest1}/${PropertyPlaceHolderTest2}"); assertEquals("jdbc://uno/due", foo); } + + /* + * Start of tests related to TOMEE-2968 + */ + + @Test + public void singleCurlyBrace() { + final String foo = PropertyPlaceHolderHelper.simpleValue("tiger...}"); + assertEquals("tiger...}", foo); + } + + @Test + public void singleCurlyBraceAsStringOrCharArray() { + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("tiger...}"); + assertEquals("tiger...}", foo); + } + + @Test + public void singleCurlyBraceWithSubstitution() { + SystemInstance.get().setProperty("PropertyPlaceHolderTest1", "tiger...}"); + SystemInstance.get().setProperty("PropertyPlaceHolderTest2", "due"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${PropertyPlaceHolderTest1}/${PropertyPlaceHolderTest2}"); + assertEquals("tiger...}/due", foo); + } + + @Test + public void singleCurlyBraceAsStringOrCharArrayWithSubstitution() { + SystemInstance.get().setProperty("PropertyPlaceHolderTest1", "tiger...}"); + SystemInstance.get().setProperty("PropertyPlaceHolderTest2", "due"); + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${PropertyPlaceHolderTest1}/${PropertyPlaceHolderTest2}"); + assertEquals("tiger...}/due", foo); + } + + @Test + public void singleCurlyBraceAfterVariableReplacementGroup() { + SystemInstance.get().setProperty("foo", "bar"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${foo}}"); + assertEquals("bar}", foo); + } + + @Test + public void singleCurlyBraceAsStringOrCharArrayAfterVariableReplacementGroup() { + SystemInstance.get().setProperty("foo", "bar"); + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${foo}}"); + assertEquals("bar}", foo); + } + + @Test + public void escapeCharacterSubstitutionSkip() { + SystemInstance.get().setProperty("{foo}", "bar"); + //$ is treated as an escape character, thus skipping substitution of variable with key = '{foo}'. + final String foo = PropertyPlaceHolderHelper.simpleValue("$${{foo}}"); + assertEquals("${{foo}}", foo); + } + + @Test + public void escapeCharacterSubstitutionSkipAsStringOrCharArray() { + SystemInstance.get().setProperty("{foo}", "bar"); + //$ is treated as an escape character, thus skipping substitution of variable with key = '{foo}'. + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("$${{foo}}"); + assertEquals("${{foo}}", foo); + } + + @Test + public void nestingSubstitution() { + SystemInstance.get().setProperty("foo", "abc}"); + SystemInstance.get().setProperty("abc}", "foo"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${${foo}}"); + assertEquals("foo", foo); + } + + @Test + public void nestingSubstitutionAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "abc}"); + SystemInstance.get().setProperty("abc}", "foo"); + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${${foo}}"); + assertEquals("foo", foo); + } + + @Test + public void escapedNestingWithNonExistentKey() { + SystemInstance.get().setProperty("bar", "bar"); + //'$${foo}' is substituted to '${foo}', which does not exist. `${foo}` is thus substituted to its key `foo`. + final String foo = PropertyPlaceHolderHelper.simpleValue("${$${foo}}"); + assertEquals("foo", foo); + } + + @Test + public void escapedNestingWithNonExistentKeyAsStringOrCharArray() { + SystemInstance.get().setProperty("bar", "bar"); + //'$${foo}' is substituted to '${foo}', which does not exist. `${foo}` is thus substituted to its key `foo`. + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${$${foo}}"); + assertEquals("foo", foo); + } + + @Test + public void combinedNestingWithNonExistentKey() { + SystemInstance.get().setProperty("foo", "bar"); + //variable for key 'bar' does not exist + final String foo = PropertyPlaceHolderHelper.simpleValue("${foo}-${${bar}}"); + assertEquals("bar-bar", foo); + } + + @Test + public void combinedNestingWithNonExistentKeyAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "bar"); + //variable for key 'bar' does not exist + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${foo}-${${bar}}"); + assertEquals("bar-bar", foo); + } + + @Test + public void nestedMultipleReplacementsWithClosingCurlyBraces() { + SystemInstance.get().setProperty("foo", "abc}"); + SystemInstance.get().setProperty("abc}", "food"); + SystemInstance.get().setProperty("bar", "yammie"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${bar}/${${foo}}"); + assertEquals("yammie/food", foo); + } + + @Test + public void nestedMultipleReplacementsWithClosingCurlyBracesAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "abc}"); + SystemInstance.get().setProperty("abc}", "food"); + SystemInstance.get().setProperty("bar", "yammie"); + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${bar}/${${foo}}"); + assertEquals("yammie/food", foo); + } + + @Test + public void nestedMissingPropertiesKeyAsStringOrCharArray() { + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${${foo}.bar}"); + assertEquals("foo.bar", foo); + } + + @Test + public void nestedMissingPropertiesKey() { + final String foo = PropertyPlaceHolderHelper.simpleValue("${${foo}.bar}"); + assertEquals("foo.bar", foo); + } + + @Test + public void nestedPropertiesKeyAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("bar.bar", "val"); + final Object foo = PropertyPlaceHolderHelper.simpleValueAsStringOrCharArray("${${foo}.bar}"); + assertEquals("val", foo); + } + + @Test + public void nestedPropertiesKey() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("bar.bar", "val"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${${foo}.bar}"); + assertEquals("val", foo); + } + + @Test + public void escapedPropertyKey() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("bar", "val"); + final String foo = PropertyPlaceHolderHelper.simpleValue("$${foo}.${bar}"); + assertEquals("${foo}.val", foo); + } + + @Test + public void escapedPropertyKeyAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("bar", "val"); + final Object foo = PropertyPlaceHolderHelper.simpleValue("$${foo}.${bar}"); + assertEquals("${foo}.val", foo); + } + + @Test + public void escapedPropertyKeyUnknownAndKnownVar() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("known", "bar"); + final String foo = PropertyPlaceHolderHelper.simpleValue("${bar}.$${foo}.${known}"); + assertEquals("bar.${foo}.bar", foo); + } + + @Test + public void escapedPropertyKeyUnknownAndKnownVarAsStringOrCharArray() { + SystemInstance.get().setProperty("foo", "bar"); + SystemInstance.get().setProperty("known", "bar"); + final Object foo = PropertyPlaceHolderHelper.simpleValue("${bar}.$${foo}.${known}"); + assertEquals("bar.${foo}.bar", foo); + } + } diff --git a/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatDataSourceConfigurationTest.java b/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatDataSourceConfigurationTest.java index f2f8892..42e9d24 100644 --- a/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatDataSourceConfigurationTest.java +++ b/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatDataSourceConfigurationTest.java @@ -55,11 +55,12 @@ public class TomcatDataSourceConfigurationTest { .p(prefix + ".MaxWait", "5000") .p(prefix + ".MinEvictableIdleTimeMillis", "7200000") .p(prefix + ".TimeBetweenEvictionRuns", "7300000") + .p(prefix + ".password", "tiger...}") .build(); } /* - * TOMEE-2125 + * TOMEE-2125 and TOMEE-2968 */ @Test public void testPoolConfiguration() { @@ -77,6 +78,7 @@ public class TomcatDataSourceConfigurationTest { assertEquals(5000, poolConfig.getMaxWait()); assertEquals(7200000, poolConfig.getMinEvictableIdleTimeMillis()); assertEquals(7300000, poolConfig.getTimeBetweenEvictionRunsMillis()); + assertEquals("tiger...}", poolConfig.getPassword()); }