fix: corrects cli arg stringification (#34156)

closes: #34155

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java

@@ -53,7 +53,7 @@ public final class ShowConfig extends AbstractCommand implements Runnable {
 
     public static final String NAME = "show-config";
     private static final List<String> ignoredPropertyKeys = List.of(
-            "kc.config.args", "kc.show.config", "kc.profile", "kc.quarkus-properties-enabled", "kc.home.dir");
+            "kc.show.config", "kc.profile", "kc.quarkus-properties-enabled", "kc.home.dir");
 
     @Parameters(
             paramLabel = "filter",

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java

@@ -21,6 +21,7 @@ import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_SHORT_PREFIX;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PART_SEPARATOR_CHAR;
 import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -29,6 +30,8 @@ import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import io.smallrye.config.ConfigValue;
 import io.smallrye.config.PropertiesConfigSource;
@@ -41,9 +44,6 @@ import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
  * <p>A configuration source for mapping configuration arguments to their corresponding properties so that they can be recognized
  * when building and running the server.
  *
- * <p>The mapping is based on the system property {@code kc.config.args}, where the value is a comma-separated list of
- * the arguments passed during build or runtime. E.g: "--http-enabled=true,--http-port=8180,--database-vendor=postgres".
- *
  * <p>Each argument is going to be mapped to its corresponding configuration property by prefixing the key with the {@link MicroProfileConfigProvider#NS_KEYCLOAK} namespace.
  */
 public class ConfigArgsConfigSource extends PropertiesConfigSource {
@@ -52,9 +52,8 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource {
 
     public static final Set<String> SHORT_OPTIONS_ACCEPTING_VALUE = Set.of(Main.PROFILE_SHORT_NAME, Main.CONFIG_FILE_SHORT_NAME);
 
-    public static final String CLI_ARGS = "kc.config.args";
+    private static final String CLI_ARGS = "kc.config.args";
     public static final String NAME = "CliConfigSource";
-    private static final String ARG_SEPARATOR = ";;";
     private static final Pattern ARG_KEY_VALUE_SPLIT = Pattern.compile("=");
 
     protected ConfigArgsConfigSource() {
@@ -62,7 +61,8 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource {
     }
 
     public static void setCliArgs(String... args) {
-        System.setProperty(CLI_ARGS, String.join(ARG_SEPARATOR, args));
+        System.setProperty(CLI_ARGS,
+                Stream.of(args).map(arg -> arg.replaceAll(",", ",,")).collect(Collectors.joining(", ")));
     }
 
     /**
@@ -73,22 +73,36 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource {
      * @return the invoked command from the CLI, or empty List if not set.
      */
     public static List<String> getAllCliArgs() {
-        if(System.getProperty(CLI_ARGS) == null) {
+        String args = System.getProperty(CLI_ARGS);
+        if(args == null) {
             return Collections.emptyList();
         }
-
+        
-        return List.of(System.getProperty(CLI_ARGS).split(ARG_SEPARATOR));
+        List<String> result = new ArrayList<String>();
-    }
+        boolean escaped = false;
-
+        StringBuilder arg = new StringBuilder();
-    private static String getRawConfigArgs() {
+        for (int i = 0; i < args.length(); i++) {
-        String args = System.getProperty(CLI_ARGS);
+            char c = args.charAt(i);
-
+            if (c == ',') {
-        if (args != null) {
+                if (escaped) {
-            return args;
+                    arg.append(c);
+                }
+                escaped = !escaped;
+            } else if (c == ' ') {
+                if (escaped) {
+                    result.add(arg.toString());
+                    arg.setLength(0);
+                    escaped = false;
+                } else {
+                    arg.append(c);
+                }
+            } else {
+                arg.append(c);
+            }
         }
-
+        result.add(arg.toString());
-        // make sure quarkus.args property is properly formatted
+        
-        return String.join(ARG_SEPARATOR, System.getProperty("quarkus.args", "").split(" "));
+        return result;
     }
 
     @Override
@@ -103,12 +117,6 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource {
     }
 
     private static Map<String, String> parseArguments() {
-        String rawArgs = getRawConfigArgs();
-
-        if (rawArgs == null || "".equals(rawArgs.trim())) {
-            return Collections.emptyMap();
-        }
-
         Map<String, String> properties = new HashMap<>();
 
         parseConfigArgs(getAllCliArgs(), new BiConsumer<String, String>() {

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java

@@ -86,7 +86,6 @@ public final class PropertyMappers {
 
         return isBuildTimeProperty
                 && !"kc.version".equals(name)
-                && !ConfigArgsConfigSource.CLI_ARGS.equals(name)
                 && !"kc.home.dir".equals(name)
                 && !"kc.config.file".equals(name)
                 && !org.keycloak.common.util.Environment.PROFILE.equals(name)

quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java

@@ -40,5 +40,17 @@ public class ConfigArgsConfigSourceTest {
         ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else", "other-value"), (key, value) -> values.add(key+'='+value), ignored -> {});
         assertEquals(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else=other-value"), values);
     }
+    
+    @Test
+    public void testArgEndingInSemiColon() {
+        ConfigArgsConfigSource.setCliArgs("--some=thing;", "--else=value");
+        assertEquals(Arrays.asList("--some=thing;", "--else=value"), ConfigArgsConfigSource.getAllCliArgs());
+    }
+    
+    @Test
+    public void testArgCommas() {
+        ConfigArgsConfigSource.setCliArgs("--some=,,,", "--else=,");
+        assertEquals(Arrays.asList("--some=,,,", "--else=,"), ConfigArgsConfigSource.getAllCliArgs());
+    }
 
 }

quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/AbstractConfigurationTest.java

@@ -26,6 +26,7 @@ import org.eclipse.microprofile.config.ConfigProvider;
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.junit.After;
 import org.keycloak.Config;
+import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
 import org.keycloak.quarkus.runtime.configuration.Configuration;
 import org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider;
 import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
@@ -111,6 +112,7 @@ public abstract class AbstractConfigurationTest {
 
         SmallRyeConfigProviderResolver.class.cast(ConfigProviderResolver.instance()).releaseConfig(ConfigProvider.getConfig());
         PropertyMappers.reset();
+        ConfigArgsConfigSource.setCliArgs();
     }
 
     protected Config.Scope initConfig(String... scope) {

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

@@ -21,7 +21,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.keycloak.quarkus.runtime.Environment.isWindows;
-import static org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource.CLI_ARGS;
 
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
@@ -177,13 +176,13 @@ public class ConfigurationTest extends AbstractConfigurationTest {
         assertEquals(1, config.getPropertyNames().size());
         assertEquals("JKS", config.get("type"));
 
-        System.getProperties().remove(CLI_ARGS);
+        ConfigArgsConfigSource.setCliArgs();
         System.setProperty("kc.spi-client-registration-openid-connect-static-jwk-url", "http://c.jwk.url");
         config = initConfig("client-registration", "openid-connect");
         assertEquals(1, config.getPropertyNames().size());
         assertEquals("http://c.jwk.url", config.get("static-jwk-url"));
 
-        System.getProperties().remove(CLI_ARGS);
+        ConfigArgsConfigSource.setCliArgs();
         System.getProperties().remove("kc.spi-client-registration-openid-connect-static-jwk-url");
         putEnvVar("KC_SPI_CLIENT_REGISTRATION_OPENID_CONNECT_STATIC_JWK_URL", "http://c.jwk.url/from-env");
         config = initConfig("client-registration", "openid-connect");
@@ -390,7 +389,7 @@ public class ConfigurationTest extends AbstractConfigurationTest {
         ConfigArgsConfigSource.setCliArgs("--db=mssql", "--db-url=jdbc:sqlserver://localhost/keycloak");
         System.setProperty("kc.db-driver", "com.microsoft.sqlserver.jdbc.SQLServerDriver");
         System.setProperty("kc.transaction-xa-enabled", "false");
-        assertTrue(System.getProperty(CLI_ARGS, "").contains("mssql"));
+        assertTrue(ConfigArgsConfigSource.getAllCliArgs().contains("--db=mssql"));
         SmallRyeConfig config = createConfig();
         assertEquals("jdbc:sqlserver://localhost/keycloak", config.getConfigValue("quarkus.datasource.jdbc.url").getValue());
         assertEquals("mssql", config.getConfigValue("quarkus.datasource.db-kind").getValue());
@@ -407,14 +406,14 @@ public class ConfigurationTest extends AbstractConfigurationTest {
     @Test
     public void testTransactionTypeChangesDriver() {
         ConfigArgsConfigSource.setCliArgs("--db=mssql", "--transaction-xa-enabled=false");
-        assertTrue(System.getProperty(CLI_ARGS, "").contains("mssql"));
+        assertTrue(ConfigArgsConfigSource.getAllCliArgs().contains("--db=mssql"));
 
         SmallRyeConfig jtaEnabledConfig = createConfig();
         assertEquals("com.microsoft.sqlserver.jdbc.SQLServerDriver", jtaEnabledConfig.getConfigValue("quarkus.datasource.jdbc.driver").getValue());
         assertEquals("enabled", jtaEnabledConfig.getConfigValue("quarkus.datasource.jdbc.transactions").getValue());
 
         ConfigArgsConfigSource.setCliArgs("--db=mssql", "--transaction-xa-enabled=true");
-        assertTrue(System.getProperty(CLI_ARGS, "").contains("mssql"));
+        assertTrue(ConfigArgsConfigSource.getAllCliArgs().contains("--db=mssql"));
         SmallRyeConfig xaConfig = createConfig();
 
         assertEquals("com.microsoft.sqlserver.jdbc.SQLServerXADataSource", xaConfig.getConfigValue("quarkus.datasource.jdbc.driver").getValue());