imbajin commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3647172029
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,92 @@ 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
+
+ if ! [[ "$port" =~ ^[0-9]+$ ]]; then
+ return 0
+ fi
+ port=$((10#$port))
+ if (( port < 1 || port > 65535 )); then
+ return 0
+ fi
+
+ # Extract host
+ if [[ "$url" =~ \[([^\]]*)\] ]]; then
+ host="${BASH_REMATCH[1]}"
+ else
+ host=$(echo "$url" | sed 's|.*://||' | sed 's|:.*||')
+ fi
+
+ local linux_pattern
+ local bsd_pattern
+ if [[ -z "$host" || "$host" == "0.0.0.0" || "$host" == "::" || "$host" ==
"*" ]]; then
+ linux_pattern=":${port}([[:space:]]|$)"
+ bsd_pattern="(\.|:)${port}([[:space:]]|$)"
+ else
+ local esc_host="${host//./\.}"
+
linux_pattern="(0\.0\.0\.0|\*|\[::\]|::|${esc_host}|\[${esc_host}\]):${port}([[:space:]]|$)"
+
bsd_pattern="(0\.0\.0\.0|\*|\[::\]|::|${esc_host})(\.|:)${port}([[:space:]]|$)"
+ fi
+
+ local in_use=0
+ local port_checked=0
+ local out=""
+
+ if command_available "ss"; then
+ if out=$(ss -ltn 2>/dev/null); then
+ port_checked=1
+ if echo "$out" | grep -qE "$linux_pattern"; then
Review Comment:
‼️ `ss -n` and `netstat -n` report numeric addresses, but this pattern
compares them with the configured hostname text.
`ServerOptionsTest.testUrlNormalizationPreservesHostnameCase` explicitly
supports hostnames; with `restserver.url=http://localhost:8080` and `ss`
reporting `127.0.0.1:8080`, this branch sets `port_checked=1`, misses the
listener, and never reaches the resolving `/dev/tcp` fallback. A controlled run
on this head returns success (port free) for that occupied endpoint. Please
resolve hostnames to their IPv4/IPv6 addresses before matching, or use a
deadline-bounded probe for hostname inputs, and add a
hostname-to-numeric-listener regression.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-check-port.sh:
##########
@@ -0,0 +1,481 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# test-check-port.sh — Unit tests for check_port() in util.sh
+#
+# Strategy: each test runs check_port in a subshell that overrides
+# command_available() to control which probe branch is taken, and
+# overrides the tool functions (ss, netstat, timeout) to control
+# what they return — no real network connections needed.
+#
+# check_port calls `exit 1` when the port is in use, so the subshell
+# exits 1; it returns normally (exit 0) when the port is free.
+#
+# Usage: ./test-check-port.sh [path-to-hugegraph-static-dir]
+# path-to-hugegraph-static-dir: directory containing bin/util.sh
+# Defaults to current directory.
+# In CI: $TRAVIS_DIR/test-check-port.sh
hugegraph-server/hugegraph-dist/src/assembly/static
+
+set -uo pipefail
+
+STATIC_DIR="${1:-$(pwd)}"
+UTIL_SH="$STATIC_DIR/bin/util.sh"
+
+REPO_ROOT="$(cd "$(dirname "$0")/../../../../.." && pwd)"
+PD_UTIL_SH="$REPO_ROOT/hugegraph-pd/hg-pd-dist/src/assembly/static/bin/util.sh"
+
+PASS=0
+FAIL=0
+ERRORS=()
+
+GREEN='\033[0;32m'
+RED='\033[0;31m'
+YELLOW='\033[1;33m'
+NC='\033[0m'
+
+pass() { echo -e "${GREEN} PASS${NC} $1"; PASS=$((PASS + 1)); }
+fail() { echo -e "${RED} FAIL${NC} $1"; ERRORS+=("$1"); FAIL=$((FAIL + 1)); }
+section() { echo ""; echo "── $1 ──"; }
+
+echo ""
+echo "check_port() unit test suite"
+echo "util.sh: $UTIL_SH"
+echo ""
+
+if [[ ! -f "$UTIL_SH" ]]; then
+ echo -e "${RED}ERROR:${NC} $UTIL_SH not found."
+ echo " Pass the HugeGraph static assembly dir as \$1"
+ exit 1
+fi
+
+# ── ss branch
─────────────────────────────────────────────────────────────────
+
+section "ss branch — IPv4"
+
+(
+ # shellcheck source=/dev/null
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:8080 0.0.0.0:*"; }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: IPv4 port occupied → exit 1" \
+ || fail "ss: IPv4 port occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:9090 0.0.0.0:*"; }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: IPv4 port free → exit 0" \
+ || fail "ss: IPv4 port free → expected exit 0, got 1"
+
+section "ss branch — IPv6 URL with scheme (http://[::1]:8080)"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "http://[::1]:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: http://[::1]:8080 occupied → exit 1" \
+ || fail "ss: http://[::1]:8080 occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "http://[::1]:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: http://[::1]:8080 free → exit 0" \
+ || fail "ss: http://[::1]:8080 free → expected exit 0, got 1"
+
+section "ss branch — IPv6 URL without scheme ([::1]:8080)"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "[::1]:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: [::1]:8080 (no scheme) occupied → exit 1" \
+ || fail "ss: [::1]:8080 (no scheme) occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "[::1]:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: [::1]:8080 (no scheme) free → exit 0" \
+ || fail "ss: [::1]:8080 (no scheme) free → expected exit 0, got 1"
+
+section "ss branch — wildcard 0.0.0.0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:8080 0.0.0.0:*"; }
+ check_port "http://0.0.0.0:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: 0.0.0.0:8080 occupied → exit 1" \
+ || fail "ss: 0.0.0.0:8080 occupied → expected exit 1, got 0"
+
+section "ss branch — wildcard ::"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "http://[::]:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: [::]:8080 occupied → exit 1" \
+ || fail "ss: [::]:8080 occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "http://[::]:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: [::]:8080 free → exit 0" \
+ || fail "ss: [::]:8080 free → expected exit 0, got 1"
+
+# ── netstat branch
────────────────────────────────────────────────────────────
+
+section "netstat branch — Linux format (-ltn), occupied"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 0.0.0.0:8080 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "netstat -ltn: port 8080 occupied → exit 1" \
+ || fail "netstat -ltn: port 8080 occupied → expected exit 1, got 0"
+
+section "netstat branch — Linux format (-ltn), free"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 0.0.0.0:9090 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "netstat -ltn: port 8080 free → exit 0" \
+ || fail "netstat -ltn: port 8080 free → expected exit 0, got 1"
+
+section "netstat branch — BSD/macOS fallback (-an), occupied"
+
+# Simulate netstat that produces no output for -ltn (Linux flag unsupported)
+# but outputs BSD-format lines for -an
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() {
+ if [[ "$1" == "-ltn" ]]; then
+ return 1 # flag not supported on BSD
+ fi
+ echo "tcp4 0 0 *.8080 *.* LISTEN"
+ }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "netstat -an BSD: port 8080 occupied → exit 1" \
+ || fail "netstat -an BSD: port 8080 occupied → expected exit 1, got 0"
+
+section "netstat branch — IP octet false-positive guard"
+
+# Port 80 check; netstat output contains 192.168.80.1:443
+# The .80 in the IP address must NOT match port 80
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 192.168.80.1:443 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:80"
+)
+[[ $? -eq 0 ]] \
+ && pass "netstat: IP octet .80 does not false-positive for port 80 → exit
0" \
+ || fail "netstat: IP octet .80 false-positived for port 80 → expected exit
0, got 1"
+
+# ── /dev/tcp fallback branch
──────────────────────────────────────────────────
+
+section "/dev/tcp fallback — timeout available, port occupied"
+
+# timeout exits 0 → connection succeeded → port in use
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "timeout" ]]; }
+ timeout() {
+ # Assert correct invocation: timeout 1 bash -c SCRIPT _ HOST PORT
+ [[ "$1" == "1" && "$2" == "bash" && "$3" == "-c" && "$5" == "_" ]] \
+ || { echo "timeout mock: unexpected argv: $*"; return 2; }
+ return 0
+ }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "/dev/tcp+timeout: connection succeeded (exit 0) → port occupied →
exit 1" \
+ || fail "/dev/tcp+timeout: connection succeeded → expected exit 1, got 0"
+
+section "/dev/tcp fallback — timeout available, port free"
+
+# timeout exits 1 → connection refused → port free
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "timeout" ]]; }
+ timeout() {
+ [[ "$1" == "1" && "$2" == "bash" && "$3" == "-c" && "$5" == "_" ]] \
+ || { echo "timeout mock: unexpected argv: $*"; return 2; }
+ return 1
+ }
+ check_port "http://127.0.0.1:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "/dev/tcp+timeout: connection refused (exit 1) → port free → exit
0" \
+ || fail "/dev/tcp+timeout: connection refused → expected exit 0, got 1"
+
+section "/dev/tcp fallback — real loopback (no mock)"
+
+(
+ if ! command -v python3 >/dev/null 2>&1; then
+ echo "SKIP: python3 not available — skipping real loopback test"
+ exit 77
+ fi
+
+ source "$UTIL_SH"
+ # Force the fallback by pretending ss/netstat don't exist
+ command_available() {
+ if [[ "$1" == "ss" || "$1" == "netstat" ]]; then return 1; fi
+ command -v "$1" >/dev/null 2>&1
+ }
+ # Start a real listener on an ephemeral high port (e.g., 54321)
+ # Use python as a simple cross-platform web server since nc arguments vary
+ python3 -m http.server 54321 --bind 127.0.0.1 >/dev/null 2>&1 &
+ PY_PID=$!
+ trap "kill -9 $PY_PID 2>/dev/null || true" EXIT
+ sleep 1 # wait for it to bind
Review Comment:
⚠️ This is a fixed port, not an ephemeral one, and the test never verifies
that the Python child actually bound it. If 54321 is already occupied, Python
exits immediately, `check_port` connects to the unrelated listener, and the
test still passes. Please bind port 0 and return the selected port to the test
(or at least assert child liveness/readiness before calling `check_port`) so
this cannot produce a false-positive green result.
--
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]