imbajin commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3650933499
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -286,8 +516,14 @@ function download() {
exit 1
}
fi
- curl -L "${download_url}" -o "${path}/$(basename "${download_url}")"
+ curl -fL "${download_url}" -o "${path}/$(basename "${download_url}")"
Review Comment:
⚠️ This writes curl output directly to the final destination. On a transfer
error, curl can leave a partial file there; the telemetry callers then see that
path on the next startup, skip the download, and fail checksum validation until
an operator deletes it. The PD and Store copies have the same behavior, and the
new failure test checks only the return code. Please download to a temporary
file in the destination directory and rename only after success (cleaning it on
failure), then add a partial-transfer retry regression.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,245 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# Run a command with a hard deadline via background watchdog.
+# Returns the command's exit code if it finishes in time.
+# If the deadline expires, the command is killed (exit code reflects signal).
+# Works without the timeout command — uses sleep + kill -9 pattern.
+function run_with_deadline() {
+ local cmd="$1"
+ local deadline="$2"
+ shift 2
+
+ bash -c "$cmd" bash "$@" &
+ local child_pid=$!
+ (
+ sleep "$deadline"
+ kill -9 "$child_pid" 2>/dev/null
+ ) 2>/dev/null &
+ local watchdog_pid=$!
+
+ wait "$child_pid" 2>/dev/null
+ local rc=$?
+ kill -9 "$watchdog_pid" 2>/dev/null || true
+ wait "$watchdog_pid" 2>/dev/null || true
+ return $rc
+}
+
+# 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
+
+ # Strip leading/trailing whitespace from URL (handles whitespace from
ServerOptions)
+ url="${url#"${url%%[![:space:]]*}"}"
+ url="${url%"${url##*[![:space:]]}"}"
+
+ # Extract authority: strip scheme (http://, https://, or none) and path.
+ # This ensures port is extracted from host:port, not from a colon in the
path.
+ local authority
+ if [[ "$url" =~ ^[a-zA-Z][a-zA-Z0-9+.-]*:// ]]; then
+ authority=$(echo "$url" | sed 's|^[^/]*://||; s|/.*||')
Review Comment:
⚠️ Authority extraction removes only a `/` suffix, not `?` or `#`. A
controlled run with `http://127.0.0.1:8080?probe=x` and `ss` reporting
`127.0.0.1:8080` returns success because the authority becomes
`127.0.0.1:8080?probe=x`, while Java URI parsing still uses host `127.0.0.1`
and port `8080`. Please terminate authority at the first `/`, `?`, or `#`
(preferably through one URI-normalization path) and add query/fragment
regressions.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,245 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# Run a command with a hard deadline via background watchdog.
+# Returns the command's exit code if it finishes in time.
+# If the deadline expires, the command is killed (exit code reflects signal).
+# Works without the timeout command — uses sleep + kill -9 pattern.
+function run_with_deadline() {
+ local cmd="$1"
+ local deadline="$2"
+ shift 2
+
+ bash -c "$cmd" bash "$@" &
+ local child_pid=$!
+ (
+ sleep "$deadline"
+ kill -9 "$child_pid" 2>/dev/null
+ ) 2>/dev/null &
+ local watchdog_pid=$!
+
+ wait "$child_pid" 2>/dev/null
+ local rc=$?
+ kill -9 "$watchdog_pid" 2>/dev/null || true
+ wait "$watchdog_pid" 2>/dev/null || true
+ return $rc
+}
+
+# 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
+
+ # Strip leading/trailing whitespace from URL (handles whitespace from
ServerOptions)
+ url="${url#"${url%%[![:space:]]*}"}"
+ url="${url%"${url##*[![:space:]]}"}"
+
+ # Extract authority: strip scheme (http://, https://, or none) and path.
+ # This ensures port is extracted from host:port, not from a colon in the
path.
+ local authority
+ if [[ "$url" =~ ^[a-zA-Z][a-zA-Z0-9+.-]*:// ]]; then
+ authority=$(echo "$url" | sed 's|^[^/]*://||; s|/.*||')
+ else
+ authority=$(echo "$url" | sed 's|/.*||')
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
+
+ # Extract host and port from authority.
+ # Assumes IPv6 is bracketed (e.g. [::1]:8080). Unbracketed IPv6 like
+ # ::1:8080 would misparse as host=::, port=1. ServerOptions does not
+ # enforce bracketing at the config layer; if this ever fires on bad input
+ # the downstream bind would fail with a clearer error than the preflight.
+ if [[ "$authority" =~ ^\[([^\]]*)\]:([0-9]+)$ ]]; then
+ # IPv6 with port: [::1]:8080
+ host="${BASH_REMATCH[1]}"
+ port="${BASH_REMATCH[2]}"
+ elif [[ "$authority" =~ :([0-9]+)$ ]]; then
+ # IPv4 or hostname with port: 127.0.0.1:8080
+ port="${BASH_REMATCH[1]}"
+ host="${authority%:*}"
+ else
+ # No explicit port in authority
+ host="$authority"
+ port=""
+ fi
+
+ # Handle default ports from scheme when no explicit port given
+ if [[ -z "$port" ]]; then
+ # Reject invalid port: authority has exactly one colon
(host:non-digits).
+ # IPv6 addresses (2+ colons, e.g. [::1] or ::1) are not caught by this.
+ if [[ "$authority" != *:*:* && "$authority" == *:* ]]; then
+ return 0
+ fi
+ if [[ "$url" == https://* ]]; then
+ port="443"
+ elif [[ "$url" == http://* ]]; then
+ port="80"
+ fi
+ fi
+
+ if [[ -z "$port" ]]; then
+ return 0
+ fi
+
+ if ! [[ "$port" =~ ^[0-9]+$ ]]; then
+ return 0
+ fi
+ port=$((10#$port))
+ if (( port < 1 || port > 65535 )); then
+ return 0
+ fi
+
+ # Strip any leading/trailing whitespace from host
+ host="${host#"${host%%[![:space:]]*}"}"
+ host="${host%"${host##*[![:space:]]}"}"
+
+ # Resolve hostname → numeric IPs so ss/netstat (which use -n) can match
them.
+ # A "hostname" is anything that is not blank, a wildcard, or already
numeric.
+ # Resolution runs with a deadline to prevent hangs from slow/stuck
DNS/LDAP.
+ # If timeout is unavailable, skip resolution and fall through to bounded
/dev/tcp.
+ local resolved_addrs=""
+ local is_hostname=0
+ if [[ -n "$host" && "$host" != "0.0.0.0" && "$host" != "::" && "$host" !=
"*" ]] \
+ && ! [[ "$host" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]] \
+ && ! [[ "$host" =~ ^[0-9a-fA-F:]*:[0-9a-fA-F:]*$ ]]; then
+ is_hostname=1
+ if command_available "getent" && command_available "timeout"; then
+ resolved_addrs=$(timeout 2 getent hosts "$host" 2>/dev/null | awk
'{print $1}')
+ elif command_available "dscacheutil" && command_available "timeout";
then
+ resolved_addrs=$(timeout 2 dscacheutil -q host -a name "$host"
2>/dev/null \
+ | awk '/ip_address:/{print $2}')
+ fi
+ # If timeout unavailable or resolution times out, resolved_addrs stays
empty
+ # and we fall through to the bounded /dev/tcp probe (2s watchdog)
instead.
+ fi
+
+ local linux_pattern
+ local bsd_pattern
+ if [[ -z "$host" || "$host" == "0.0.0.0" || "$host" == "::" || "$host" ==
"*" ]]; then
+ # Wildcard patterns match any address family (IPv4 or IPv6) on the port
+ # Note: This treats IPv4 and IPv6 wildcard listeners as conflicts,
which is conservative
+ # but may be overly strict for dual-stack systems where they can
coexist
+ linux_pattern=":${port}([[:space:]]|$)"
+ bsd_pattern="(\.|:)${port}([[:space:]]|$)"
+ else
+ # For hostnames, only use resolved numeric addresses (ss/netstat
report numeric).
+ # Keep wildcard matching family-aware to avoid cross-family false
conflicts.
+ local addr_alts=""
+ local match_ipv4=0
+ local match_ipv6=0
+ if [[ $is_hostname -eq 1 ]]; then
+ # Hostname: use resolved addresses and infer address family set
+ while IFS= read -r addr; do
+ [[ -z "$addr" ]] && continue
+ local esc_addr="${addr//./\\.}"
+ if [[ "$addr" == *":"* ]]; then
+ esc_addr="${esc_addr//:/\\:}"
+ match_ipv6=1
+ else
+ match_ipv4=1
+ fi
+ if [[ -z "$addr_alts" ]]; then
+ addr_alts="${esc_addr}|\[${esc_addr}\]"
+ else
+ addr_alts="${addr_alts}|${esc_addr}|\[${esc_addr}\]"
+ fi
+ done <<< "$resolved_addrs"
+ else
+ # Numeric address: use it directly and infer address family
+ local esc_host="${host//./\\.}"
Review Comment:
‼️ Numeric IPv6 addresses are compared as escaped text, so equivalent
spellings are treated as different endpoints. On this head, mocked `ss` output
for `[::1]:8080` makes `check_port "http://[0:0:0:0:0:0:0:1]:8080"` return
success even though both addresses identify the same socket and the later bind
fails with `EADDRINUSE`; mixed-case hex has the same problem. Please
canonicalize configured and listener-table IPv6 addresses before comparison (or
compare parsed binary addresses), and add compressed/expanded and
case-variation regressions.
--
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]
