diff --git a/common/src/main/java/org/keycloak/common/util/StringPropertyReplacer.java b/common/src/main/java/org/keycloak/common/util/StringPropertyReplacer.java index 3e5916756e2..0b2b4fd36cc 100755 --- a/common/src/main/java/org/keycloak/common/util/StringPropertyReplacer.java +++ b/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) { diff --git a/common/src/test/java/org/keycloak/common/util/StringPropertyReplacerTest.java b/common/src/test/java/org/keycloak/common/util/StringPropertyReplacerTest.java new file mode 100644 index 00000000000..a1e61a247fb --- /dev/null +++ b/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 Marek Posolda + */ +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}")); + } + +} diff --git a/quarkus/runtime/src/test/java/org/keycloak/provider/quarkus/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/provider/quarkus/ConfigurationTest.java index 110b5f575e6..d2e1e3b4e15 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/provider/quarkus/ConfigurationTest.java +++ b/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); diff --git a/quarkus/runtime/src/test/resources/META-INF/keycloak.properties b/quarkus/runtime/src/test/resources/META-INF/keycloak.properties index f4bfd2d346c..366c0e337b6 100644 --- a/quarkus/runtime/src/test/resources/META-INF/keycloak.properties +++ b/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 \ No newline at end of file +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}}