From 4e89f43f415b48f2f4ac58025fdedadf165097d9 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Tue, 10 Jun 2025 22:22:33 +0200 Subject: [PATCH 1/2] harden Ping_TimedOut_* tests --- .../tests/FunctionalTests/PingTest.cs | 81 +++++++++++-------- .../tests/FunctionalTests/TestSettings.cs | 3 + 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs index f21cbce950a543..760c565e3aac10 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs @@ -745,53 +745,64 @@ public async Task SendPingToExternalHostWithLowTtlTest() Assert.NotEqual(IPAddress.Any, pingReply.Address); } - [Fact] - [OuterLoop] - public void Ping_TimedOut_Sync_Success() + private async Task Ping_TimedOut_Core(Func> sendPing) { - var sender = new Ping(); - PingReply reply = sender.Send(TestSettings.UnreachableAddress); + Ping sender = new Ping(); + PingReply reply = await sendPing(sender, TestSettings.UnreachableAddress); + if (reply.Status == IPStatus.DestinationNetworkUnreachable) + { + // The network has dropped the packet towards UnreachableAddress. Repeat the PING attempt on another address. + reply = await sendPing(sender, TestSettings.UnreachableAddress2); + } + + if (reply.Status == IPStatus.DestinationNetworkUnreachable) + { + // Do another attempt. + reply = await sendPing(sender, TestSettings.UnreachableAddress3); + } + Assert.Equal(IPStatus.TimedOut, reply.Status); } [Fact] [OuterLoop] - public async Task Ping_TimedOut_EAP_Success() - { - var sender = new Ping(); - sender.PingCompleted += (s, e) => - { - var tcs = (TaskCompletionSource)e.UserState; + public Task Ping_TimedOut_Sync_Success() + => Ping_TimedOut_Core((sender, address) => Task.Run(() => sender.Send(address))); - if (e.Cancelled) - { - tcs.TrySetCanceled(); - } - else if (e.Error != null) - { - tcs.TrySetException(e.Error); - } - else + [Fact] + [OuterLoop] + public Task Ping_TimedOut_EAP_Success() + => Ping_TimedOut_Core(async (sender, address) => + { + static void PingCompleted(object sender, PingCompletedEventArgs e) { - tcs.TrySetResult(e.Reply); - } - }; + var tcs = (TaskCompletionSource)e.UserState; - var tcs = new TaskCompletionSource(); - sender.SendAsync(TestSettings.UnreachableAddress, tcs); - - PingReply reply = await tcs.Task; - Assert.Equal(IPStatus.TimedOut, reply.Status); - } + if (e.Cancelled) + { + tcs.TrySetCanceled(); + } + else if (e.Error != null) + { + tcs.TrySetException(e.Error); + } + else + { + tcs.TrySetResult(e.Reply); + } + } + sender.PingCompleted += PingCompleted; + var tcs = new TaskCompletionSource(); + sender.SendAsync(address, tcs); + PingReply reply = await tcs.Task; + sender.PingCompleted -= PingCompleted; + return reply; + }); [Fact] [OuterLoop] - public async Task Ping_TimedOut_TAP_Success() - { - var sender = new Ping(); - PingReply reply = await sender.SendPingAsync(TestSettings.UnreachableAddress); - Assert.Equal(IPStatus.TimedOut, reply.Status); - } + public Task Ping_TimedOut_TAP_Success() + => Ping_TimedOut_Core((sender, address) => sender.SendPingAsync(address)); private static bool IsRemoteExecutorSupportedAndPrivilegedProcess => RemoteExecutor.IsSupported && PlatformDetection.IsPrivilegedProcess; diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs index 0e48bb4777b2e1..78b2be986651e6 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs @@ -11,6 +11,9 @@ internal static class TestSettings { public static readonly string LocalHost = "localhost"; public static readonly string UnreachableAddress = "192.0.2.0"; // TEST-NET-1 + public static readonly string UnreachableAddress2 = "100.64.0.1"; // CGNAT block + public static readonly string UnreachableAddress3 = "10.255.255.1"; // High address in the private 10.0.0.0/8 range. Likely unused and unrouted. + public const int PingTimeout = 10 * 1000; public const string PayloadAsString = "'Post hoc ergo propter hoc'. 'After it, therefore because of it'. It means one thing follows the other, therefore it was caused by the other. But it's not always true. In fact it's hardly ever true."; From 4ca61776f25f7e5877d35e22709009b0c219162a Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 11 Jun 2025 13:57:20 +0200 Subject: [PATCH 2/2] improve comments --- .../System.Net.Ping/tests/FunctionalTests/PingTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs index 760c565e3aac10..808887732dd5fd 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs @@ -751,13 +751,13 @@ private async Task Ping_TimedOut_Core(Func> sendPi PingReply reply = await sendPing(sender, TestSettings.UnreachableAddress); if (reply.Status == IPStatus.DestinationNetworkUnreachable) { - // The network has dropped the packet towards UnreachableAddress. Repeat the PING attempt on another address. + // A network middleware has dropped the packed and replied with DestinationNetworkUnreachable. Repeat the PING attempt on another address. reply = await sendPing(sender, TestSettings.UnreachableAddress2); } if (reply.Status == IPStatus.DestinationNetworkUnreachable) { - // Do another attempt. + // Do yet another attempt. reply = await sendPing(sender, TestSettings.UnreachableAddress3); }