modification of kc.sh to remove param eval (#22585)

* test * modification of kc.sh to remove eval of env/args Closes #22337 --------- Co-authored-by: rmartinc <rmartinc@redhat.com>
2023-10-12 11:10:53 -04:00 · 2023-10-12 11:10:53 -04:00 · 478ceb0b34
commit 478ceb0b34
parent 197b39492e
10 changed files with 64 additions and 36 deletions
--- a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc
+++ b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc
@ -86,3 +86,18 @@ In this release, it was completely removed, and it is no longer supported.

 When executing the `start` command, the server is automatically built based on the configuration.
 In order to prevent this behavior, set the `--optimized` flag.
+
+
+= kc.sh and shell metacharacters
+
+The kc.sh no longer uses an additional shell eval on parameters and the environment variables JAVA_OPTS_APPEND and JAVA_ADD_OPENS, thus the continued use of double escaping/quoting will result in the parameter being misunderstood.  For example instead of 
+
+```
+bin/kc.sh start --db postgres --db-username keycloak --db-url "\"jdbc:postgresql://localhost:5432/keycloak?ssl=false&connectTimeout=30\"" --db-password keycloak --hostname localhost
+```
+
+Use a single escape:
+
+```
+bin/kc.sh start --db postgres --db-username keycloak --db-url "jdbc:postgresql://localhost:5432/keycloak?ssl=false&connectTimeout=30" --db-password keycloak --hostname localhost
+```
--- a/quarkus/container/Dockerfile
+++ b/quarkus/container/Dockerfile
@ -17,7 +17,7 @@ RUN mv /tmp/keycloak/keycloak-* /opt/keycloak && mkdir -p /opt/keycloak/data
 RUN chmod -R g+rwX /opt/keycloak

 ADD ubi-null.sh /tmp/
-RUN bash /tmp/ubi-null.sh java-17-openjdk-headless glibc-langpack-en
+RUN bash /tmp/ubi-null.sh java-17-openjdk-headless glibc-langpack-en findutils

 FROM registry.access.redhat.com/ubi9-micro
 ENV LANG en_US.UTF-8
--- a/quarkus/dist/src/main/content/bin/kc.bat
+++ b/quarkus/dist/src/main/content/bin/kc.bat
@ -28,7 +28,7 @@ set CONFIG_ARGS=

 rem Read command-line args, the ~ removes the quotes from the parameter
 :READ-ARGS
-set KEY=%~1
+set "KEY=%~1"
 if "%KEY%" == "" (
    goto MAIN
 )
@ -55,14 +55,14 @@ if not "%KEY:~0,2%"=="--" if "%KEY:~0,2%"=="-D" (
  shift
 )
 if not "%KEY:~0,2%"=="--" if not "%KEY:~0,1%"=="-" (
-  set CONFIG_ARGS=%CONFIG_ARGS% %KEY%
+  set CONFIG_ARGS=%CONFIG_ARGS% %1
 )
 if not "%KEY:~0,2%"=="-D" (
  if "%KEY:~0,1%"=="-" (
      if "%~2"=="" (
-        set CONFIG_ARGS=%CONFIG_ARGS% %KEY%
+        set CONFIG_ARGS=%CONFIG_ARGS% %1
      ) else (
-        set CONFIG_ARGS=%CONFIG_ARGS% %KEY% %2%
+        set CONFIG_ARGS=%CONFIG_ARGS% %1 %2
      )
      shift
  )
@ -155,7 +155,7 @@ if not errorlevel == 1 (

 if "%PRINT_ENV%" == "true" (
  echo "Using JAVA_OPTS: %JAVA_OPTS%"
-  echo "Using JAVA_RUN_OPTS: %JAVA_RUN_OPTS%"
+  echo "Using JAVA_RUN_OPTS: !JAVA_RUN_OPTS!"
 )

 set START_SERVER=true
@ -163,17 +163,17 @@ set START_SERVER=true
 if "!CONFIG_ARGS:%OPTIMIZED_OPTION%=!"=="!CONFIG_ARGS!" if "!CONFIG_ARGS:%BUILD_OPTION%=!"=="!CONFIG_ARGS!" if "!CONFIG_ARGS:%HELP_LONG_OPTION%=!"=="!CONFIG_ARGS!" if "%IS_HELP_SHORT%" == "false" (
    setlocal enabledelayedexpansion

-    "%JAVA%" -Dkc.config.build-and-exit=true %JAVA_RUN_OPTS%
+    "%JAVA%" -Dkc.config.build-and-exit=true !JAVA_RUN_OPTS!

    if not !errorlevel! == 0 (
        set START_SERVER=false
    )

-    set JAVA_RUN_OPTS=-Dkc.config.built=true %JAVA_RUN_OPTS%
+    set JAVA_RUN_OPTS=-Dkc.config.built=true !JAVA_RUN_OPTS!
 )

 if "%START_SERVER%" == "true" (
-    "%JAVA%" %JAVA_RUN_OPTS%
+    "%JAVA%" !JAVA_RUN_OPTS!
 )

 :END
--- a/quarkus/dist/src/main/content/bin/kc.sh
+++ b/quarkus/dist/src/main/content/bin/kc.sh
@ -44,8 +44,11 @@ DEBUG_MODE="${DEBUG:-false}"
 DEBUG_PORT="${DEBUG_PORT:-8787}"
 DEBUG_SUSPEND="${DEBUG_SUSPEND:-n}"

-CONFIG_ARGS=${CONFIG_ARGS:-""}
+esceval() {
+    printf '%s\n' "$1" | sed "s/'/'\\\\''/g; 1 s/^/'/; $ s/$/'/"
+}

+PRE_BUILD=true
 while [ "$#" -gt 0 ]
 do
    case "$1" in
@ -61,10 +64,16 @@ do
          break
          ;;
      *)
+          OPT=$(esceval "$1")
          case "$1" in
            start-dev) CONFIG_ARGS="$CONFIG_ARGS --profile=dev $1";;
-            -D*) SERVER_OPTS="$SERVER_OPTS $1";;
-            *) CONFIG_ARGS="$CONFIG_ARGS $1";;
+            -D*) SERVER_OPTS="$SERVER_OPTS ${OPT}";;
+            *) case "$1" in
+                 --optimized | --help | --help-all | -h) PRE_BUILD=false;;
+                 build) if [ -z "$CONFIG_ARGS" ]; then PRE_BUILD=false; fi;;
+               esac 
+               CONFIG_ARGS="$CONFIG_ARGS ${OPT}"
+               ;;
          esac
          ;;
    esac
