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

src/StackExchange.Redis/ClientInfo.cs

@@ -235,7 +235,7 @@ internal static bool TryParse(string? input, [NotNullWhen(true)] out ClientInfo[
 
                         switch (key)
                         {
-                            case "addr" when Format.TryParseEndPoint(value, out var addr): client.Address = addr; break;
+                            case "addr" when Format.TryParseServerEndPoint(value, out var addr): client.Address = addr; break;
                             case "age": client.AgeSeconds = Format.ParseInt32(value); break;
                             case "idle": client.IdleSeconds = Format.ParseInt32(value); break;
                             case "db": client.Database = Format.ParseInt32(value); break;

src/StackExchange.Redis/ClusterConfiguration.cs

@@ -371,7 +371,7 @@ internal ClusterNode(ClusterConfiguration configuration, string raw, EndPoint or
             }
             AuxFields ??= [];
 
-            if (Format.TryParseEndPoint(ep, out var epResult))
+            if (Format.TryParseServerEndPoint(ep, out var epResult))
             {
                 EndPoint = epResult;
             }

src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs

@@ -48,7 +48,7 @@ internal void InitializeSentinel(ILogger? log)
                 {
                     string[] messageParts = ((string)message!).Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
                     // We don't care about the result of this - we're just trying
-                    _ = Format.TryParseEndPoint(string.Format("{0}:{1}", messageParts[1], messageParts[2]), out var switchBlame);
+                    _ = Format.TryParseEndPoint(messageParts[1], messageParts[2], out var switchBlame);
 
                     // Resolve the child under the lock, but run the (potentially multi-second,
                     // blocking) SwitchPrimary reconfigure OUTSIDE it: otherwise a failover for one

src/StackExchange.Redis/Format.cs

@@ -65,6 +65,31 @@ internal static bool TryParseEndPoint(string host, string? port, [NotNullWhen(tr
             return false;
         }
 
+        /// <summary>
+        /// Parses an <c>ip:port</c> pair as the server writes it (<c>CLUSTER NODES</c>, <c>MOVED</c>/<c>ASK</c>,
+        /// <c>CLIENT LIST</c>, the <c>MOVING</c>/<c>SMIGRATED</c> maintenance notifications): an IPv6 address is <b>not</b> bracketed and a port is always present, for example
+        /// <c>2a06:4e80:3:a::a:7000</c>. Splitting at the last colon is only safe when a port is known to follow,
+        /// which is why this is kept apart from <see cref="TryParseEndPoint(string?, out EndPoint?)"/> (the RFC 3986
+        /// form used in configuration strings, where <c>::1</c> is an address with no port). Anything that is not
+        /// an unbracketed IPv6 address followed by a port is handed to that method unchanged.
+        /// </summary>
+        internal static bool TryParseServerEndPoint(string? value, [NotNullWhen(true)] out EndPoint? endpoint)
+        {
+            if (value is not null && value.IndexOf(']') < 0)
+            {
+                int lastColon = value.LastIndexOf(':');
+                if (lastColon > 0 && value.IndexOf(':') != lastColon
+                    && TryParseInt32(value.Substring(lastColon + 1), out int port)
+                    && port is >= IPEndPoint.MinPort and <= IPEndPoint.MaxPort
+                    && IPAddress.TryParse(value.Substring(0, lastColon), out var address))
+                {
+                    endpoint = new IPEndPoint(address, port);
+                    return true;
+                }
+            }
+            return TryParseEndPoint(value, out endpoint);
+        }
+
         internal static string ToString(long value) => value.ToString(NumberFormatInfo.InvariantInfo);
 
         internal static string ToString(ulong value) => value.ToString(NumberFormatInfo.InvariantInfo);

src/StackExchange.Redis/PhysicalConnection.Maintenance.cs

@@ -258,8 +258,13 @@ private List<ClusterSlotMigration> ReadSlotMigrations(ref RespReader reader)
     /// <summary>
     /// Parses one end of a migration, treating the placeholder forms as "not named" rather than as an error.
     /// </summary>
+    /// <remarks>
+    /// This is server-emitted <c>ip:port</c> text, so it takes the same parser as <c>CLUSTER NODES</c> and
+    /// <c>MOVED</c>: an unbracketed IPv6 address read as a configuration string parses as an address with port 0,
+    /// which the placeholder check below would then silently discard.
+    /// </remarks>
     private static EndPoint? ParseMigrationEndPoint(string? value)
-        => string.IsNullOrEmpty(value) || value == "?" || !Format.TryParseEndPoint(value, out var parsed) || IsPlaceholderEndPoint(parsed)
+        => string.IsNullOrEmpty(value) || value == "?" || !Format.TryParseServerEndPoint(value, out var parsed) || IsPlaceholderEndPoint(parsed)
             ? null
             : parsed;
 

src/StackExchange.Redis/ResultProcessor.cs

@@ -324,7 +324,7 @@ private bool HandleCommonError(Message message, RespReader reader, PhysicalConne
                 log = false;
                 string[] parts = reader.ReadString()!.Split(StringSplits.Space, 3);
                 if (Format.TryParseInt32(parts[1], out int hashSlot)
-                    && Format.TryParseEndPoint(parts[2], out var endpoint)
+                    && Format.TryParseServerEndPoint(parts[2], out var endpoint)
                     && IsUnroutableRedirect(endpoint))
                 {
                     // the slot moved, but the reply does not say where to: a hostname-preferring node
@@ -341,7 +341,7 @@ private bool HandleCommonError(Message message, RespReader reader, PhysicalConne
                         : "The server redirected to an endpoint that does not identify a node that can be connected to; a topology refresh has been requested. ";
                 }
                 else if (Format.TryParseInt32(parts[1], out hashSlot)
-                    && Format.TryParseEndPoint(parts[2], out endpoint))
+                    && Format.TryParseServerEndPoint(parts[2], out endpoint))
                 {
                     // Check if MOVED points to same endpoint
                     bool isSameEndpoint = Equals(server?.EndPoint, endpoint);

tests/StackExchange.Redis.Tests/ClientInfoParseUnitTests.cs

@@ -0,0 +1,33 @@
+using System.Net;
+using Xunit;
+
+namespace StackExchange.Redis.Tests;
+
+/// <summary>
+/// Parsing the raw text of <c>CLIENT LIST</c>, independent of any server.
+/// </summary>
+public class ClientInfoParseUnitTests
+{
+    [Fact]
+    public void UnbracketedIPv6AddressIsParsed()
+    {
+        // the server does not bracket IPv6 in "addr", so the port is the text after the *last* colon (#1422)
+        const string Raw = "id=8897064 addr=fd40:8017:11:7032:7011:600:ac18:bc:54265 fd=60 name=i-230510-t age=1548 idle=40 flags=P db=0 sub=2 psub=0 multi=-1 qbuf=0 qbuf-free=0 argv-mem=0 obl=0 oll=0 omem=0 tot-mem=20496 ow=0 owmem=0 events=r cmd=ping user=default numops=31\n";
+
+        Assert.True(ClientInfo.TryParse(Raw, out var clients));
+        var client = Assert.Single(clients);
+        Assert.Equal(new IPEndPoint(IPAddress.Parse("fd40:8017:11:7032:7011:600:ac18:bc"), 54265), client.Address);
+        Assert.Equal(8897064, client.Id);
+        Assert.Equal("i-230510-t", client.Name);
+    }
+
+    [Fact]
+    public void IPv4AddressIsParsed()
+    {
+        const string Raw = "id=3 addr=127.0.0.1:52555 fd=8 name= age=855 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 obl=0 oll=0 omem=0 events=r cmd=client\n";
+
+        Assert.True(ClientInfo.TryParse(Raw, out var clients));
+        var client = Assert.Single(clients);
+        Assert.Equal(new IPEndPoint(IPAddress.Loopback, 52555), client.Address);
+    }
+}

tests/StackExchange.Redis.Tests/ClusterNodeParseUnitTests.cs

@@ -46,6 +46,18 @@ public void PreFourZeroLineHasNoBusPort()
         Assert.Empty(node.AuxFields);
     }
 
+    [Theory]
+    [InlineData("abc 2a06:4e80:3:a::a:7000@17000")]
+    [InlineData("abc 2a06:4e80:3:a::a:7000")] // pre-4.0 form, no bus port
+    public void UnbracketedIPv6AddressIsParsed(string prefix)
+    {
+        // CLUSTER NODES does not bracket IPv6 addresses, so the port is the text after the *last* colon (#1422);
+        // before the fix this parsed as the (valid!) address 2a06:4e80:3:a::a:7000 with port 0
+        var node = Parse(prefix + Flags);
+
+        Assert.Equal(new IPEndPoint(IPAddress.Parse("2a06:4e80:3:a::a"), 7000), node.EndPoint);
+    }
+
     [Fact]
     public void HostnameIsParsed()
     {

tests/StackExchange.Redis.Tests/FormatTests.cs

@@ -120,6 +120,36 @@ public void ParseEndPoint(string data, EndPoint expected, string? expectedFormat
         Assert.Equal(expectedFormat, s);
     }
 
+    public static IEnumerable<object?[]> ServerEndpointData()
+    {
+        // what the server writes in CLUSTER NODES / MOVED / CLIENT LIST: IPv6 is *not* bracketed, and
+        // a port always follows, so the port is whatever comes after the last colon (#1422)
+        yield return new object?[] { "2a06:4e80:3:a::a:7000", new IPEndPoint(IPAddress.Parse("2a06:4e80:3:a::a"), 7000) };
+        yield return new object?[] { "fd40:8017:11:7032:7011:600:ac18:bc:54265", new IPEndPoint(IPAddress.Parse("fd40:8017:11:7032:7011:600:ac18:bc"), 54265) };
+        yield return new object?[] { "::1:6379", new IPEndPoint(IPAddress.IPv6Loopback, 6379) }; // contrast with EndpointData
+        // everything else is exactly as TryParseEndPoint sees it
+        yield return new object?[] { "[::1]:6379", new IPEndPoint(IPAddress.IPv6Loopback, 6379) };
+        yield return new object?[] { "127.0.0.1:6379", new IPEndPoint(IPAddress.Loopback, 6379) };
+        yield return new object?[] { "host.example.com:6379", new DnsEndPoint("host.example.com", 6379) };
+        yield return new object?[] { "?", new DnsEndPoint("?", 0) };
+    }
+
+    [Theory]
+    [MemberData(nameof(ServerEndpointData))]
+    public void ParseServerEndPoint(string data, EndPoint expected)
+    {
+        Assert.True(Format.TryParseServerEndPoint(data, out var result));
+        Assert.Equal(expected, result);
+    }
+
+    [Fact]
+    public void ParseServerEndPointWithOutOfRangePortDoesNotThrow()
+    {
+        // not a port, so not an unbracketed IPv6 + port; whatever this is, it must not throw
+        Assert.True(Format.TryParseServerEndPoint("::1:99999", out var result));
+        Assert.IsType<DnsEndPoint>(result);
+    }
+
     // UDS endpoints live outside EndpointData because UnixDomainSocketEndPoint does not implement
     // value equality — these compare via ToString instead.
 #if NET

tests/StackExchange.Redis.Tests/MaintenanceNotificationTests.cs

@@ -183,6 +183,43 @@ public async Task MovingWithAPlaceholderYieldsNoEndpoint(string placeholder)
         }
     }
 
+    [Fact]
+    public async Task MovingToAnUnbracketedIPv6AddressIsParsed()
+    {
+        // the server writes IPv6 without brackets (#1422); read as a configuration string that is an address
+        // with no port, which the placeholder check would then drop - losing the one thing MOVING is for
+        var (server, conn, events) = await ConnectAsync(log);
+        using (server)
+        await using (conn)
+        {
+            Assert.True(server.SendRawPush(null, "MOVING", "1", "15", "2a06:4e80:3:a::a:7002") > 0);
+
+            var evt = await events.NextAsync();
+            log.WriteLine(evt.RawMessage ?? "(no message)");
+            Assert.Equal(new IPEndPoint(IPAddress.Parse("2a06:4e80:3:a::a"), 7002), evt.NewEndPoint);
+        }
+    }
+
+    [Fact]
+    public async Task SlotMigrationBetweenUnbracketedIPv6AddressesIsParsed()
+    {
+        var (server, conn, events) = await ConnectAsync(log);
+        using (server)
+        await using (conn)
+        {
+            Assert.Equal(1, server.SendSlotMigrations(null, MaintenanceNotificationKind.SlotMigrated,
+            [
+                ("2a06:4e80:3:a::a:7000", "2a06:4e80:3:a::b:7001", "0-99"),
+            ]));
+
+            var evt = await events.NextAsync();
+            log.WriteLine(evt.RawMessage ?? "(no message)");
+            var migration = Assert.Single(evt.SlotMigrations);
+            Assert.Equal(new IPEndPoint(IPAddress.Parse("2a06:4e80:3:a::a"), 7000), migration.Source);
+            Assert.Equal(new IPEndPoint(IPAddress.Parse("2a06:4e80:3:a::b"), 7001), migration.Target);
+        }
+    }
+
     [Theory]
     [InlineData(MaintenanceNotificationKind.SlotMigrating, MaintenanceNotificationType.SlotMigrating)]
     [InlineData(MaintenanceNotificationKind.SlotMigrated, MaintenanceNotificationType.SlotMigrated)]