You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rz...@apache.org on 2021/03/04 09:16:10 UTC

[tomee] branch tomee-7.1.x updated: TOMEE-2968 - Fixes connection error when a password contains "}"

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 <13...@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());
     }