imbajin commented on code in PR #3126:
URL: https://github.com/apache/hugegraph/pull/3126#discussion_r3698444897
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -142,8 +153,55 @@ case "$GC_OPTION" in
esac
JVM_OPTIONS="-Dlog4j.configurationFile=${CONF}/log4j2.xml"
+SECURITY_MANAGER_OPTION=""
if [[ ${OPEN_SECURITY_CHECK} == "true" ]]; then
- JVM_OPTIONS="${JVM_OPTIONS}
-Djava.security.manager=org.apache.hugegraph.security.HugeSecurityManager"
+ if [[ ${JAVA_VERSION} -gt ${MAX_SECURITY_JAVA_VERSION} ]]; then
+ SECURITY_UNSUPPORTED_MSG=$(cat <<EOF
+The security check requires Java
${MIN_JAVA_VERSION}-${MAX_SECURITY_JAVA_VERSION}, current is ${JAVA_VERSION}.
+JDK 24+ removed the Security Manager (JEP 486), so HugeSecurityManager can no
longer be installed.
+Run the server on Java ${MAX_SECURITY_JAVA_VERSION} or lower, or start it with
the security check
+disabled: 'start-hugegraph.sh -s false'.
+EOF
+)
+ echo "${SECURITY_UNSUPPORTED_MSG}" >&2
+ echo "${SECURITY_UNSUPPORTED_MSG}" >> "${OUTPUT}"
+ exit 1
+ fi
+
+ SECURITY_PROPERTIES="${CONF}/java-security.properties"
+ if [[ ! -r ${SECURITY_PROPERTIES} ]]; then
+ # An operator may deliberately replace the bundled policy with their
own
+ # -Djava.security.properties=<file>, which the JVM applies last and
which
+ # makes a missing bundled file harmless. Track the last such option,
since
+ # an empty value clears any earlier override.
+ SECURITY_PROPERTIES_OVERRIDDEN="false"
+ for OPTION in ${JAVA_OPTIONS} ${_JAVA_OPTIONS:-}; do
+ case "${OPTION}" in
+ -Djava.security.properties=)
+ SECURITY_PROPERTIES_OVERRIDDEN="false" ;;
+ -Djava.security.properties=?*)
+ SECURITY_PROPERTIES_OVERRIDDEN="true" ;;
Review Comment:
⚠️ Any non-empty `-Djava.security.properties=...` token suppresses the
missing bundled-policy diagnostic without checking whether the replacement file
exists, is readable, or supplies a valid positive TTL. The bootstrap then
rejects the invalid policy, but daemon stderr goes to
`hugegraph-server-stdout.log` while `start-hugegraph.sh` directs operators to
`hugegraph-server.log`, which now lacks the cause. Please suppress this
diagnostic only after validating the effective override, or mirror bootstrap
failures into the advertised log, and cover missing/unreadable/invalid override
files in daemon mode.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -93,8 +97,15 @@ else
JAVA="$JAVA_HOME/bin/java -server"
fi
-JAVA_VERSION=$($JAVA -version 2>&1 | head -1 | cut -d'"' -f2 | sed 's/^1\.//'
| cut -d'.' -f1)
-if [[ $? -ne 0 || $JAVA_VERSION -lt $MIN_JAVA_VERSION ]]; then
+# Pick the version line explicitly: the JVM prints a preamble such as
+# "Picked up JAVA_TOOL_OPTIONS: ..." before it whenever JAVA_TOOL_OPTIONS or
+# _JAVA_OPTIONS is set, and reading that line instead would leave JAVA_VERSION
+# unusable and silently skip every version-gated option below.
+JAVA_VERSION=$($JAVA -version 2>&1 | awk -F'"' '/version "/ {print $2; exit}' |
Review Comment:
⚠️ This still accepts the first arbitrary line containing `version "`, not
specifically the JVM banner. For example, `APM agent version "7.2.0"` before
`openjdk version "21.0.8"` makes this pipeline return `7`, so a supported JDK
is rejected; a preamble containing `version "24"` can likewise trigger the new
upper-bound branch. The test helper uses the same broad match, so it cannot
catch this. Please anchor the match to the actual `java`/`openjdk version` line
and add a tool-options/agent preamble regression.
--
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]
