feat(dist): Make conf/logs/plugins/pid paths overridable and fix log4j2 config resolution

.github/workflows/server-tests.yml

@@ -161,6 +161,17 @@ jobs:
           fi
           echo "can_run=true" >> "$GITHUB_OUTPUT"
 
+      - name: Run path-override guard and quoting unit tests
+        if: ${{ env.BACKEND == 'rocksdb' }}
+        run: |
+          # Pure bash harnesses: no JVM/cluster/real crontab needed (crontab is
+          # stubbed).
+          SHELL_TEST_DIR=hugegraph-server/hugegraph-dist/src/test/shell
+          bash "$SHELL_TEST_DIR/test-canonicalize.sh"
+          bash "$SHELL_TEST_DIR/test-cron-quote.sh"
+          bash "$SHELL_TEST_DIR/test-override-guards.sh"
+          bash "$SHELL_TEST_DIR/test-launcher-argv.sh"
+
       - name: Run start-hugegraph.sh foreground mode tests
         if: ${{ env.BACKEND == 'rocksdb' && steps.server-preflight.outputs.can_run == 'true' }}
         run: |

hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh

@@ -30,14 +30,18 @@ if [ -z "$DAEMON" ]; then
     DAEMON="true"
 fi
 
-while getopts "d:g:j:y:" arg; do
+while getopts "c:d:g:i:j:l:o:y:" arg; do
     case ${arg} in
+        c) CONF_OVERRIDE="$OPTARG" ;;
         g) GC_OPTION="$OPTARG" ;;
+        i) PID_FILE_OVERRIDE="$OPTARG" ;;
         j) USER_OPTION="$OPTARG" ;;
+        l) LOGS_OVERRIDE="$OPTARG" ;;
+        o) PLUGINS_OVERRIDE="$OPTARG" ;;
         # Telemetry is used to collect metrics, traces and logs
         d) DAEMON="$OPTARG" ;;
         y) OPEN_TELEMETRY="$OPTARG" ;;
-        ?) echo "USAGE: $0 [-d true|false] [-g g1|ZGC] [-j xxx] [-y true|false]" && exit 1 ;;
+        ?) echo "USAGE: $0 [-d true|false] [-c conf_dir] [-g g1|ZGC] [-i pid_file] [-j xxx] [-l logs_dir] [-o plugins_dir] [-y true|false]" && exit 1 ;;
     esac
 done
 
