Skip to content

Support IPv6 node URLs and filter wildcard redirect endpoints - #21

Merged
HTHou merged 2 commits into
apache:developfrom
PDGGK:fix/ipv6-node-url
Jul 24, 2026
Merged

Support IPv6 node URLs and filter wildcard redirect endpoints#21
HTHou merged 2 commits into
apache:developfrom
PDGGK:fix/ipv6-node-url

Conversation

@PDGGK

Copy link
Copy Markdown
Contributor

Problem

parseNodeUrls splits each node URL on : and requires exactly two parts, so an IPv6 endpoint such as [::1]:6667 is rejected up front with "Invalid nodeUrl format", making the client unusable with IPv6 nodes. Separately, on the redirect path the client caches and connects to whatever endpoint the server advertises in a redirectNode, with no guard against a wildcard/listen-all address.

Fix

Following the same approach as apache/iotdb#18162:

  • Parsing: parse the bracketed [ipv6]:port form by stripping the brackets and reading the port after them, keeping IPv4 and hostname parsing unchanged. A bare (unbracketed) IPv6 address with a port is ambiguous and rejected; the [ipv6]:port form must be used.
  • Wildcard filtering: after an insert, the server's redirectNode is stored in Session.lastRedirectEndpoint and later cached and connected to by the pool via getAndClearLastRedirect(). That consumption point now ignores a redirect whose address is a wildcard/listen-all address (0.0.0.0, ::), which is not a connectable remote endpoint — the current endpoint is kept.

Tests

  • parseNodeUrls: bracketed IPv6 ([::1]:6667, [2001:db8::1]:6668) and malformed inputs (bare IPv6, unbalanced bracket, missing colon, empty port).
  • isWildcardAddress, and Session.getAndClearLastRedirect on the real path: a wildcard redirect endpoint is dropped, a normal one passes through.
  • Parsing is unit-tested including the [::1] loopback address; live IPv6-loopback connection tests (as in #18162) would need an IPv6-capable CI/test server, which I'm happy to add as a follow-up if the client CI supports it.

Existing tests and tsc pass. Follows the dev@ "[DISCUSS] Improve IPv6 endpoint support" thread; one focused PR per client as @HTHou suggested.

parseNodeUrls split each node URL on ":" and required exactly two parts, so
an IPv6 endpoint such as "[::1]:6667" was rejected up front with "Invalid
nodeUrl format", making the client unusable with IPv6 nodes.
Parse the bracketed [ipv6]:port form (consistent with apache/iotdb#18162),
keeping IPv4 and hostname parsing unchanged; a bare (unbracketed) IPv6
address remains rejected as ambiguous. Also ignore a server redirect
recommendation whose address is a wildcard/listen-all address (0.0.0.0 / ::):
the endpoint set from the server's redirectNode is consumed by
Session.getAndClearLastRedirect() and then cached and connected to by the
pool, so the guard is applied there. Add tests for both.
Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com>

@HTHouHTHou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One endpoint-parsing issue to address.

Comment threadsrc/utils/Config.ts Outdated
parseNodeUrl used parseInt, which accepts a numeric prefix, so
"[::1]:6667junk" and "[::1]:6667:9999" were silently parsed as port
6667. The port must now be an all-digits string before conversion.
Adds regression tests for the bracketed, hostname and IPv4 forms.
Addresses review feedback on apache#21.
Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com>
@PDGGK

Copy link
Copy Markdown
ContributorAuthor

Thanks @HTHou — you're right, parseInt accepting a numeric prefix let malformed endpoints slip through. Fixed: the port must now be an all-digits string (validated with /^\d+$/) before conversion, so [::1]:6667junk and [::1]:6667:9999 — and the hostname/IPv4 equivalents — are rejected with the Invalid nodeUrl format error rather than parsed as 6667. Added regression tests covering all four cases.

@HTHou
HTHou merged commit a8ca4d2 into apache:developJul 24, 2026
4 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@PDGGK@CritasWang@HTHou