[GitHub] [tomee] rzo1 commented on a change in pull request #763: TOMEE-2968: Postgres connection error when a password contains "}"

[GitBox <gi...@apache.org>]

rzo1 commented on a change in pull request #763:
URL: https://github.com/apache/tomee/pull/763#discussion_r585708625



##########
File path: container/openejb-core/src/main/java/org/apache/openejb/util/PropertyPlaceHolderHelper.java
##########
@@ -53,33 +55,34 @@ public static void reset() {
     }
 
     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:")) {
             value = value.substring(5);
         }
-        return decryptIfNeeded(value.replace(PREFIX, "").replace(SUFFIX, ""), true);
+
+        // no key defined and no escape sequence found -> substitute to key instead
+        if (!(raw.contains(ESCAPE_SEQUENCE) || value.contains(ESCAPE_SEQUENCE))

Review comment:
       Thx @rmannibucau  for the follow up!
   
   Yes - testing with `contains` is a simplification and neglects some weirdo cases... and yes: it would require char, position + offsets to properly test and replace the closing and opening brackets. 
   
   Thus, I like the idea of using `PropertiesLookup#lookup` and returning the key there. The parsing "magic" (char, position, offsets) is already done in the substitutor, so I guess fallbacking there is a good option. 
   
   I will take a look at it.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org