@@ -53,24 +57,58 @@ function abs_path() {
 
 BIN=$(abs_path)
 TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
+
+. "$BIN"/util.sh
+
+# Validate the RAW overrides before canonicalization: command substitution
+# strips trailing newlines, which would hide an embedded CR/LF.
+reject_unsafe_path "$CONF_OVERRIDE" "-c conf dir" crlf || exit 1
+reject_unsafe_path "$LOGS_OVERRIDE" "-l logs dir" crlf || exit 1
+reject_unsafe_path "$PID_FILE_OVERRIDE" "-i pid file" crlf || exit 1
+# The plugins dir is joined into ':'-separated classpath/agent settings
+reject_unsafe_path "$PLUGINS_OVERRIDE" "-o plugins dir" classpath || exit 1
+
+# Canonicalize relative path overrides to absolute paths.
+CONF_OVERRIDE="$(canonicalize_dir "$CONF_OVERRIDE" nocreate)" || exit 1
+LOGS_OVERRIDE="$(canonicalize_dir "$LOGS_OVERRIDE")" || exit 1
+PLUGINS_OVERRIDE="$(canonicalize_dir "$PLUGINS_OVERRIDE")" || exit 1
+PID_FILE_OVERRIDE="$(canonicalize_file "$PID_FILE_OVERRIDE")" || exit 1
+
+CONF="${CONF_OVERRIDE:-$TOP/conf}"
 LIB="$TOP/lib"
-PLUGINS="$TOP/plugins"
-LOGS="$TOP/logs"
-OUTPUT=${LOGS}/hugegraph-pd-stdout.log
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
+LOGS="${LOGS_OVERRIDE:-$TOP/logs}"
+OUTPUT="${LOGS}/hugegraph-pd-stdout.log"
 GITHUB="https://github.com"
-PID_FILE="$BIN/pid"
+PID_FILE="${PID_FILE_OVERRIDE:-$BIN/pid}"
 JVM_MODULE_OPTIONS="$BIN/jvm-module.options"
 
 if [[ ! -f "$JVM_MODULE_OPTIONS" || ! -r "$JVM_MODULE_OPTIONS" ]]; then
     echo "Error: JVM module options file is missing or unreadable: $JVM_MODULE_OPTIONS" >&2
     exit 1
 fi
 
-. "$BIN"/util.sh
 
 ensure_path_writable "$LOGS"
 ensure_path_writable "$PLUGINS"
+ensure_path_writable "$(dirname "$PID_FILE")"
+
+# These are resolved relative to the working directory (set to $TOP below), not
+# relative to $CONF, so a relative value would silently keep reading the
+# packaged conf dir even though -c points somewhere else.
+if [ -n "$CONF_OVERRIDE" ]; then
+    # The file is also handed to the JVM as -Dspring.config.location below.
+    # Spring Boot would reject a missing location, but only after the JVM is
+    # up -- by which point this script has written the PID file and exited 0,
+    # so a supervisor sees a successful start and a process that dies moments
+    # later. Fail here instead, while the exit code still means something.
+    ensure_file_readable "${CONF}/application.yml" "PD config" || exit 1
+    for PROPERTY in verify-path license-path; do
+        VALUE=$(sed -n "s/^[[:space:]]*${PROPERTY}:[[:space:]]*//p" \
+                "${CONF}/application.yml" | head -1 | tr -d "'\"")
+        reject_relative_conf_path "$VALUE" "$PROPERTY" "${CONF}/application.yml" || exit 1
+    done
+fi
 
 # The maximum and minimum heap memory that service can use
 MAX_MEM=$((32 * 1024))
@@ -88,51 +126,65 @@ else
 fi
 
 # check jdk version
-JAVA_VERSION=$($JAVA -version 2>&1 |
+JAVA_VERSION=$("$JAVA" -version 2>&1 |
                awk -F'"' '/^(java|openjdk) version "/ {print $2; exit}' |
                sed 's/^1\.//' | cut -d'.' -f1)
 JAVA_VERSION="${JAVA_VERSION%%[!0-9]*}"
 if [[ -z $JAVA_VERSION || $JAVA_VERSION -lt $EXPECT_JDK_VERSION ]]; then
-    echo "Please make sure that the JDK is installed and the version >= $EXPECT_JDK_VERSION"  >> ${OUTPUT}
+    echo "Please make sure that the JDK is installed and the version >= $EXPECT_JDK_VERSION"  >> "${OUTPUT}"
     exit 1
 fi
 
-# Set Java options
-if [ "$JAVA_OPTIONS" = "" ]; then
+# Set Java options.
+# Script-generated options are kept in arrays so that overridden paths
+# containing spaces (CONF/LOGS/PLUGINS) remain a single argument when passed
+# to java. User-supplied strings ($JAVA_OPTIONS / $USER_OPTION) keep the
+# historical whitespace-splitting contract.
+JAVA_OPTIONS_ENV="${JAVA_OPTIONS}"
+JAVA_OPTIONS=()
+if [ "$JAVA_OPTIONS_ENV" = "" ]; then
     XMX=$(calc_xmx $MIN_MEM $MAX_MEM)
     if [ $? -ne 0 ]; then
         echo "Failed to start HugeGraphPDServer, requires at least ${MIN_MEM}m free memory" \
-             >> ${OUTPUT}
+             >> "${OUTPUT}"
         exit 1
     fi
-    JAVA_OPTIONS="-Xms${MIN_MEM}m -Xmx${XMX}m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${LOGS} ${USER_OPTION}"
+    JAVA_OPTIONS=("-Xms${MIN_MEM}m" "-Xmx${XMX}m" "-XX:+HeapDumpOnOutOfMemoryError"
+                  "-XX:HeapDumpPath=${LOGS}")
+
+    # Split user-supplied options on whitespace (legacy behaviour)
+    USER_OPTIONS=()
+    read -r -a USER_OPTIONS <<< "${USER_OPTION}"
+    JAVA_OPTIONS+=(${USER_OPTIONS[@]+"${USER_OPTIONS[@]}"})
 
     # Rolling out detailed GC logs
-    #JAVA_OPTIONS="${JAVA_OPTIONS} -XX:+UseGCLogFileRotation -XX:GCLogFileSize=10M -XX:NumberOfGCLogFiles=3 \
-    #              -Xloggc:./logs/gc.log -XX:+PrintHeapAtGC -XX:+PrintGCDetails -XX:+PrintGCDateStamps"
+    #JAVA_OPTIONS+=("-Xlog:gc=info:file=${LOGS}/gc.log:time,uptime,level,tags:filecount=3,filesize=100m")
+else
+    read -r -a JAVA_OPTIONS <<< "${JAVA_OPTIONS_ENV}"
 fi
 
 # Keep JVM/caller GC selection by default; explicitly select G1 when requested.
 case "$GC_OPTION" in
     ""|g1|G1)
         if [[ "$GC_OPTION" == g1 || "$GC_OPTION" == G1 ]]; then
             echo "Using G1GC"
-            JAVA_OPTIONS="${JAVA_OPTIONS} -XX:+UseG1GC"
+            JAVA_OPTIONS+=("-XX:+UseG1GC")
         else
             echo "Using JVM garbage collector configuration"
         fi
-        JAVA_OPTIONS="-XX:+ParallelRefProcEnabled -XX:InitiatingHeapOccupancyPercent=50 \
-                      -XX:G1RSetUpdatingPauseTimePercent=5 ${JAVA_OPTIONS}"
+        JAVA_OPTIONS=("-XX:+ParallelRefProcEnabled" "-XX:InitiatingHeapOccupancyPercent=50"
+                      "-XX:G1RSetUpdatingPauseTimePercent=5"
+                      ${JAVA_OPTIONS[@]+"${JAVA_OPTIONS[@]}"})
         ;;
     zgc|ZGC)
         echo "Using ZGC as the default garbage collector (requires Java 17 or later)"
