bitflicker64 commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3664126053
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -118,18 +119,175 @@ function process_id() {
return "$pid"
}
-# check the port of rest server 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
+# Extract a validated TCP port from a configured server URL.
+# Echoes the port on success. Returns 1 when the value carries no usable port
+# or is ambiguous, in which case the caller skips the preflight.
+function parse_port_from_url() {
+ local url="$1"
+
+ # ServerOptions tolerates surrounding whitespace, so strip it first.
+ url="${url#"${url%%[![:space:]]*}"}"
+ url="${url%"${url##*[![:space:]]}"}"
+ [[ -z "$url" ]] && return 1
+
+ # The scheme is optional and case-insensitive.
+ local scheme="" rest="$url"
+ if [[ "$url" == *"://"* ]]; then
+ scheme=$(echo "${url%%://*}" | tr '[:upper:]' '[:lower:]')
+ rest="${url#*://}"
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
- echo "The port $port has already been used"
- exit 1
+
+ # The authority ends at the first '/', '?' or '#'.
+ local authority="${rest%%[/?#]*}"
+ [[ -z "$authority" ]] && return 1
+
+ # Drop any userinfo prefix; its colon would otherwise look like an
+ # unbracketed IPv6 separator.
+ authority="${authority##*@}"
+ [[ -z "$authority" ]] && return 1
+
+ local port=""
+ if [[ "$authority" =~ ^\[[^]]*\](:([0-9]+))?$ ]]; then
+ # Bracketed IPv6, with or without a port: [::1] or [::1]:8080
+ port="${BASH_REMATCH[2]}"
+ elif [[ "$authority" == *:*:* ]]; then
+ # Unbracketed IPv6 is ambiguous: in "::1:8080" the trailing group may
+ # be a port or another hextet. Refuse to guess.
+ # TODO(check_port): no preflight runs at all for this form. If
+ # ServerOptions ever guarantees a normalized bracketed value here, this
+ # branch can resolve the port instead of skipping the check.
+ echo "WARN: ambiguous IPv6 authority '$authority' in server URL;" \
+ "use bracket notation such as [::1]:8080." >&2
+ return 1
+ elif [[ "$authority" == *:* ]]; then
+ port="${authority##*:}"
+ fi
+
+ # Fall back to the scheme's default port.
+ # TODO(check_port): a scheme-less value with no explicit port (e.g. plain
+ # "127.0.0.1") has no derivable port, so it is skipped rather than guessed.
+ # Reading the configured default from ServerOptions would close this gap.
+ if [[ -z "$port" ]]; then
+ case "$scheme" in
+ http) port="80" ;;
+ https) port="443" ;;
+ *) return 1 ;;
+ esac
+ fi
+
+ [[ "$port" =~ ^[0-9]+$ ]] || return 1
+ # Normalise leading-zero forms textually; Java reads 08080 as decimal 8080.
+ # Arithmetic conversion must not happen before the value is bounded: Bash
+ # evaluates in 64-bit and wraps silently, so 18446744073709559616 would
+ # otherwise pass the range check as port 8000.
+ port="${port#"${port%%[!0]*}"}"
+ [[ -z "$port" ]] && return 1
+ (( ${#port} <= 5 )) || return 1
+ (( port >= 1 && port <= 65535 )) || return 1
+
+ echo "$port"
+}
+
+# Echo "busy", "free" or "unknown" for the given TCP port.
+#
+# Detection is deliberately port-only, matching the conservative behaviour of
+# the `lsof -i :PORT` call this replaces. Reproducing kernel socket semantics
+# in Bash - dual-stack IPV6_V6ONLY, wildcard versus specific binds, address
+# canonicalisation - produced more wrong answers than it prevented, so we only
+# ask "is anything already listening on this port?".
+#
+# Only LISTEN rows and only the local-address column are inspected, so an
+# unrelated outbound connection to the same port number is never mistaken for
+# a local listener. A tool that is missing, fails, or yields no recognisable
+# listener row reports "unknown" rather than "free".
+#
+# TODO(check_port): port-only matching ignores the listener's address, so a
+# listener bound to one local address (127.0.0.1:8080) reports the port busy
+# even when the server would bind a different one (192.168.1.5:8080). This is
+# deliberate - it is what `lsof -i :PORT` did, and it fails safe - but it can
+# refuse a bind that would have succeeded. If that is reported in practice,
+# revisit by comparing the local-address column instead of only its port.
+#
+# TODO(check_port): a host with genuinely zero LISTEN sockets is indistinguish-
+# able from a restricted or unparseable table, so both report "unknown" and
+# warn on every start. Distinguishing them needs a positive signal that the
+# table was readable (e.g. an exit status ss/netstat do not currently give).
+function port_listen_state() {
+ local port="$1"
+ local out
+ local os
+ os=$(uname)
+
+ # $4 is the local address for both `ss -ltn` and BSD `netstat -an`.
+ # Splitting on the last separator keeps IPv6 hextets (for example
+ # [2001:db8::80]:443) from being misread as the port.
+ local parser='
+ NF >= 4 && (!want_listen || $NF == "LISTEN") {
+ addr = $4
+ cut = 0
+ for (k = length(addr); k > 0; k--) {
+ if (substr(addr, k, 1) == sep) { cut = k; break }
+ }
+ if (cut == 0) next
+ rows++
+ if (substr(addr, cut + 1) == port) { found = 1; exit }
+ }
+ END {
+ if (found) print "busy"
+ else if (rows > 0) print "free"
+ else print "unknown"
+ }'
+
+ if [[ "$os" == "Darwin" || "$os" == *BSD* ]]; then
+ if command_available "netstat" && out=$(netstat -an -p tcp
2>/dev/null) \
+ && [[ -n "$out" ]]; then
+ echo "$out" | awk -v port="$port" -v sep="." -v want_listen=1
"$parser"
+ return 0
+ fi
+ else
+ # `ss -H -ltn` already restricts output to listening sockets.
+ if command_available "ss" && out=$(ss -H -ltn 2>/dev/null) && [[ -n
"$out" ]]; then
+ echo "$out" | awk -v port="$port" -v sep=":" -v want_listen=0
"$parser"
+ return 0
+ fi
+ if command_available "netstat" && out=$(netstat -ltn 2>/dev/null) \
+ && [[ -n "$out" ]]; then
+ echo "$out" | awk -v port="$port" -v sep=":" -v want_listen=1
"$parser"
+ return 0
+ fi
fi
+
+ # TODO(check_port): with neither ss nor netstat present (some minimal
Review Comment:
Confirmed, and it is worse than a lost warning. Checked the base image
rather than reasoning about it:
```
$ docker run --rm eclipse-temurin:11-jre-jammy \
bash -c 'for t in ss netstat lsof ip; do printf "%-8s " $t; command -v
$t || echo MISSING; done'
ss MISSING
netstat MISSING
lsof MISSING
ip MISSING
```
So the image installs `lsof` for a probe nothing calls any more, and ships
nothing the new probe can use. Against a real listener, sourcing the exact-head
`util.sh` inside that image:
```
as shipped (lsof): port 8080 -> unknown port 9999 -> unknown
with iproute2: port 8080 -> busy port 9999 -> free
```
Fixed in `d93b6f9d` by taking the second option you offered: `lsof` is
replaced with `iproute2` in `hugegraph-server/Dockerfile` and
`Dockerfile-hstore`. Retaining an `lsof` fallback would put back the call whose
fd-table scan is the hang this PR exists to fix, and it would be reached
exactly where it hurts — a container with a large `ulimit -n`.
Flagging one judgement call: I **removed** `lsof` rather than keeping it
alongside `iproute2`. Nothing shipped calls it (`checksocket.sh` shells out to
`CheckSocket`, `docker-entrypoint.sh` does not use it), so it was only a
debugging convenience like `vim`. Say the word if you would rather keep both.
On the image-level test: the cheap half is in, the expensive half is not.
`docker-build-ci.yml` already builds each Dockerfile on any PR touching
`**/Dockerfile*`, so it now asserts the probe can answer inside the built image:
```yaml
STATE=$(docker run --rm "$IMAGE_ID" bash -c \
'source /hugegraph-server/bin/util.sh && port_listen_state 8080')
[[ "$STATE" != "unknown" ]] || { echo "ERROR: no usable socket-table tool";
exit 1; }
```
That pins the dependency, not the duplicate-start behaviour it protects. A
real duplicate-start/stop check needs a booted server with a backend, which
belongs with the e2e job rather than the image build — left as
`TODO(docker-ci)` in the workflow rather than half-done here.
--
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]