@ -115,19 +124,25 @@ if [ "$DEBUG_MODE" = "true" ]; then
    fi
 fi

-JAVA_RUN_OPTS="$JAVA_OPTS $SERVER_OPTS -cp $CLASSPATH_OPTS io.quarkus.bootstrap.runner.QuarkusEntryPoint ${CONFIG_ARGS#?}"
+esceval_args() {
+  while IFS= read -r entry; do
+    result="$result $(esceval "$entry")"
+  done
+  echo $result
+}
+
+JAVA_RUN_OPTS=$(echo "$JAVA_OPTS" | xargs printf '%s\n' | esceval_args)
+
+JAVA_RUN_OPTS="$JAVA_RUN_OPTS $SERVER_OPTS -cp $CLASSPATH_OPTS io.quarkus.bootstrap.runner.QuarkusEntryPoint ${CONFIG_ARGS#?}"

 if [ "$PRINT_ENV" = "true" ]; then
  echo "Using JAVA_OPTS: $JAVA_OPTS"
  echo "Using JAVA_RUN_OPTS: $JAVA_RUN_OPTS"
 fi

-case "$CONFIG_ARGS" in
-  " build"* | *--optimized* | *-h | *--help*) ;;
-  *)
+if [ "$PRE_BUILD" = "true" ]; then
  eval "'$JAVA'" -Dkc.config.build-and-exit=true $JAVA_RUN_OPTS || exit $?
  JAVA_RUN_OPTS="-Dkc.config.built=true $JAVA_RUN_OPTS"
-    ;;
-esac
+fi

 eval exec "'$JAVA'" $JAVA_RUN_OPTS
--- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/CustomLegacyUserProviderDistTest.java
+++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/CustomLegacyUserProviderDistTest.java
@ -23,7 +23,6 @@ import org.keycloak.it.junit5.extension.DistributionTest;
 import org.keycloak.it.junit5.extension.LegacyStore;
 import org.keycloak.it.junit5.extension.RawDistOnly;
 import org.keycloak.it.junit5.extension.TestProvider;
-import com.acme.provider.legacy.jpa.entity.CustomLegacyJpaEntityProvider;
 import com.acme.provider.legacy.jpa.user.CustomLegacyUserProvider;

 import io.quarkus.test.junit.main.Launch;
