From c8a7265d475c24834ed3497140cdf98bbfd50975 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 15:59:00 +0000 Subject: [PATCH] format ipv6 endpoints correctly (#2813) --- src/StackExchange.Redis/Format.cs | 15 +++++- .../ConnectionFailedErrorsTests.cs | 4 +- .../FailoverTests.cs | 10 ++-- .../StackExchange.Redis.Tests/FormatTests.cs | 52 +++++++++++-------- 4 files changed, 49 insertions(+), 32 deletions(-) diff --git a/src/StackExchange.Redis/Format.cs b/src/StackExchange.Redis/Format.cs index 6836a70da..329da4363 100644 --- a/src/StackExchange.Redis/Format.cs +++ b/src/StackExchange.Redis/Format.cs @@ -97,7 +97,20 @@ internal static string ToString(EndPoint? endpoint) if (dns.Port == 0) return dns.Host; return dns.Host + ":" + Format.ToString(dns.Port); case IPEndPoint ip: - if (ip.Port == 0) return ip.Address.ToString(); + var addr = ip.Address.ToString(); + + if (ip.Port == 0) + { + // no port specified; use naked IP + return addr; + } + + if (addr.IndexOf(':') >= 0) + { + // ipv6 with port; use "[IP]:port" notation + return "[" + addr + "]:" + Format.ToString(ip.Port); + } + // ipv4 with port; use "IP:port" notation return ip.Address + ":" + Format.ToString(ip.Port); #if UNIX_SOCKET case UnixDomainSocketEndPoint uds: diff --git a/tests/StackExchange.Redis.Tests/ConnectionFailedErrorsTests.cs b/tests/StackExchange.Redis.Tests/ConnectionFailedErrorsTests.cs index df3802118..d359bd4b4 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionFailedErrorsTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionFailedErrorsTests.cs @@ -178,8 +178,8 @@ public async Task CheckFailureRecovered() { using var conn = Create(keepAlive: 1, connectTimeout: 10000, allowAdmin: true, log: Writer, shared: false); - await RunBlockingSynchronousWithExtraThreadAsync(innerScenario).ForAwait(); - void innerScenario() + await RunBlockingSynchronousWithExtraThreadAsync(InnerScenario).ForAwait(); + void InnerScenario() { conn.GetDatabase(); var server = conn.GetServer(conn.GetEndPoints()[0]); diff --git a/tests/StackExchange.Redis.Tests/FailoverTests.cs b/tests/StackExchange.Redis.Tests/FailoverTests.cs index e40d12a14..fdbd23a94 100644 --- a/tests/StackExchange.Redis.Tests/FailoverTests.cs +++ b/tests/StackExchange.Redis.Tests/FailoverTests.cs @@ -208,10 +208,7 @@ public async Task SubscriptionsSurviveConnectionFailureAsync() var sub = conn.GetSubscriber(); int counter = 0; Assert.True(sub.IsConnected()); - await sub.SubscribeAsync(channel, delegate - { - Interlocked.Increment(ref counter); - }).ConfigureAwait(false); + await sub.SubscribeAsync(channel, (arg1, arg2) => Interlocked.Increment(ref counter)).ConfigureAwait(false); var profile1 = Log(profiler); @@ -257,8 +254,7 @@ await sub.SubscribeAsync(channel, delegate // Ensure we've sent the subscribe command after reconnecting var profile2 = Log(profiler); - //Assert.Equal(1, profile2.Count(p => p.Command == nameof(RedisCommand.SUBSCRIBE))); - + // Assert.Equal(1, profile2.Count(p => p.Command == nameof(RedisCommand.SUBSCRIBE))); Log("Issuing ping after reconnected"); sub.Ping(); @@ -395,7 +391,7 @@ public async Task SubscriptionsSurvivePrimarySwitchAsync() Log(" IsReplica: " + !server.IsReplica); Log(" Type: " + server.ServerType); } - //Skip.Inconclusive("Not enough latency."); + // Skip.Inconclusive("Not enough latency."); } Assert.True(sanityCheck, $"B Connection: {TestConfig.Current.FailoverPrimaryServerAndPort} should be a replica"); Assert.False(bConn.GetServer(TestConfig.Current.FailoverReplicaServerAndPort).IsReplica, $"B Connection: {TestConfig.Current.FailoverReplicaServerAndPort} should be a primary"); diff --git a/tests/StackExchange.Redis.Tests/FormatTests.cs b/tests/StackExchange.Redis.Tests/FormatTests.cs index 8b3c152ed..cf7593a08 100644 --- a/tests/StackExchange.Redis.Tests/FormatTests.cs +++ b/tests/StackExchange.Redis.Tests/FormatTests.cs @@ -11,38 +11,46 @@ public class FormatTests : TestBase { public FormatTests(ITestOutputHelper output) : base(output) { } - public static IEnumerable EndpointData() + public static IEnumerable EndpointData() { + // note: the 3rd arg is for formatting; null means "expect the original string" + // DNS - yield return new object[] { "localhost", new DnsEndPoint("localhost", 0) }; - yield return new object[] { "localhost:6390", new DnsEndPoint("localhost", 6390) }; - yield return new object[] { "bob.the.builder.com", new DnsEndPoint("bob.the.builder.com", 0) }; - yield return new object[] { "bob.the.builder.com:6390", new DnsEndPoint("bob.the.builder.com", 6390) }; + yield return new object?[] { "localhost", new DnsEndPoint("localhost", 0), null }; + yield return new object?[] { "localhost:6390", new DnsEndPoint("localhost", 6390), null }; + yield return new object?[] { "bob.the.builder.com", new DnsEndPoint("bob.the.builder.com", 0), null }; + yield return new object?[] { "bob.the.builder.com:6390", new DnsEndPoint("bob.the.builder.com", 6390), null }; // IPv4 - yield return new object[] { "0.0.0.0", new IPEndPoint(IPAddress.Parse("0.0.0.0"), 0) }; - yield return new object[] { "127.0.0.1", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 0) }; - yield return new object[] { "127.1", new IPEndPoint(IPAddress.Parse("127.1"), 0) }; - yield return new object[] { "127.1:6389", new IPEndPoint(IPAddress.Parse("127.1"), 6389) }; - yield return new object[] { "127.0.0.1:6389", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 6389) }; - yield return new object[] { "127.0.0.1:1", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 1) }; - yield return new object[] { "127.0.0.1:2", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 2) }; - yield return new object[] { "10.10.9.18:2", new IPEndPoint(IPAddress.Parse("10.10.9.18"), 2) }; + yield return new object?[] { "0.0.0.0", new IPEndPoint(IPAddress.Parse("0.0.0.0"), 0), null }; + yield return new object?[] { "127.0.0.1", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 0), null }; + yield return new object?[] { "127.1", new IPEndPoint(IPAddress.Parse("127.1"), 0), "127.0.0.1" }; + yield return new object?[] { "127.1:6389", new IPEndPoint(IPAddress.Parse("127.1"), 6389), "127.0.0.1:6389" }; + yield return new object?[] { "127.0.0.1:6389", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 6389), null }; + yield return new object?[] { "127.0.0.1:1", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 1), null }; + yield return new object?[] { "127.0.0.1:2", new IPEndPoint(IPAddress.Parse("127.0.0.1"), 2), null }; + yield return new object?[] { "10.10.9.18:2", new IPEndPoint(IPAddress.Parse("10.10.9.18"), 2), null }; // IPv6 - yield return new object[] { "::1", new IPEndPoint(IPAddress.Parse("::1"), 0) }; - yield return new object[] { "::1:6379", new IPEndPoint(IPAddress.Parse("::0.1.99.121"), 0) }; // remember your brackets! - yield return new object[] { "[::1]:6379", new IPEndPoint(IPAddress.Parse("::1"), 6379) }; - yield return new object[] { "[::1]", new IPEndPoint(IPAddress.Parse("::1"), 0) }; - yield return new object[] { "[::1]:1000", new IPEndPoint(IPAddress.Parse("::1"), 1000) }; - yield return new object[] { "[2001:db7:85a3:8d2:1319:8a2e:370:7348]", new IPEndPoint(IPAddress.Parse("2001:db7:85a3:8d2:1319:8a2e:370:7348"), 0) }; - yield return new object[] { "[2001:db7:85a3:8d2:1319:8a2e:370:7348]:1000", new IPEndPoint(IPAddress.Parse("2001:db7:85a3:8d2:1319:8a2e:370:7348"), 1000) }; + yield return new object?[] { "::1", new IPEndPoint(IPAddress.Parse("::1"), 0), null }; + yield return new object?[] { "::1:6379", new IPEndPoint(IPAddress.Parse("::0.1.99.121"), 0), "::0.1.99.121" }; // remember your brackets! + yield return new object?[] { "[::1]:6379", new IPEndPoint(IPAddress.Parse("::1"), 6379), null }; + yield return new object?[] { "[::1]", new IPEndPoint(IPAddress.Parse("::1"), 0), "::1" }; + yield return new object?[] { "[::1]:1000", new IPEndPoint(IPAddress.Parse("::1"), 1000), null }; + yield return new object?[] { "2001:db7:85a3:8d2:1319:8a2e:370:7348", new IPEndPoint(IPAddress.Parse("2001:db7:85a3:8d2:1319:8a2e:370:7348"), 0), null }; + yield return new object?[] { "[2001:db7:85a3:8d2:1319:8a2e:370:7348]", new IPEndPoint(IPAddress.Parse("2001:db7:85a3:8d2:1319:8a2e:370:7348"), 0), "2001:db7:85a3:8d2:1319:8a2e:370:7348" }; + yield return new object?[] { "[2001:db7:85a3:8d2:1319:8a2e:370:7348]:1000", new IPEndPoint(IPAddress.Parse("2001:db7:85a3:8d2:1319:8a2e:370:7348"), 1000), null }; } [Theory] [MemberData(nameof(EndpointData))] - public void ParseEndPoint(string data, EndPoint expected) + public void ParseEndPoint(string data, EndPoint expected, string? expectedFormat) { - _ = Format.TryParseEndPoint(data, out var result); + Assert.True(Format.TryParseEndPoint(data, out var result)); Assert.Equal(expected, result); + + // and write again + var s = Format.ToString(result); + expectedFormat ??= data; + Assert.Equal(expectedFormat, s); } [Theory]