diff --git a/src/StackExchange.Redis/ClientInfo.cs b/src/StackExchange.Redis/ClientInfo.cs index 511aee723..3ea71203d 100644 --- a/src/StackExchange.Redis/ClientInfo.cs +++ b/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; diff --git a/src/StackExchange.Redis/ClusterConfiguration.cs b/src/StackExchange.Redis/ClusterConfiguration.cs index f642a3870..0b2fb7b48 100644 --- a/src/StackExchange.Redis/ClusterConfiguration.cs +++ b/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; } diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs index 31f5296ce..dc8daba80 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs +++ b/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 diff --git a/src/StackExchange.Redis/Format.cs b/src/StackExchange.Redis/Format.cs index 0c4e33272..a7b65e899 100644 --- a/src/StackExchange.Redis/Format.cs +++ b/src/StackExchange.Redis/Format.cs @@ -65,6 +65,31 @@ internal static bool TryParseEndPoint(string host, string? port, [NotNullWhen(tr return false; } + /// + /// Parses an ip:port pair as the server writes it (CLUSTER NODES, MOVED/ASK, + /// CLIENT LIST, the MOVING/SMIGRATED maintenance notifications): an IPv6 address is not bracketed and a port is always present, for example + /// 2a06:4e80:3:a::a:7000. Splitting at the last colon is only safe when a port is known to follow, + /// which is why this is kept apart from (the RFC 3986 + /// form used in configuration strings, where ::1 is an address with no port). Anything that is not + /// an unbracketed IPv6 address followed by a port is handed to that method unchanged. + /// + 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); diff --git a/src/StackExchange.Redis/PhysicalConnection.Maintenance.cs b/src/StackExchange.Redis/PhysicalConnection.Maintenance.cs index a993b2a69..68635dca0 100644 --- a/src/StackExchange.Redis/PhysicalConnection.Maintenance.cs +++ b/src/StackExchange.Redis/PhysicalConnection.Maintenance.cs @@ -258,8 +258,13 @@ private List ReadSlotMigrations(ref RespReader reader) /// /// Parses one end of a migration, treating the placeholder forms as "not named" rather than as an error. /// + /// + /// This is server-emitted ip:port text, so it takes the same parser as CLUSTER NODES and + /// MOVED: 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. + /// 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; diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index 9b2180301..7df8b8321 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/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); diff --git a/tests/StackExchange.Redis.Tests/ClientInfoParseUnitTests.cs b/tests/StackExchange.Redis.Tests/ClientInfoParseUnitTests.cs new file mode 100644 index 000000000..2d0cb1d5b --- /dev/null +++ b/tests/StackExchange.Redis.Tests/ClientInfoParseUnitTests.cs @@ -0,0 +1,33 @@ +using System.Net; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Parsing the raw text of CLIENT LIST, independent of any server. +/// +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); + } +} diff --git a/tests/StackExchange.Redis.Tests/ClusterNodeParseUnitTests.cs b/tests/StackExchange.Redis.Tests/ClusterNodeParseUnitTests.cs index 8cade9fb8..1d46a6f6c 100644 --- a/tests/StackExchange.Redis.Tests/ClusterNodeParseUnitTests.cs +++ b/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() { diff --git a/tests/StackExchange.Redis.Tests/FormatTests.cs b/tests/StackExchange.Redis.Tests/FormatTests.cs index 886ea794e..1cab95d96 100644 --- a/tests/StackExchange.Redis.Tests/FormatTests.cs +++ b/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 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(result); + } + // UDS endpoints live outside EndpointData because UnixDomainSocketEndPoint does not implement // value equality — these compare via ToString instead. #if NET diff --git a/tests/StackExchange.Redis.Tests/MaintenanceNotificationTests.cs b/tests/StackExchange.Redis.Tests/MaintenanceNotificationTests.cs index 1e197f48c..bfdb3a7a5 100644 --- a/tests/StackExchange.Redis.Tests/MaintenanceNotificationTests.cs +++ b/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)]