@ -36,7 +35,7 @@ public class CustomLegacyUserProviderDistTest {

    @Test
    @TestProvider(CustomLegacyUserProvider.class)
-    @Launch({ "start-dev", "--spi-user-provider=custom_jpa --spi-user-jpa-enabled=false" })
+    @Launch({ "start-dev", "--spi-user-provider=custom_jpa", "--spi-user-jpa-enabled=false" })
    void testUserManagedEntityNotAddedToDefaultPU(LaunchResult result) {
        CLIResult cliResult = (CLIResult) result;
        cliResult.assertMessage("KC-SERVICES0047: custom_jpa (com.acme.provider.legacy.jpa.user.MyUserProviderFactory) is implementing the internal SPI user. This SPI is internal and may change without notice");
--- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java
+++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java
@ -82,7 +82,7 @@ public class LoggingDistTest {
    }

    @Test
-    @Launch({ "start-dev", "--log-level=\"off,org.keycloak:warn,debug\"" })
+    @Launch({ "start-dev", "--log-level=off,org.keycloak:warn,debug" })
    void testWinSetLastRootLevelIfMultipleSet(LaunchResult result) {
        CLIResult cliResult = (CLIResult) result;
        assertTrue(cliResult.getOutput().contains("DEBUG [io.netty.util.internal"));
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/containers/KeycloakQuarkusServerDeployableContainer.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/containers/KeycloakQuarkusServerDeployableContainer.java
@ -176,6 +176,13 @@ public class KeycloakQuarkusServerDeployableContainer extends AbstractQuarkusDep
    private ProcessBuilder getProcessBuilder() {
        Map<String, String> env = new HashMap<>();
        String[] processCommands = getArgs(env).toArray(new String[0]);
+        if (suiteContext.get().isAuthServerMigrationEnabled() && configuration.getImportFile() != null) {
+            for (int i = 0; i < processCommands.length; i++) {
+                if (processCommands[i].startsWith("--db-url=")) {
+                    processCommands[i]= "--db-url=\"" + processCommands[i].substring(9) + "\"";
+                }
+            }
+        }
        ProcessBuilder pb = new ProcessBuilder(processCommands);

        pb.environment().putAll(env);
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/model/StoreProvider.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/model/StoreProvider.java
@ -55,7 +55,7 @@ public enum StoreProvider {
        @Override
        public void addStoreOptions(List<String> commands) {
            commands.add("--storage=" + getAlias());
-            commands.add("--storage-hotrod-host='" + System.getProperty("keycloak.connectionsHotRod.host") + "'");
+            commands.add("--storage-hotrod-host=" + System.getProperty("keycloak.connectionsHotRod.host"));
            commands.add("--storage-hotrod-username=" + System.getProperty("keycloak.connectionsHotRod.username", "admin"));
            commands.add("--storage-hotrod-password=" + System.getProperty("keycloak.connectionsHotRod.password", "admin"));
        }
@ -69,7 +69,7 @@ public enum StoreProvider {
            if ("mssql".equals(getDbVendor().orElse(null))){
                commands.add("--transaction-xa-enabled=false");
            }
-            commands.add("--db-url='" + System.getProperty("keycloak.connectionsJpa.url") + "'");
+            commands.add("--db-url=" + System.getProperty("keycloak.connectionsJpa.url"));
        }

        @Override
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientSearchTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientSearchTest.java
@ -27,7 +27,6 @@ import org.keycloak.models.ClientProvider;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.testsuite.arquillian.containers.AbstractQuarkusDeployableContainer;

-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@ -142,13 +141,10 @@ public class ClientSearchTest extends AbstractClientTest {
            System.setProperty(SEARCHABLE_ATTRS_PROP, String.join(",", searchableAttributes));
            controller.start(suiteContext.getAuthServerInfo().getQualifier());
        } else if (suiteContext.getAuthServerInfo().isQuarkus()) {
-            searchableAttributes = Arrays.stream(searchableAttributes)
-                    .map(a -> a.replace(" ", "\\ ").replace("\"", "\\\\\\\""))
-                    .toArray(String[]::new);
            String s = String.join(",",searchableAttributes);
            controller.stop(suiteContext.getAuthServerInfo().getQualifier());
            AbstractQuarkusDeployableContainer container = (AbstractQuarkusDeployableContainer)suiteContext.getAuthServerInfo().getArquillianContainer().getDeployableContainer();
-            container.setAdditionalBuildArgs(Collections.singletonList("--spi-client-jpa-searchable-attributes=\""+ s + "\""));
+            container.setAdditionalBuildArgs(Collections.singletonList("--spi-client-jpa-searchable-attributes="+ s));
            controller.start(suiteContext.getAuthServerInfo().getQualifier());
        } else {
            throw new RuntimeException("Don't know how to config");
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java
@ -8,7 +8,6 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.keycloak.testsuite.auth.page.AuthRealm.TEST;

-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@ -230,14 +229,11 @@ public class GroupSearchTest extends AbstractGroupTest {
            System.setProperty(SEARCHABLE_ATTRS_PROP, String.join(",", searchableAttributes));
            controller.start(suiteContext.getAuthServerInfo().getQualifier());
        } else if (suiteContext.getAuthServerInfo().isQuarkus()) {
-            searchableAttributes = Arrays.stream(searchableAttributes)
-                    .map(a -> a.replace(" ", "\\ ").replace("\"", "\\\\\\\""))
-                    .toArray(String[]::new);
            String s = String.join(",", searchableAttributes);
            controller.stop(suiteContext.getAuthServerInfo().getQualifier());
            AbstractQuarkusDeployableContainer container = (AbstractQuarkusDeployableContainer) suiteContext.getAuthServerInfo().getArquillianContainer().getDeployableContainer();
            container.setAdditionalBuildArgs(
-                    Collections.singletonList("--spi-group-jpa-searchable-attributes=\"" + s + "\""));
+                    Collections.singletonList("--spi-group-jpa-searchable-attributes=" + s));
            controller.start(suiteContext.getAuthServerInfo().getQualifier());
        } else {
            throw new RuntimeException("Don't know how to config");