imbajin commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4219399736
##########
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh:
##########
@@ -166,22 +178,22 @@ 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 -cE "${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"
# Turn on security check
if [[ $DAEMON == "true" ]]; then
echo "Starting HugeGraphPDServer in daemon mode..."
if [[ "${STDOUT_MODE:-false}" == "true" ]]; then
exec ${JAVA} -Dname="HugeGraphPD" ${JVM_OPTIONS} ${JAVA_OPTIONS} -jar \
- -Dspring.config.location=${CONF}/application.yml
${LIB}/hg-pd-service-*.jar &
+ -Dspring.config.location="${CONF}"/application.yml
${LIB}/hg-pd-service-*.jar &
Review Comment:
‼️ Critical: `-jar` must be followed immediately by the JAR path, but the
first token here is `-Dspring.config.location=...`; Java therefore tries to
open that token as the JAR and never reaches `hg-pd-service-*.jar`. The Store
launcher repeats this order in daemon and foreground modes. Please move
`-Dspring.config.location` before `-jar`, then pass the service JAR immediately
after it in both roles. Evidence: exact-head command order at PD lines 192-193
and Store lines 248-250.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -312,27 +312,64 @@ function check_port() {
return 0
}
+# Emit $1 as ONE complete POSIX shell word, INCLUDING the surrounding quotes.
+# Callers should NOT add another pair of quotes:
+# /data/team's-logs -> '/data/team'\''s-logs'
+#
+function shell_quote() {
+ local sq="'" repl="'\\''"
+ printf "'%s'" "${1//$sq/$repl}"
+}
+
+# Validate a path-like value. $1=value $2=label for the error $3=crlf|cron
+# crlf : reject CR/LF. Call on the RAW value (e.g. $OPTARG) BEFORE
+# canonicalize_dir/canonicalize_file.
+# cron : crlf + reject '%'.
+function reject_unsafe_path() {
+ case "$1" in
+ *[$'\n\r']*)
+ echo "ERROR: $2 must not contain CR/LF characters" >&2
+ return 1 ;;
+ esac
+
+ [ "$3" = "cron" ] || return 0
+
+ case "$1" in
+ *%*)
+ echo "ERROR: $2 must not contain '%' (reserved by cron)" >&2
+ return 1 ;;
+ esac
+ return 0
+}
+
+# Validate (cron rules) then quote. $1=value $2=label. Prints the quoted word.
+function cron_quote() {
+ reject_unsafe_path "$1" "$2" cron || return 1
+ shell_quote "$1"
+}
+
function crontab_append() {
local job="$1"
- crontab -l | grep -F "$job" >/dev/null 2>&1
+ crontab -l 2>/dev/null | grep -F -- "$job" >/dev/null 2>&1
if [ $? -eq 0 ]; then
return 1
fi
- (crontab -l ; echo "$job") | crontab -
+ # printf, not echo: echo may interpret backslashes or swallow a leading
-n/-e
+ { crontab -l 2>/dev/null; printf '%s\n' "$job"; } | crontab -
Review Comment:
⚠️ Important: This writer pipes `crontab -l` into `crontab -` without
checking that the read succeeded. If the read fails after existing jobs are
present, the pipeline can install only this new line; `crontab_remove` has the
same unchecked read/filter/write pattern and can install an empty table. Please
capture and validate the current table before replacing it, distinguish an
absent crontab from a read error, and apply the guard to the PD and Store
copies too. Evidence: `crontab_append` at line 358 and `crontab_remove` at line
369.
##########
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh:
##########
@@ -166,22 +178,22 @@ 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 -cE "${CONF}")" -ne 0 ]; then
Review Comment:
⚠️ Important: This process guard passes the user-supplied `CONF` path to
`grep -E`, so valid override paths containing regex metacharacters are not
matched literally. For example, `/data/pd[1]/conf` can miss the running JVM's
literal command line or match a different path, leading to a duplicate PD start
or incorrectly skipping this start. Please use fixed-string matching such as
`grep -F -- "$CONF"`. Evidence: `-c` rejects CR/LF but does not reject regex
characters before this check.
--
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]