-        JAVA_OPTIONS="${JAVA_OPTIONS} -XX:+UseZGC -XX:+UnlockExperimentalVMOptions \
-                                      -XX:ConcGCThreads=2 -XX:ParallelGCThreads=6 \
-                                      -XX:ZCollectionInterval=120 -XX:ZAllocationSpikeTolerance=5 \
-                                      -XX:+UnlockDiagnosticVMOptions -XX:-ZProactive"
+        JAVA_OPTIONS+=("-XX:+UseZGC" "-XX:+UnlockExperimentalVMOptions"
+                       "-XX:ConcGCThreads=2" "-XX:ParallelGCThreads=6"
+                       "-XX:ZCollectionInterval=120" "-XX:ZAllocationSpikeTolerance=5"
+                       "-XX:+UnlockDiagnosticVMOptions" "-XX:-ZProactive")
         ;;
     *)
-        echo "Unrecognized gc option: '$GC_OPTION', supported options: g1, ZGC" >> ${OUTPUT}
+        echo "Unrecognized gc option: '$GC_OPTION', supported options: g1, ZGC" >> "${OUTPUT}"
         exit 1
 esac
 
@@ -146,7 +198,7 @@ if [ "${OPEN_TELEMETRY}" == "true" ]; then
             "${GITHUB}/open-telemetry/opentelemetry-java-instrumentation/releases/download/v2.1.0/${OT_JAR}"
 
         if [[ ! -e "${OT_JAR_PATH}" ]]; then
-            echo "## Error: Failed to download ${OT_JAR}." >>${OUTPUT}
+            echo "## Error: Failed to download ${OT_JAR}." >>"${OUTPUT}"
             exit 1
         fi
     fi
@@ -156,13 +208,24 @@ if [ "${OPEN_TELEMETRY}" == "true" ]; then
     actual_md5=$(md5sum "${OT_JAR_PATH}" | awk '{print $1}')
 
     if [[ "${expected_md5}" != "${actual_md5}" ]]; then
-        echo "## Error: MD5 checksum verification failed for ${OT_JAR_PATH}." >>${OUTPUT}
-        echo "## Tips: Remove the file and try again." >>${OUTPUT}
+        echo "## Error: MD5 checksum verification failed for ${OT_JAR_PATH}." >>"${OUTPUT}"
+        echo "## Tips: Remove the file and try again." >>"${OUTPUT}"
         exit 1
     fi
 
     # Note: check carefully if multi "javeagent" params are set
