bitflicker64 commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4393004508
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -318,32 +318,333 @@ function check_port() {
return 0
}
-function crontab_append() {
- local job="$1"
- crontab -l | grep -F "$job" >/dev/null 2>&1
- if [ $? -eq 0 ]; then
+# 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|classpath
+# crlf : reject CR/LF. Call on the RAW value (e.g. $OPTARG) BEFORE
+# canonicalize_dir/canonicalize_file.
+# cron : crlf + reject '%'.
+# classpath : crlf + reject ':' and ';'. For a directory whose contents are
+# joined into a ':'-separated CLASSPATH (the plugins dir), where
+# a ':' in the path itself would split one entry into two bogus
+# ones. On Cygwin, hugegraph-server.sh converts that classpath
+# with 'cygpath -p -w', which re-delimits it on ';' instead, so a
+# ';' in the path would do the same splitting there.
+function reject_unsafe_path() {
+ case "$1" in
+ *[$'\n\r']*)
+ echo "ERROR: $2 must not contain CR/LF characters" >&2
+ return 1 ;;
+ esac
+
+ case "$3" in
+ cron)
+ case "$1" in
+ *%*)
+ echo "ERROR: $2 must not contain '%' (reserved by cron)"
>&2
+ return 1 ;;
+ esac ;;
+ classpath)
+ case "$1" in
+ *:*)
+ echo "ERROR: $2 must not contain ':' (CLASSPATH
separator)" >&2
+ return 1 ;;
+ *';'*)
+ echo "ERROR: $2 must not contain ';' (CLASSPATH separator
on Cygwin)" >&2
+ return 1 ;;
+ esac ;;
+ 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"
+}
+
+# Fail fast when a required config file is missing or unreadable.
+# read_property() returns an empty string in that case, which otherwise looks
+# exactly like "the property is not set" and lets startup continue with silent
+# defaults.
+# $1 = file $2 = for error message
+function ensure_file_readable() {
+ local file="$1" description="$2"
+ if [ ! -f "$file" ]; then
+ echo "ERROR: missing $description: '$file'" >&2
+ if [ -n "${CONF_OVERRIDE:-}" ]; then
+ echo " The conf dir passed with -c must contain the same
files" >&2
+ echo " as the packaged conf directory." >&2
+ fi
return 1
fi
- (crontab -l ; echo "$job") | crontab -
+ if [ ! -r "$file" ]; then
+ echo "ERROR: unreadable $description: '$file'" >&2
+ echo " Check the file's ownership and permissions." >&2
+ return 1
+ fi
+ return 0
}
-function crontab_remove() {
- local job="$1"
- # check exist before remove
- crontab -l | grep -F "$job" >/dev/null 2>&1
- if [ $? -eq 1 ]; then
+# A relative path inside a config file is resolved by the JVM against the
+# working directory, which the start scripts set to $TOP -- not against the
+# directory the config file itself lives in. So when -c points at an external
+# conf dir, a value such as 'graphs=./conf/graphs' still loads
+# $TOP/conf/graphs and the operator's copy is ignored with no message at all.
+# Refuse to start in such case when -c is set and say exactly what to change.
+# $1 = value read from the config $2 = property name $3 = config file
+function reject_relative_conf_path() {
+ local value="$1" property="$2" file="$3"
+ # Only an overridden conf dir can disagree with the working directory.
+ [ -z "${CONF_OVERRIDE:-}" ] && return 0
+ [ -z "$value" ] && return 0
+ case "$value" in
+ /*) return 0 ;;
+ esac
+ # Build the suggestion: the stock values are written relative to $TOP (e.g.
+ # './conf/graphs'), while $CONF_OVERRIDE already IS the conf dir, so drop a
+ # leading './' and a leading 'conf/' to avoid suggesting
'.../conf/conf/...'.
+ local suggestion="${value#./}"
+ suggestion="${suggestion#conf/}"
+ echo "ERROR: '$property' is the relative path '$value' in $file." >&2
+ echo " Relative paths resolve against the working directory, so
with" >&2
+ echo " -c '$CONF_OVERRIDE' this would silently keep using the
packaged" >&2
+ echo " directory instead of your copy. Set '$property' to an
absolute" >&2
+ echo " path (for example '$CONF_OVERRIDE/$suggestion')." >&2
+ return 1
+}
+
+# Serialize concurrent cron edits (across both processes and scripts: start,
+# stop and restart can all touch the crontab around the same time). Uses
+# flock(1) when available; falls back to an atomic mkdir-based lock with a
+# stale-lock timeout on systems without flock (e.g. stock macOS).
+# NOTE: neither path is reentrant. A process that already holds this lock
+# must not call crontab_lock_acquire (directly, or via crontab_append /
+# crontab_remove) again before releasing it: flock(1) keys locks off the open
+# file description, not the pid, so a second acquire from the SAME process
+# blocks on itself for the full CRONTAB_LOCK_TIMEOUT_S and then fails.
+# The lock file is keyed to the calling user's uid: a single shared name
+# under /tmp would be created and owned by whichever user got there first,
+# so every other account could then fail to open it (wrong owner, or a
+# hardened fs.protected_regular sysctl, which blocks even root) with no
+# obvious cause.
+CRONTAB_LOCK_FILE="${TMPDIR:-/tmp}/.hugegraph-crontab-$(id -u).lock"
+CRONTAB_LOCK_FD=9
+CRONTAB_LOCK_TIMEOUT_S=30
+
+function crontab_lock_acquire() {
+ if command_available flock; then
+ local open_err
+ open_err="$(eval "exec ${CRONTAB_LOCK_FD}>\"\$CRONTAB_LOCK_FILE\""
2>&1)"
Review Comment:
Critical: `exec` runs inside `$(...)`, so fd 9 opens only in that subshell
and the parent's `flock -w ... 9` fails with `Bad file descriptor`; on Linux
`-m true` never installs the monitor, `stop-hugegraph.sh` never removes one,
and `test-cron-quote.sh` fails 40 cases. Open the fd in the current shell, for
example `if ! eval "exec ${CRONTAB_LOCK_FD}>\"\$CRONTAB_LOCK_FILE\""; then`, in
the PD, Store and Server `util.sh`.
--
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]
