imbajin commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3640932477
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,60 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# check whether the REST server port is occupied
function check_port() {
- local port=$(echo "$1" | sed 's|.*:||' | sed 's|/.*||')
- if ! command_available "lsof"; then
- echo "Required lsof but it is unavailable"
- exit 1
+ local url="$1"
+ local host
+ local port
+
+ # Extract port: last colon-delimited segment, strip trailing path
+ port=$(echo "$url" | sed 's|.*:||' | sed 's|/.*||')
+ if [[ -z "$port" ]]; then
+ return 0
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
+
+ # Extract host: handle IPv6 bracket notation e.g. http://[::1]:8080
+ if [[ "$url" =~ ://\[([^\]]*)\] ]]; then
+ host="${BASH_REMATCH[1]}"
+ else
+ host=$(echo "$url" | sed 's|.*://||' | sed 's|:.*||')
+ fi
+
+ local in_use=0
+ if command_available "ss"; then
+ if ss -ltn 2>/dev/null | grep -qE "(:${port}\b|:${port}$)"; then
+ in_use=1
+ fi
+ elif command_available "netstat"; then
+ if netstat -ltn 2>/dev/null | grep -qE "(:${port}\b|:${port}$)"; then
+ in_use=1
+ elif netstat -an 2>/dev/null | grep -i "listen" | grep -qE
"(\.|:)${port}([[:space:]]|$)"; then
+ in_use=1
+ fi
+ else
+ # Wildcard binds include loopback: normalize to loopback for the probe
+ if [[ -z "$host" || "$host" == "0.0.0.0" ]]; then
+ host="127.0.0.1"
+ elif [[ "$host" == "::" ]]; then
+ host="::1"
+ fi
+
+ # Use bash /dev/tcp as a fallback if ss/netstat are missing,
+ # with a short timeout and opening without a newline payload.
+ if command_available "timeout"; then
+ if timeout 1 bash -c ": >/dev/tcp/$host/$port" 2>/dev/null; then
Review Comment:
⚠️ `host` and `port` come directly from `rest-server.properties`, but
interpolating them into the code string passed to `bash -c` reparses
configuration text as shell syntax. With `ss`/`netstat` absent and `timeout`
present, `restserver.url=http://127.0.0.1:1;printf INJECTED` executes the
appended command; a controlled run prints `INJECTED` before reporting the port
in use. Please validate the port as an integer in `1..65535` and pass both
values as positional parameters, for example `bash -c ': >/dev/tcp/"$1"/"$2"' _
"$host" "$port"`, rather than embedding them in the command string.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,60 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# check whether the REST server port is occupied
function check_port() {
- local port=$(echo "$1" | sed 's|.*:||' | sed 's|/.*||')
- if ! command_available "lsof"; then
- echo "Required lsof but it is unavailable"
- exit 1
+ local url="$1"
+ local host
+ local port
+
+ # Extract port: last colon-delimited segment, strip trailing path
+ port=$(echo "$url" | sed 's|.*:||' | sed 's|/.*||')
+ if [[ -z "$port" ]]; then
+ return 0
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
+
+ # Extract host: handle IPv6 bracket notation e.g. http://[::1]:8080
+ if [[ "$url" =~ ://\[([^\]]*)\] ]]; then
Review Comment:
⚠️ This IPv6 branch requires `://`, although
`ServerOptionsTest.testUrlNormalizationEdgeCases` explicitly supports
`[::1]:8080`. The shell receives that raw property before Java normalization,
falls through here, and parses `host="["`; on systems without `ss`/`netstat`,
`/dev/tcp` then probes an invalid host and can miss an occupied port. Please
trim and normalize the value consistently with `ServerOptions` (including
bracketed IPv6 with an optional scheme), validate the numeric port, and add a
`[::1]:8080` regression.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,60 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# check whether the REST server port is occupied
function check_port() {
- local port=$(echo "$1" | sed 's|.*:||' | sed 's|/.*||')
- if ! command_available "lsof"; then
- echo "Required lsof but it is unavailable"
- exit 1
+ local url="$1"
+ local host
+ local port
+
+ # Extract port: last colon-delimited segment, strip trailing path
+ port=$(echo "$url" | sed 's|.*:||' | sed 's|/.*||')
+ if [[ -z "$port" ]]; then
+ return 0
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
+
+ # Extract host: handle IPv6 bracket notation e.g. http://[::1]:8080
+ if [[ "$url" =~ ://\[([^\]]*)\] ]]; then
+ host="${BASH_REMATCH[1]}"
+ else
+ host=$(echo "$url" | sed 's|.*://||' | sed 's|:.*||')
+ fi
+
+ local in_use=0
+ if command_available "ss"; then
Review Comment:
⚠️ These new `ss`, GNU/BSD `netstat`, `timeout`, and raw `/dev/tcp` branches
have no focused tests, while the only startup script test still requires `lsof`
at lines 148-151 and uses it for cleanup at lines 79-82. That suite therefore
cannot exercise the central no-lsof contract in the target environment. Please
add PATH/mock-driven shell tests for each probe branch, occupied/free IPv4 and
IPv6 cases, and timeout behavior, and make the startup test's prerequisite and
cleanup lsof-free.
--
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]
