KEYCLOAK-15632 Support nested expressions inside StringPropertyReplacer

common/src/main/java/org/keycloak/common/util/StringPropertyReplacer.java

@@ -117,6 +117,7 @@ public final class StringPropertyReplacer
         boolean properties = false;
         int state = NORMAL;
         int start = 0;
+        int openBracketsCount = 0;
         for (int i = 0; i < chars.length; ++i)
         {
             char c = chars[i];
@@ -125,18 +126,28 @@ public final class StringPropertyReplacer
             if (c == '$' && state != IN_BRACKET)
                 state = SEEN_DOLLAR;
 
-                // Open bracket immediatley after dollar
+            // Open bracket immediately after dollar
             else if (c == '{' && state == SEEN_DOLLAR)
             {
                 buffer.append(string.substring(start, i - 1));
                 state = IN_BRACKET;
                 start = i - 1;
+                openBracketsCount = 1;
             }
 
+            // Seeing open bracket after we already saw some open bracket without corresponding closed bracket. This causes "nested" expressions. For example ${foo:${bar}}
+            else if (c == '{' && state == IN_BRACKET)
+                openBracketsCount++;
+
             // No open bracket after dollar
             else if (state == SEEN_DOLLAR)
                 state = NORMAL;
 
+            // Seeing closed bracket, but we already saw more than one open bracket before. Hence "nested" expression is still not fully closed.
+            // For example expression ${foo:${bar}} is closed after the second closed bracket, not after the first closed bracket.
+            else if (c == '}' && state == IN_BRACKET && openBracketsCount > 1)
+                openBracketsCount--;
+
                 // Closed bracket after open bracket
             else if (c == '}' && state == IN_BRACKET)
             {

common/src/test/java/org/keycloak/common/util/StringPropertyReplacerTest.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.keycloak.common.util;
+
+import java.security.NoSuchAlgorithmException;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class StringPropertyReplacerTest {
+
+    @Test
+    public void testSystemProperties() throws NoSuchAlgorithmException {
+        System.setProperty("prop1", "val1");
+        Assert.assertEquals("foo-val1", StringPropertyReplacer.replaceProperties("foo-${prop1}"));
+
+        Assert.assertEquals("foo-def", StringPropertyReplacer.replaceProperties("foo-${prop2:def}"));
+        System.setProperty("prop2", "val2");
+        Assert.assertEquals("foo-val2", StringPropertyReplacer.replaceProperties("foo-${prop2:def}"));
+
+        // It looks for the property "prop3", then fallback to "prop4", then fallback to "prop5" and finally default value.
+        // This syntax is supported by Quarkus (and underlying Microprofile)
+        Assert.assertEquals("foo-def", StringPropertyReplacer.replaceProperties("foo-${prop3:${prop4:${prop5:def}}}"));
+        System.setProperty("prop5", "val5");
+        Assert.assertEquals("foo-val5", StringPropertyReplacer.replaceProperties("foo-${prop3:${prop4:${prop5:def}}}"));
+        System.setProperty("prop4", "val4");
+        Assert.assertEquals("foo-val4", StringPropertyReplacer.replaceProperties("foo-${prop3:${prop4:${prop5:def}}}"));
+        System.setProperty("prop3", "val3");
+        Assert.assertEquals("foo-val3", StringPropertyReplacer.replaceProperties("foo-${prop3:${prop4:${prop5:def}}}"));
+
+        // It looks for the property "prop6", then fallback to "prop7" then fallback to value "def" .
+        // This syntax is not supported by Quarkus (microprofile), however Wildfly probably supports this
+        Assert.assertEquals("foo-def", StringPropertyReplacer.replaceProperties("foo-${prop6,prop7:def}"));
+        System.setProperty("prop7", "val7");
+        Assert.assertEquals("foo-val7", StringPropertyReplacer.replaceProperties("foo-${prop6,prop7:def}"));
+        System.setProperty("prop6", "val6");
+        Assert.assertEquals("foo-val6", StringPropertyReplacer.replaceProperties("foo-${prop6,prop7:def}"));
+    }
+
+}

quarkus/runtime/src/test/java/org/keycloak/provider/quarkus/ConfigurationTest.java

@@ -31,8 +31,10 @@ import org.eclipse.microprofile.config.ConfigProvider;
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.hibernate.dialect.MariaDBDialect;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Test;
 import org.keycloak.Config;
+import org.keycloak.common.util.StringPropertyReplacer;
 import org.keycloak.configuration.KeycloakConfigSourceProvider;
 import org.keycloak.configuration.MicroProfileConfigProvider;
 
@@ -210,6 +212,28 @@ public class ConfigurationTest {
         assertEquals("jdbc:mariadb://localhost/keycloak?test=test&test1=test1", config.getConfigValue("quarkus.datasource.url").getValue());
     }
 
+    // KEYCLOAK-15632
+    @Test
+    public void testNestedDatabaseProperties() {
+        System.setProperty("kc.home.dir", "/tmp/kc/bin/../");
+        SmallRyeConfig config = createConfig();
+        assertEquals("jdbc:h2:file:/tmp/kc/bin/..//data/keycloakdb", config.getConfigValue("quarkus.datasource.foo").getValue());
+
+        Assert.assertEquals("foo-def-suffix", config.getConfigValue("quarkus.datasource.bar").getValue());
+
+        System.setProperty("kc.prop5", "val5");
+        config = createConfig();
+        Assert.assertEquals("foo-val5-suffix", config.getConfigValue("quarkus.datasource.bar").getValue());
+
+        System.setProperty("kc.prop4", "val4");
+        config = createConfig();
+        Assert.assertEquals("foo-val4", config.getConfigValue("quarkus.datasource.bar").getValue());
+
+        System.setProperty("kc.prop3", "val3");
+        config = createConfig();
+        Assert.assertEquals("foo-val3", config.getConfigValue("quarkus.datasource.bar").getValue());
+    }
+
     private Config.Scope initConfig(String... scope) {
         Config.init(new MicroProfileConfigProvider(createConfig()));
         return Config.scope(scope);

quarkus/runtime/src/test/resources/META-INF/keycloak.properties

@@ -7,4 +7,8 @@ quarkus.datasource.driver=org.h2.jdbcx.JdbcDataSource
 quarkus.datasource.url = jdbc:h2:file:${kc.home.dir:~}/data/keycloakdb;;AUTO_SERVER=TRUE
 quarkus.datasource.username = sa
 quarkus.datasource.password = keycloak
-quarkus.datasource.jdbc.transactions=xa
+quarkus.datasource.jdbc.transactions=xa
+
+# For test nested properties
+quarkus.datasource.foo = jdbc:h2:file:${kc.home.dir:${kc.db.url.path:~}}/data/keycloakdb
+quarkus.datasource.bar = foo-${kc.prop3:${kc.prop4:${kc.prop5:def}-suffix}}