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


##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-java-security-properties.sh:
##########
@@ -559,6 +603,346 @@ assert_argument "true" "$ENABLED_CAPTURE"
 assert_argument "-Doperator.marker=preserved" "$ENABLED_CAPTURE"
 assert_no_argument '^-D(networkaddress\.cache\.ttl|sun\.net\.inetaddr\.ttl)=' \
                    "$ENABLED_CAPTURE"
+# Heap dump and crash log defaults go first in JAVA_TOOL_OPTIONS, so the JVM's
+# own precedence lets every operator source override them. Check the values a
+# real JVM settles on when started the way a captured launcher run would start
+# it: the JAVA_TOOL_OPTIONS, JDK_JAVA_OPTIONS and _JAVA_OPTIONS the launcher
+# passed on, and the crash-related -XX: arguments of its command line.
+effective_flag() {
+    local capture="$1"
+    local flag="$2"
+    local args=()
+    local line
+    local flags
+    while IFS= read -r line; do
+        if [[ "$line" =~ 
^-XX:([+-]HeapDumpOnOutOfMemoryError|HeapDumpPath=|ErrorFile=) ]]; then
+            args+=("$line")
+        fi
+    done < "$capture"
+    if ! flags=$(JAVA_TOOL_OPTIONS="$(cat "${capture}.tool-options")" \
+                 JDK_JAVA_OPTIONS="$(cat "${capture}.jdk-java-options")" \
+                 _JAVA_OPTIONS="$(cat "${capture}.underscore-java-options")" \
+                 "$JAVA_BIN" ${args[@]+"${args[@]}"} -XX:+PrintFlagsFinal 
-version \
+                 2>"${capture}.flags.err"); then
+        fail "JVM rejected the options captured in ${capture}: $(cat 
"${capture}.flags.err")"
+    fi
+    # The value sits between "= " and the trailing "{origin}" columns and may
+    # contain spaces.
+    # A here-string, not a pipe: awk exits early, which would kill printf with
+    # SIGPIPE under pipefail.
+    awk -v flag="$flag" '$2 == flag {
+        sub(/^[^=]*= /, ""); sub(/ *(\{[^}]*\} *)+$/, ""); print; exit }' <<< 
"$flags"
+}
+
+assert_effective_flag() {
+    local capture="$1"
+    local flag="$2"
+    local pattern="$3"
+    local value
+    value=$(effective_flag "$capture" "$flag")
+    [[ "$value" =~ $pattern ]] ||
+        fail "effective ${flag} is '${value}', expected to match ${pattern}"
+}
+
+LOGS_PATTERN=$(printf '%s' "${SERVER_ROOT}/logs" | sed 
's/[][\.*^$+?(){}|]/\\&/g')
+# The names carry the host name, the launch time and, if needed, a counter,
+# since a restarted container often reuses the PID, pods may share one log
+# volume, HotSpot truncates an existing crash log (JDK 17+), and it will not
+# write a heap dump over an existing file.
+# HeapDumpPath names a directory per launch, so each JVM writes its own
+# java_pid<pid>.hprof there.
+NAME_ID_PATTERN="[A-Za-z0-9._-]+_[0-9]{8}-[0-9]{6}(-[0-9]+)?"
+HEAP_DUMP_PATTERN="^${LOGS_PATTERN}/heapdump_${NAME_ID_PATTERN}$"
+ERROR_FILE_PATTERN="^${LOGS_PATTERN}/hs_err_pid%p_${NAME_ID_PATTERN}\.log$"
+
+assert_no_argument 
'^-XX:([+-]HeapDumpOnOutOfMemoryError|HeapDumpPath=|ErrorFile=)' \
+                   "$ENABLED_CAPTURE"
+assert_effective_flag "$ENABLED_CAPTURE" HeapDumpOnOutOfMemoryError '^true$'
+assert_effective_flag "$ENABLED_CAPTURE" HeapDumpPath "$HEAP_DUMP_PATTERN"
+assert_effective_flag "$ENABLED_CAPTURE" ErrorFile "$ERROR_FILE_PATTERN"
+ENABLED_DUMP_DIR=$(effective_flag "$ENABLED_CAPTURE" HeapDumpPath)
+[[ -d "$ENABLED_DUMP_DIR" ]] ||
+    fail "launcher did not create the heap dump directory ${ENABLED_DUMP_DIR}"
+
+# Child JVMs the Server starts inherit JAVA_TOOL_OPTIONS. Two JVMs started with
+# the launcher's options, like the Server and a computer job, must each write
+# their own heap dump into the launch directory instead of competing for one.
+OOM_SOURCE="${TEMP_DIR}/OomCheck.java"
+cat > "$OOM_SOURCE" <<'JAVA'
+import java.util.ArrayList;
+import java.util.List;
+
+public class OomCheck {
+    public static void main(String[] args) {
+        List<long[]> hold = new ArrayList<>();
+        while (true) {
+            hold.add(new long[1 << 20]);
+        }
+    }
+}
+JAVA
+OOM_DUMP_DIR="$ENABLED_DUMP_DIR"
+for OOM_RUN in 1 2; do
+    JAVA_TOOL_OPTIONS="$(cat "${ENABLED_CAPTURE}.tool-options")" \
+        "$JAVA_BIN" -Xmx64m "$OOM_SOURCE" >"${TEMP_DIR}/oom-${OOM_RUN}.log" 
2>&1 || true
+done
+OOM_DUMPS=$(find "$OOM_DUMP_DIR" -maxdepth 1 -name 'java_pid*.hprof' | wc -l)
+if [[ "$OOM_DUMPS" -ne 2 ]]; then
+    ls -l "$OOM_DUMP_DIR" >&2 || true
+    cat "${TEMP_DIR}"/oom-*.log >&2 || true
+    fail "expected a heap dump per JVM in ${OOM_DUMP_DIR}, found ${OOM_DUMPS}"
+fi
+rm -f "$OOM_DUMP_DIR"/java_pid*.hprof
+
+# JAVA_OPTIONS replaces the default heap options; the crash defaults stay, and
+# paths given there win.
+CUSTOM_OPTIONS_CAPTURE="${TEMP_DIR}/custom-java-options.args"
+CAPTURE_FILE="$CUSTOM_OPTIONS_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JAVA_OPTIONS="-Xmx1g -XX:ErrorFile=/operator/hs_err.log \
+                  -XX:HeapDumpPath=/operator/dumps" \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_argument "-Xmx1g" "$CUSTOM_OPTIONS_CAPTURE"
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" HeapDumpOnOutOfMemoryError 
'^true$'
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" HeapDumpPath 
'^/operator/dumps$'
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" ErrorFile 
'^/operator/hs_err\.log$'
+
+# The JVM, not the launcher, parses the operator's JAVA_TOOL_OPTIONS: a quoted
+# opt-out works, and flag-like text inside a property value changes nothing.
+QUOTED_TOOL_CAPTURE="${TEMP_DIR}/quoted-tool-options.args"
+CAPTURE_FILE="$QUOTED_TOOL_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JAVA_TOOL_OPTIONS='"-XX:-HeapDumpOnOutOfMemoryError" 
"-Dmarker=-XX:ErrorFile=/nope"' \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$QUOTED_TOOL_CAPTURE" HeapDumpOnOutOfMemoryError 
'^false$'
+assert_effective_flag "$QUOTED_TOOL_CAPTURE" ErrorFile "$ERROR_FILE_PATTERN"
+
+# JDK_JAVA_OPTIONS, including an @argfile, also overrides the defaults.
+ARGFILE="${TEMP_DIR}/jdk-java-options.args"
+echo '-XX:ErrorFile=/operator/argfile-hs_err.log' > "$ARGFILE"
+ARGFILE_CAPTURE="${TEMP_DIR}/argfile.args"
+CAPTURE_FILE="$ARGFILE_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JDK_JAVA_OPTIONS="@${ARGFILE}" \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$ARGFILE_CAPTURE" ErrorFile 
'^/operator/argfile-hs_err\.log$'
+assert_effective_flag "$ARGFILE_CAPTURE" HeapDumpPath "$HEAP_DUMP_PATTERN"
+
+# _JAVA_OPTIONS comes after the command line, and -j lands on the command line
+# when JAVA_OPTIONS is unset; both override the defaults.
+UNDERSCORE_CAPTURE="${TEMP_DIR}/underscore-java-options.args"
+CAPTURE_FILE="$UNDERSCORE_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    _JAVA_OPTIONS="-XX:HeapDumpPath=/operator/underscore" \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true \
+    "-XX:ErrorFile=/operator/user-option-hs_err.log" >/dev/null
+
+assert_effective_flag "$UNDERSCORE_CAPTURE" HeapDumpPath 
'^/operator/underscore$'
+assert_effective_flag "$UNDERSCORE_CAPTURE" ErrorFile 
'^/operator/user-option-hs_err\.log$'
+
+# A launch never reuses a name already taken in logs/, even within the same
+# second: with a fixed clock and host name, the counter moves past an existing
+# heap dump directory, a crash log and a dangling symlink. The launcher never
+# removes dump directories: an empty one from an earlier launch may still be 
the
+# target of a computer-job JVM that outlived its Server.
+MOCK_DATE_BIN="${TEMP_DIR}/mock-date-bin"
+mkdir -p "$MOCK_DATE_BIN"
+printf '#!/bin/bash\necho 20200101-000000\n' > "${MOCK_DATE_BIN}/date"
+chmod +x "${MOCK_DATE_BIN}/date"
+mkdir -p "${SERVER_ROOT}/logs"
+# Only fixtures this run creates are recorded for removal, so an existing file
+# with the same name in the supplied distribution is left alone.
+add_crash_fixture() {
+    local path="$1"
+    local kind="$2"
+    if [[ ! -e "$path" && ! -L "$path" ]]; then
+        CRASH_NAME_FIXTURES+=("$path")
+        case "$kind" in
+            file) : > "$path" ;;
+            empty-dir) mkdir "$path" ;;
+            dump-dir) mkdir "$path" && : > "${path}/java_pid1.hprof" ;;
+            dangling-link) ln -s "${TEMP_DIR}/missing-target" "$path" ;;
+        esac
+    fi
+}
+USED_DUMP_DIR="${SERVER_ROOT}/logs/heapdump_test-pod-a_20200101-000000"
+OLD_EMPTY_DUMP_DIR="${SERVER_ROOT}/logs/heapdump_test-pod-a_20191231-000000"
+OTHER_HOST_DUMP_DIR="${SERVER_ROOT}/logs/heapdump_test-pod-c_20191231-000000"
+add_crash_fixture "$USED_DUMP_DIR" dump-dir
+add_crash_fixture 
"${SERVER_ROOT}/logs/hs_err_pid1_test-pod-a_20200101-000000-1.log" file
+add_crash_fixture "${SERVER_ROOT}/logs/heapdump_test-pod-a_20200101-000000-2" 
dangling-link
+LAUNCHER_DUMP_DIRS+=("${SERVER_ROOT}/logs/heapdump_test-pod-a_20200101-000000-3"
+                     "${SERVER_ROOT}/logs/heapdump_test_pod-b_20200101-000000")
+add_crash_fixture "$OLD_EMPTY_DUMP_DIR" empty-dir
+add_crash_fixture "$OTHER_HOST_DUMP_DIR" empty-dir
+UNIQUE_NAME_CAPTURE="${TEMP_DIR}/unique-name.args"
+CAPTURE_FILE="$UNIQUE_NAME_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    HOSTNAME=test-pod-a PATH="${MOCK_DATE_BIN}:${PATH}" STDOUT_MODE=true 
"$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$UNIQUE_NAME_CAPTURE" HeapDumpPath \
+    "^${LOGS_PATTERN}/heapdump_test-pod-a_20200101-000000-3$"
+assert_effective_flag "$UNIQUE_NAME_CAPTURE" ErrorFile \
+    "^${LOGS_PATTERN}/hs_err_pid%p_test-pod-a_20200101-000000-3\.log$"
+[[ -f "${SERVER_ROOT}/logs/hs_err_pid1_test-pod-a_20200101-000000-1.log" ]] ||
+    fail "launcher removed an existing crash log"
+[[ -d "$OLD_EMPTY_DUMP_DIR" ]] ||
+    fail "launcher removed an empty heap dump directory from an earlier launch"
+[[ -d "$OTHER_HOST_DUMP_DIR" ]] ||
+    fail "launcher removed another host's heap dump directory"
+[[ -f "${USED_DUMP_DIR}/java_pid1.hprof" ]] ||
+    fail "launcher removed an existing heap dump"
+
+# Another pod sharing the volume gets its own names in the same second, and
+# characters unsafe in a file name are replaced.
+OTHER_HOST_CAPTURE="${TEMP_DIR}/other-host.args"
+CAPTURE_FILE="$OTHER_HOST_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    HOSTNAME='test/pod-b' PATH="${MOCK_DATE_BIN}:${PATH}" STDOUT_MODE=true 
"$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$OTHER_HOST_CAPTURE" HeapDumpPath \
+    "^${LOGS_PATTERN}/heapdump_test_pod-b_20200101-000000$"
+if [[ ${#CRASH_NAME_FIXTURES[@]} -gt 0 ]]; then
+    rm -rf "${CRASH_NAME_FIXTURES[@]}"
+fi
+CRASH_NAME_FIXTURES=()
+
+# A launcher error before Java starts must reach stderr, which 
start-hugegraph.sh
+# passes to the terminal or container log, as well as the server log, and the
+# failed launch must not leave a heap dump directory behind.
+: > "$SERVER_LOG"
+PREFLIGHT_ERROR="${TEMP_DIR}/preflight.err"
+PREFLIGHT_DUMP_DIR="${SERVER_ROOT}/logs/heapdump_test-preflight_20200101-000000"
+LAUNCHER_DUMP_DIRS+=("$PREFLIGHT_DUMP_DIR")
+if JAVA_HOME="$MOCK_JAVA_HOME" HOSTNAME=test-preflight 
PATH="${MOCK_DATE_BIN}:${PATH}" \
+   STDOUT_MODE=true "$SERVER_SCRIPT" \
+   "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true "" 
"bad-gc" \
+   >/dev/null 2>"$PREFLIGHT_ERROR"; then
+    fail "launcher accepted an unknown GC option"
+fi
+grep -Fq "Unrecognized gc option: 'bad-gc'" "$PREFLIGHT_ERROR" ||
+    fail "launcher preflight error did not reach stderr"
+grep -Fq "Unrecognized gc option: 'bad-gc'" "$SERVER_LOG" ||
+    fail "launcher preflight error did not reach the server log"
+[[ ! -e "$PREFLIGHT_DUMP_DIR" ]] ||
+    fail "a launch that failed before Java started left ${PREFLIGHT_DUMP_DIR}"
+
+# The riscv64 libatomic check runs before anything else, and its error must 
also
+# reach the server log. Mock uname and ldconfig so no libatomic is found; skip 
if
+# this machine has one at a fixed path the launcher also probes.
+RISCV_LIBATOMIC_FOUND="false"
+for RISCV_CANDIDATE in /lib/riscv64-linux-gnu/libatomic.so.1 \
+                       /usr/lib/riscv64-linux-gnu/libatomic.so.1 \
+                       /lib64/lp64d/libatomic.so.1 
/usr/lib64/lp64d/libatomic.so.1 \
+                       /lib64/libatomic.so.1 /usr/lib64/libatomic.so.1; do
+    if [[ -r "$RISCV_CANDIDATE" ]]; then
+        RISCV_LIBATOMIC_FOUND="true"
+    fi
+done
+if [[ "$RISCV_LIBATOMIC_FOUND" == "false" ]]; then
+    MOCK_RISCV_BIN="${TEMP_DIR}/mock-riscv-bin"
+    mkdir -p "$MOCK_RISCV_BIN"
+    printf '#!/bin/bash\ncase "$1" in -s) echo Linux ;; -m) echo riscv64 ;; *) 
echo Linux ;; esac\n' \
+        > "${MOCK_RISCV_BIN}/uname"
+    printf '#!/bin/bash\nexit 0\n' > "${MOCK_RISCV_BIN}/ldconfig"
+    chmod +x "${MOCK_RISCV_BIN}/uname" "${MOCK_RISCV_BIN}/ldconfig"
+    : > "$SERVER_LOG"
+    RISCV_ERROR="${TEMP_DIR}/riscv.err"
+    if JAVA_HOME="$MOCK_JAVA_HOME" PATH="${MOCK_RISCV_BIN}:${PATH}" 
LD_PRELOAD="" \
+       STDOUT_MODE=true "$SERVER_SCRIPT" \
+       "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true \
+       >/dev/null 2>"$RISCV_ERROR"; then
+        fail "launcher started on riscv64 without libatomic"
+    fi
+    grep -Fq "RISC-V RocksDB requires libatomic.so.1" "$RISCV_ERROR" ||
+        fail "the riscv64 libatomic error did not reach stderr"
+    grep -Fq "RISC-V RocksDB requires libatomic.so.1" "$SERVER_LOG" ||
+        fail "the riscv64 libatomic error did not reach the server log"
+fi
+
+# A full or read-only logs volume must not stop the Server: when the dump
+# directory cannot be created, the launcher warns and dumps into logs/ itself.
+FAILING_MKDIR_BIN="${TEMP_DIR}/failing-mkdir-bin"
+mkdir -p "$FAILING_MKDIR_BIN"
+cat > "${FAILING_MKDIR_BIN}/mkdir" <<'MKDIR'
+#!/bin/bash
+for arg in "$@"; do
+    case "$arg" in
+        */heapdump_*) echo "mkdir: ${arg}: No space left on device" >&2; exit 
1 ;;
+    esac
+done
+exec /bin/mkdir "$@"
+MKDIR
+chmod +x "${FAILING_MKDIR_BIN}/mkdir"
+: > "$SERVER_LOG"
+NO_DUMP_DIR_CAPTURE="${TEMP_DIR}/no-dump-dir.args"
+NO_DUMP_DIR_ERROR="${TEMP_DIR}/no-dump-dir.err"
+CAPTURE_FILE="$NO_DUMP_DIR_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    PATH="${FAILING_MKDIR_BIN}:${PATH}" STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true \
+    >/dev/null 2>"$NO_DUMP_DIR_ERROR" ||
+    fail "launcher did not start when the heap dump directory could not be 
created"
+grep -Fq "WARN: cannot create ${SERVER_ROOT}/logs/heapdump_" 
"$NO_DUMP_DIR_ERROR" ||
+    fail "launcher did not warn on stderr that the heap dump directory was not 
created"
+grep -Fq "WARN: cannot create ${SERVER_ROOT}/logs/heapdump_" "$SERVER_LOG" ||
+    fail "launcher did not log that the heap dump directory was not created"
+assert_effective_flag "$NO_DUMP_DIR_CAPTURE" HeapDumpPath "^${LOGS_PATTERN}$"
+
+# An unwritable logs/ is reported on stderr, where the container log keeps it.
+# Root can write anywhere, so the check only runs for other users.
+if [[ "$(id -u)" -ne 0 ]]; then
+    mkdir -p "${TEMP_DIR}/unwritable-logs-dist/logs" 
"${TEMP_DIR}/unwritable-logs-dist/plugins"
+    # The launcher reports the resolved path (TEMP_DIR is a symlink on macOS).
+    MINI_ROOT=$(cd "${TEMP_DIR}/unwritable-logs-dist" && pwd -P)
+    cp -R "${SERVER_ROOT}/bin" "${SERVER_ROOT}/conf" "${MINI_ROOT}/"
+    chmod 555 "${MINI_ROOT}/logs"
+    UNWRITABLE_ERROR="${TEMP_DIR}/unwritable-logs.err"
+    if JAVA_HOME="$MOCK_JAVA_HOME" STDOUT_MODE=true 
"${MINI_ROOT}/bin/hugegraph-server.sh" \
+       "${MINI_ROOT}/conf/gremlin-server.yaml" 
"${MINI_ROOT}/conf/rest-server.properties" true \
+       >/dev/null 2>"$UNWRITABLE_ERROR"; then
+        chmod 755 "${MINI_ROOT}/logs"
+        fail "launcher started with an unwritable logs directory"
+    fi
+    chmod 755 "${MINI_ROOT}/logs"
+    grep -Fq "No write permission on directory ${MINI_ROOT}/logs" 
"$UNWRITABLE_ERROR" ||
+        fail "launcher did not report the unwritable logs directory on stderr"
+fi
+
+# With telemetry on, the launcher appends its agent to JAVA_TOOL_OPTIONS and
+# keeps both the defaults and the operator's flags there.
+OT_EXPECTED_MD5=$(grep -E '^ *expected_md5=' "$SERVER_SCRIPT" | cut -d'"' -f2)
+MOCK_MD5_BIN="${TEMP_DIR}/mock-md5-bin"
+mkdir -p "$MOCK_MD5_BIN"
+printf '#!/bin/bash\necho "%s  $1"\n' "$OT_EXPECTED_MD5" > 
"${MOCK_MD5_BIN}/md5sum"
+chmod +x "${MOCK_MD5_BIN}/md5sum"
+OT_JAR_PATH="${SERVER_ROOT}/plugins/opentelemetry-javaagent.jar"
+if [[ ! -e "$OT_JAR_PATH" ]]; then

Review Comment:
   ⚠️ Important: `[[ ! -e "$OT_JAR_PATH" ]]` treats a dangling 
`plugins/opentelemetry-javaagent.jar` symlink as absent, then `: > 
"$OT_FIXTURE_JAR"` follows it and creates the symlink target; cleanup removes 
only the link. If that target is outside the distribution, running this 
regression test leaves an unintended file there. Please reject `-L 
"$OT_JAR_PATH"` or run the fixture in an isolated copy and clean up any target 
it creates. Evidence: the creation branch at lines 922-925 and cleanup at 
934-936.



-- 
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