-    export JAVA_TOOL_OPTIONS="-javaagent:${PLUGINS}/${OT_JAR}"
+    # The JVM splits JAVA_TOOL_OPTIONS on whitespace, so an overridden $PLUGINS
+    # containing a space would turn the agent option into two arguments. The JVM
+    # honours quoted options, so wrap it in a quote the path does not contain.
+    if [[ ${OT_JAR_PATH} != *"'"* ]]; then
+        AGENT_QUOTE="'"
+    elif [[ ${OT_JAR_PATH} != *\"* ]]; then
+        AGENT_QUOTE='"'
+    else
+        echo "## Error: ${OT_JAR_PATH} contains both quote characters, which JAVA_TOOL_OPTIONS cannot carry." >>"${OUTPUT}"
+        exit 1
+    fi
+    export JAVA_TOOL_OPTIONS="${AGENT_QUOTE}-javaagent:${OT_JAR_PATH}${AGENT_QUOTE}"
     export OTEL_TRACES_EXPORTER=otlp
     export OTEL_METRICS_EXPORTER=none
     export OTEL_LOGS_EXPORTER=none
@@ -177,12 +240,17 @@ fi
 #if [ "${JMX_EXPORT_PORT}" != "" ] && [ ${JMX_EXPORT_PORT} -ne 0 ] ; then
 #  JAVA_OPTIONS="${JAVA_OPTIONS} -javaagent:${LIB}/jmx_prometheus_javaagent-0.16.1.jar=${JMX_EXPORT_PORT}:${CONF}/jmx_exporter.yml"
 #fi
-if [ $(ps -ef|grep -v grep| grep java|grep -cE ${CONF}) -ne 0 ]; then
+if [ "$(ps -ef | grep -v grep | grep java | grep -cF -- "${CONF}")" -ne 0 ]; then
    echo "HugeGraphPDServer is already running..."
    exit 0
 fi
 
-JVM_OPTIONS="-Dlog4j.configurationFile=${CONF}/log4j2.xml -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager"
+JVM_OPTIONS=("-Dlog4j.configurationFile=${CONF}/log4j2.xml"
+             "-Dlogging.config=${CONF}/log4j2.xml"
+             "-DLOG_PATH=${LOGS}"
+             "-Dbolt.log.path=${LOGS}"
+             "-Dlogging.path=${LOGS}"
+             "-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager")
 
 source "$BIN/preload-topling.sh" || exit 1
 BOOT_JARS=("${LIB}"/hg-pd-service-*.jar)
@@ -196,14 +264,20 @@ if [ -n "${TOPLING_RUNTIME_CLASSPATH:-}" ]; then
 fi
 
 # Turn on security check
+# NOTE: "$JVM_MODULE_OPTIONS" and "$OUTPUT" are passed/redirected as single
+# quoted arguments, and JVM_OPTIONS/JAVA_OPTIONS are expanded as arrays
+# ("${JVM_OPTIONS[@]}"), so an overridden CONF/LOGS/PLUGINS path containing a
+# space stays one argument instead of being split by word-splitting.
 if [[ $DAEMON == "true" ]]; then
     echo "Starting HugeGraphPDServer in daemon mode..."
     if [[ "${STDOUT_MODE:-false}" == "true" ]]; then
-        exec ${JAVA} "@$JVM_MODULE_OPTIONS" -Dname="HugeGraphPD" ${JVM_OPTIONS} ${JAVA_OPTIONS} \
-            -Dspring.config.location=${CONF}/application.yml "${JAVA_MAIN[@]}" "$@" &
+        exec "${JAVA}" "@${JVM_MODULE_OPTIONS}" -Dname="HugeGraphPD" \
+            "${JVM_OPTIONS[@]}" "${JAVA_OPTIONS[@]}" \
+            -Dspring.config.location="${CONF}/application.yml" "${JAVA_MAIN[@]}" "$@" &
     else
-        exec ${JAVA} "@$JVM_MODULE_OPTIONS" -Dname="HugeGraphPD" ${JVM_OPTIONS} ${JAVA_OPTIONS} \
-            -Dspring.config.location=${CONF}/application.yml "${JAVA_MAIN[@]}" "$@" >> ${OUTPUT} 2>&1 &
+        exec "${JAVA}" "@${JVM_MODULE_OPTIONS}" -Dname="HugeGraphPD" \
+            "${JVM_OPTIONS[@]}" "${JAVA_OPTIONS[@]}" \
+            -Dspring.config.location="${CONF}/application.yml" "${JAVA_MAIN[@]}" "$@" >> "${OUTPUT}" 2>&1 &
     fi
     PID="$!"
     # Write pid to file
@@ -215,10 +289,12 @@ else
     echo "$$" > "$PID_FILE"
     echo "[+pid] $$"
     if [[ "${STDOUT_MODE:-false}" == "true" ]]; then
-        exec ${JAVA} "@$JVM_MODULE_OPTIONS" -Dname="HugeGraphPD" ${JVM_OPTIONS} ${JAVA_OPTIONS} \
-            -Dspring.config.location=${CONF}/application.yml "${JAVA_MAIN[@]}" "$@"
+        exec "${JAVA}" "@${JVM_MODULE_OPTIONS}" -Dname="HugeGraphPD" \
+            "${JVM_OPTIONS[@]}" "${JAVA_OPTIONS[@]}" \
+            -Dspring.config.location="${CONF}/application.yml" "${JAVA_MAIN[@]}" "$@"
     else
-        exec ${JAVA} "@$JVM_MODULE_OPTIONS" -Dname="HugeGraphPD" ${JVM_OPTIONS} ${JAVA_OPTIONS} \
-            -Dspring.config.location=${CONF}/application.yml "${JAVA_MAIN[@]}" "$@" >> ${OUTPUT} 2>&1
+        exec "${JAVA}" "@${JVM_MODULE_OPTIONS}" -Dname="HugeGraphPD" \
+            "${JVM_OPTIONS[@]}" "${JAVA_OPTIONS[@]}" \
+            -Dspring.config.location="${CONF}/application.yml" "${JAVA_MAIN[@]}" "$@" >> "${OUTPUT}" 2>&1
     fi
 fi

hugegraph-pd/hg-pd-dist/src/assembly/static/bin/stop-hugegraph-pd.sh

@@ -27,22 +27,37 @@ abs_path() {
     echo "$( cd -P "$( dirname "$SOURCE" )" && pwd )"
 }
 
+while getopts "i:" arg; do
+    case ${arg} in
+        i) PID_FILE_OVERRIDE="$OPTARG" ;;
+        ?) echo "USAGE: $0 [-i pid_file]" && exit 1 ;;
+    esac
+done
+
 BIN=$(abs_path)
 TOP="$(cd $BIN/../ && pwd)"
 
 . "$BIN"/util.sh
 
