bitflicker64 commented on code in PR #3126:
URL: https://github.com/apache/hugegraph/pull/3126#discussion_r3699298664
##########
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:
Fixed in 9d53f15. The match is now anchored to the JVM banner line itself —
only lines starting with `java version "` or `openjdk version "` are considered
— in both the launcher and the test helper's own `JAVA_MAJOR` parse.
Added two mock-JVM regressions where an agent banner precedes the JVM's:
- `Elastic APM agent version "7.2.0"` ahead of a JDK 21 banner must still
emit `-Djava.security.manager=allow` (the unanchored match reads 7 and rejects
the runtime as below the minimum).
- `APM agent version "24.0.1"` ahead of a JDK 11 banner must not trip the
JDK 24+ guard and must reach the bootstrap without a security-manager option.
Both fail against the unanchored match (re-verified by mutating the packaged
launcher back to `/version "/`).
##########
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:
Fixed in 791947c, via the second option: the bootstrap's rejection is
mirrored into the advertised log rather than validated shell-side.
I deliberately did not add launcher-side validation of the override file.
Only the JVM's own properties parsing decides what the override loads to — the
escaped-key, line-continuation and `file:`-URL cases in the test suite are all
files a plain shell check would mis-classify — so a shell validator could
disagree with the bootstrap in both directions and reintroduce exactly this
inconsistency. The bootstrap stays the single validator.
Mechanics: in daemon mode the launcher passes
`-Dhugegraph.bootstrap.error.log=${LOGS}/hugegraph-server.log`, and the
bootstrap appends the same fatal message it prints to stderr (including the
effective `java.security.properties` value, so the broken override is named) to
that file, best-effort, before the security manager is installed and without
touching the logging framework. Stdout mode is unchanged since stderr is
already operator-visible there.
New daemon-mode tests cover missing, unreadable and infinite-TTL operator
overrides, asserting both the cause and the override path land in
`hugegraph-server.log`; the unreadable fixture also carries invalid content so
the case still fails closed where permission bits do not apply (root).
No-op'ing the error-log property in the packaged launcher makes them fail.
--
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]
