Skip to content

Parse unbracketed IPv6 ip:port from the server (#1422) - #3307

Merged
mgravell merged 2 commits into
mainfrom
marc/ipv6-unbracketed-endpoints
Oct 30, 2026
Merged

mgravell merged 2 commits into
mainfrom
marc/ipv6-unbracketed-endpoints

Conversation

@mgravell

@mgravell mgravell commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1422

CLUSTER NODES, MOVED/ASK and CLIENT LIST write IPv6 addresses without brackets, e.g. 2a06:4e80:3:a::a:7000@17000. That text went through the RFC 3986 parser used for configuration strings, which treats it as a valid eight-group IPv6 address with no port. The node therefore surfaced as [2a06:4e80:3:a::a:7000]:0, which is what the issue reports and what the latest comment still reproduces on 3.4.20.

Per the discussion on the issue, the normal endpoint parser is left alone. A new internal Format.TryParseServerEndPoint handles the server's ip:port form: when the text has no ] and more than one :, it splits at the last colon and requires both halves to parse (address and in-range port); anything else is handed to the existing parser unchanged. It is used at the four sites that receive server-emitted text:

  • ClusterNode (CLUSTER NODES)
  • the MOVED/ASK redirect handling in ResultProcessor
  • ClientInfo (CLIENT LIST addr=, the case raised mid-thread)
  • ParseMigrationEndPoint in PhysicalConnection.Maintenance (the MOVING successor and SMIGRATED source/target, raised in review). Here the old behaviour was worse than a wrong port: port 0 looks like the "no address" placeholder, so the endpoint was silently dropped.

The sentinel +switch-master handler receives host and port as separate tokens and was concatenating them with : before parsing, which has the same problem for an IPv6 primary; it now uses the (host, port) overload.

Tests: FormatTests.ParseServerEndPoint (unbracketed IPv6, plus the bracketed, IPv4, hostname and ? cases to show they are unchanged), ClusterNodeParseUnitTests.UnbracketedIPv6AddressIsParsed (with and without the @cport trailer), a new ClientInfoParseUnitTests for the raw CLIENT LIST line, and MaintenanceNotificationTests.MovingToAnUnbracketedIPv6AddressIsParsed / SlotMigrationBetweenUnbracketedIPv6AddressesIsParsed (both fail without the fix). Built on all TFMs; the classes pass on net10.0.

CLUSTER NODES, MOVED/ASK and CLIENT LIST write IPv6 addresses without
brackets, i.e. "2a06:4e80:3:a::a:7000". The RFC 3986 parser used for
configuration strings reads that as a (valid!) eight-group address with
no port, so cluster nodes ended up as "[2a06:4e80:3:a::a:7000]:0" and
redirections and client addresses were equally wrong.

Add Format.TryParseServerEndPoint for the server's "ip:port" form: when
there is no ']' and more than one ':', split at the last colon and
require both halves to parse; anything else goes to the existing parser
unchanged, so configuration parsing is not touched. Use it at the three
sites that receive server-emitted text.

The sentinel +switch-master handler gets host and port as separate
tokens and was gluing them back together with ':' before parsing; pass
them to the (host, port) overload instead.
@mohsinaliwork686-rgb

Copy link
Copy Markdown
Contributor

Thanks, this is more thorough than what I had in mind. One more spot that looks like it has the same problem: ParseMigrationEndPoint in PhysicalConnection.Maintenance.cs (the MOVING endpoint and the SMIGRATED source/target) still goes through TryParseEndPoint. If the server writes an unbracketed IPv6 there, it parses with port 0, and IsPlaceholderEndPoint then treats it as "not named", so the target is silently dropped. I can't see the server side of these notifications, but redis-py splits the MOVING endpoint at the last colon (rsplit(":", 1)), which suggests the same unbracketed form. Might be worth switching it to TryParseServerEndPoint too.

@mgravell

Copy link
Copy Markdown
Collaborator Author

Agreed, same defect and with a worse failure mode: the port-0 result looks like the placeholder form, so the endpoint vanished rather than coming out wrong. ParseMigrationEndPoint now uses TryParseServerEndPoint, with tests for both the MOVING successor and SMIGRATED source/target; both fail against the old code. I can't confirm the server's wire form for these either, but the parser accepts both forms, so it costs nothing if they turn out to be bracketed.

ParseMigrationEndPoint used the configuration-string parser, so an unbracketed IPv6 address parsed as an address with port 0, which the placeholder check then discarded. Use TryParseServerEndPoint, as the other server-emitted sites do; tests for the MOVING successor and SMIGRATED source/target.
@mgravell
mgravell merged commit 35ccc98 into main Oct 30, 2026
6 checks passed
@mgravell
mgravell deleted the marc/ipv6-unbracketed-endpoints branch October 30, 2026 12:19
Sign up for free to 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.

IPv6 Cluster connection errors

2 participants