-PID_FILE=$BIN/pid
+reject_unsafe_path "$PID_FILE_OVERRIDE" "-i pid file" crlf || exit 1
+
+# Resolve to the same absolute path start-hugegraph-pd.sh wrote. "nocreate":
+# stop only reads the pid file
+PID_FILE_OVERRIDE="$(canonicalize_file "$PID_FILE_OVERRIDE" nocreate)" || exit 1
+
+PID_FILE="${PID_FILE_OVERRIDE:-$BIN/pid}"
 SERVER_SHUTDOWN_TIMEOUT_S=30
 
-if [ ! -f ${PID_FILE} ]; then
+if [ ! -f "${PID_FILE}" ]; then
     echo "The pid file $PID_FILE doesn't exist"
     exit 0
 fi
 
-PID=$(cat $PID_FILE)
-kill_process_and_wait "HugeGraphPDServer" "$PID" "$SERVER_SHUTDOWN_TIMEOUT_S"
-
-if [ $? -eq 0 ]; then
+PID=$(cat "$PID_FILE")
+# An unchecked call would let the script fall off the end with status 0, so a
+# shutdown that timed out with the JVM still alive would be reported as success.
+if kill_process_and_wait "HugeGraphPDServer" "$PID" "$SERVER_SHUTDOWN_TIMEOUT_S"; then
     rm "$PID_FILE"
+else
+    exit 1
 fi