imbajin commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4337213296


##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/restart-hugegraph-store.sh:
##########
@@ -28,5 +28,60 @@ function abs_path() {
 }
 
 BIN=$(abs_path)
-. "$BIN"/stop-hugegraph-store.sh
-. "$BIN"/start-hugegraph-store.sh
+
+DAEMON_ARG=""
+CONF_ARG=""
+GC_ARG=""
+PID_ARG=""
+USER_ARG=""
+LOGS_ARG=""
+PLUGINS_ARG=""
+TELEMETRY_ARG=""
+
+while getopts "d:c:g:i:j:l:o:y:" arg; do
+    case ${arg} in
+        d) DAEMON_ARG="$OPTARG" ;;
+        c) CONF_ARG="$OPTARG" ;;
+        g) GC_ARG="$OPTARG" ;;
+        i) PID_ARG="$OPTARG" ;;
+        j) USER_ARG="$OPTARG" ;;
+        l) LOGS_ARG="$OPTARG" ;;
+        o) PLUGINS_ARG="$OPTARG" ;;
+        y) TELEMETRY_ARG="$OPTARG" ;;
+        ?) echo "USAGE: $0 [-d true|false] [-c conf_dir] [-g g1] [-i pid_file] 
[-j opts] [-l logs_dir] [-o plugins_dir] [-y true|false]" && exit 1 ;;
+    esac
+done
+
+. "$BIN"/util.sh
+
+# -c/-l/-o are validated only by start-hugegraph-store.sh, which runs *after*
+# the stop below. Check them here so an unusable override is rejected while the
+# Store is still up, instead of stopping it and then failing to start it again.
+# -i needs no check here: stop-hugegraph-store.sh validates it first, before it
+# touches the running process.
+reject_unsafe_path "$CONF_ARG" "-c conf dir" crlf || exit 1

Review Comment:
   ⚠️ Important: These pre-stop checks reject only unsafe characters. An 
existing empty directory passed as `-c` passes here, then Store is stopped at 
line 86 before `start-hugegraph-store.sh` checks for a readable 
`application.yml` at line 164; the restart therefore leaves a healthy Store 
down on a bad override. Please validate the required config file and 
output-directory permissions before invoking the stop script. Evidence: 
exact-head trace through `restart-hugegraph-store.sh` and 
`start-hugegraph-store.sh`.



##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh:
##########
@@ -39,30 +39,60 @@ function abs_path() {
 
 BIN=$(abs_path)
 TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
-LOGS="$TOP/logs"
 SCRIPTS="$TOP/scripts"
-PID_FILE="$BIN/pid"
 
 . "$BIN"/util.sh
 
 # Note: keep ':' in the end of the string to indicate the option needs a value
-while getopts "d:g:m:p:s:j:t:y:" arg; do
-    case ${arg} in
-        d) DAEMON="$OPTARG" ;;
-        g) GC_OPTION="$OPTARG" ;;
-        m) OPEN_MONITOR="$OPTARG" ;;
-        p) PRELOAD="$OPTARG" ;;
-        s) OPEN_SECURITY_CHECK="$OPTARG" ;;
-        j) USER_OPTION="$OPTARG" ;;
-        t) SERVER_STARTUP_TIMEOUT_S="$OPTARG" ;;
-        # Telemetry is used to collect metrics, traces and logs
-        y) OPEN_TELEMETRY="$OPTARG" ;;
-        # Note: update usage info when the params changed
-        ?) exit_with_usage_help ;;
-    esac
+while getopts "c:d:g:i:j:l:m:o:p:s:t:y:" arg; do
+     case ${arg} in
+         c) reject_unsafe_path "$OPTARG" "-c conf dir" crlf || exit 1
+            CONF_OVERRIDE="$OPTARG" ;;
+         d) DAEMON="$OPTARG" ;;
+         g) GC_OPTION="$OPTARG" ;;
+         i) reject_unsafe_path "$OPTARG" "-i pid file" crlf || exit 1
+            PID_FILE_OVERRIDE="$OPTARG" ;;
+         j) USER_OPTION="$OPTARG" ;;
+         l) reject_unsafe_path "$OPTARG" "-l logs dir" crlf || exit 1
+            LOGS_OVERRIDE="$OPTARG" ;;
+         m) OPEN_MONITOR="$OPTARG" ;;
+         o) reject_unsafe_path "$OPTARG" "-o plugins dir" classpath || exit 1

Review Comment:
   ⚠️ Important: This validation rejects `:` but allows `;` in `-o`. On Cygwin, 
`hugegraph-server.sh` converts the assembled `:`-separated classpath with 
`cygpath -p -w`; a plugin path containing `;` then becomes part of the Windows 
`;`-delimited `-cp` value and splits the directory into bogus entries, so its 
JARs are not loaded. Please reject `;` for Cygwin plugin paths or encode it 
without confusing the classpath separator. Evidence: exact-head 
`reject_unsafe_path` classpath mode and `hugegraph-server.sh` line 99.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to