Timeout support for unprivileged Ping on Linux/BSD (#35461)
authorSRV <roman.sakno@gmail.com>
Mon, 25 May 2020 17:59:59 +0000 (20:59 +0300)
committerGitHub <noreply@github.com>
Mon, 25 May 2020 17:59:59 +0000 (10:59 -0700)
* Added support of timeout when unprivileged ping is used

* Fixed test according with new timeout parameter

* Fixes code style issues

* Fixed command-line flag for ping6 on BSD

* Separated timeout test

* Removed trailing whitespace

* Workaround for OSX ping6

* Removed static local method

* Suppress Ping stdout

* Fixed variable name

* Added test for zero timeout

* Handle zero timeout correctly

src/libraries/Common/src/System/Net/NetworkInformation/UnixCommandLinePing.cs
src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs
src/libraries/System.Net.Ping/tests/FunctionalTests/UnixPingUtilityTests.cs

index 157986f..a4d3143 100644 (file)
@@ -19,7 +19,8 @@ namespace System.Net.NetworkInformation
 
         private static readonly string? s_discoveredPing4UtilityPath = GetPingUtilityPath(ipv4: true);
         private static readonly string? s_discoveredPing6UtilityPath = GetPingUtilityPath(ipv4: false);
-        private static readonly bool s_isBSD = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD"));
+        private static readonly bool s_isOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
+        private static readonly bool s_isBSD = RuntimeInformation.IsOSPlatform(OSPlatform.FreeBSD);
         private static readonly Lazy<bool> s_isBusybox = new Lazy<bool>(() => IsBusyboxPing(s_discoveredPing4UtilityPath));
 
         // We don't want to pick up an arbitrary or malicious ping
@@ -72,14 +73,59 @@ namespace System.Net.NetworkInformation
         /// Constructs command line arguments appropriate for the ping or ping6 utility.
         /// </summary>
         /// <param name="packetSize">The packet size to use in the ping. Exact packet payload cannot be specified.</param>
+        /// <param name="timeout">The timeout to use in the ping, in milliseconds.</param>
         /// <param name="address">A string representation of the IP address to ping.</param>
         /// <returns>The constructed command line arguments, which can be passed to ping or ping6.</returns>
-        public static string ConstructCommandLine(int packetSize, string address, bool ipv4, int ttl = 0, PingFragmentOptions fragmentOption = PingFragmentOptions.Default)
+        public static string ConstructCommandLine(int packetSize, int timeout, string address, bool ipv4, int ttl = 0, PingFragmentOptions fragmentOption = PingFragmentOptions.Default)
         {
-
-            StringBuilder sb = new StringBuilder();
+            var sb = new StringBuilder();
             sb.Append("-c 1"); // Just send a single ping ("count = 1")
 
+            //if timeout is zero then some ping implementations can stuck infinitely if endpoint is unreachable
+            if (timeout == 0)
+                timeout = 1;
+
+            // Pass timeout argument to ping utility
+            // BusyBox, Linux: ping and ping6 requires -W flag which accepts timeout in SECONDS.
+            // FreeBSD: ping requires -W flag which accepts timeout in MILLISECONDS;
+            // ping6 requires -x which accepts timeout in MILLISECONDS
+            // OSX: ping requires -W flag which accepts timeout in MILLISECONDS; ping6 doesn't support timeout
+            if (s_isBSD)
+            {
+                if (ipv4)
+                {
+                    sb.Append(" -W ");
+                }
+                else
+                {
+                    sb.Append(" -x ");
+                }
+            }
+            else if (s_isOSX)
+            {
+                if (ipv4)
+                {
+                    sb.Append(" -W ");
+                }
+                else
+                {
+                    goto skipped_timeout;
+                }
+            }
+            else
+            {
+                sb.Append(" -W ");
+                const int millisInSecond = 1000;
+                timeout = Math.DivRem(timeout, millisInSecond, out int remainder);
+                if (remainder != 0)
+                {
+                    timeout += 1;
+                }
+            }
+            sb.Append(timeout);
+
+        skipped_timeout:
+
             // The command-line flags for "Do-not-fragment" and "TTL" are not standard.
             // In fact, they are different even between ping and ping6 on the same machine.
 
@@ -88,7 +134,7 @@ namespace System.Net.NetworkInformation
 
             if (ttl > 0)
             {
-                if (s_isBSD)
+                if (s_isBSD | s_isOSX)
                 {
                     // OSX and FreeBSD use -h to set hop limit for IPv6 and -m ttl for IPv4
                     if (ipv4)
@@ -109,9 +155,9 @@ namespace System.Net.NetworkInformation
                 sb.Append(ttl);
             }
 
-            if (fragmentOption != PingFragmentOptions.Default )
+            if (fragmentOption != PingFragmentOptions.Default)
             {
-                if (s_isBSD)
+                if (s_isBSD | s_isOSX)
                 {
                     // The bit is off by default on OSX & FreeBSD
                     if (fragmentOption == PingFragmentOptions.Dont) {
index 0d7af8c..9b5c56e 100644 (file)
@@ -278,7 +278,7 @@ namespace System.Net.NetworkInformation
             }
         }
 
-        private Process GetPingProcess(IPAddress address, byte[] buffer, PingOptions? options)
+        private Process GetPingProcess(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
         {
             bool isIpv4 = address.AddressFamily == AddressFamily.InterNetwork;
             string? pingExecutable = isIpv4 ? UnixCommandLinePing.Ping4UtilityPath : UnixCommandLinePing.Ping6UtilityPath;
@@ -293,7 +293,7 @@ namespace System.Net.NetworkInformation
                 fragmentOption = options.DontFragment ? UnixCommandLinePing.PingFragmentOptions.Do : UnixCommandLinePing.PingFragmentOptions.Dont;
             }
 
-            string processArgs = UnixCommandLinePing.ConstructCommandLine(buffer.Length, address.ToString(), isIpv4, options?.Ttl ?? 0, fragmentOption);
+            string processArgs = UnixCommandLinePing.ConstructCommandLine(buffer.Length, timeout, address.ToString(), isIpv4, options?.Ttl ?? 0, fragmentOption);
 
             ProcessStartInfo psi = new ProcessStartInfo(pingExecutable, processArgs);
             psi.RedirectStandardOutput = true;
@@ -303,7 +303,7 @@ namespace System.Net.NetworkInformation
 
         private PingReply SendWithPingUtility(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
         {
-            using (Process p = GetPingProcess(address, buffer, options))
+            using (Process p = GetPingProcess(address, buffer, timeout, options))
             {
                 p.Start();
                 if (!p.WaitForExit(timeout) || p.ExitCode == 1 || p.ExitCode == 2)
@@ -326,7 +326,7 @@ namespace System.Net.NetworkInformation
 
         private async Task<PingReply> SendWithPingUtilityAsync(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
         {
-            using (Process p = GetPingProcess(address, buffer, options))
+            using (Process p = GetPingProcess(address, buffer, timeout, options))
             {
                 var processCompletion = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
                 p.EnableRaisingEvents = true;
index 7711aff..899a2ea 100644 (file)
@@ -21,42 +21,56 @@ namespace System.Net.NetworkInformation.Tests
 
         [Theory]
         [InlineData(0)]
+        [InlineData(100)]
+        [InlineData(1000)]
+        [InlineData(1500)]
+        [PlatformSpecific(TestPlatforms.AnyUnix)]
+        public static void TimeoutIsRespected(int timeout)
+        {
+            Process p = ConstructPingProcess(IPAddress.Parse(TestSettings.UnreachableAddress), 50, timeout);
+            //suppress Ping output to console/terminal stdout during test execution
+            p.StartInfo.RedirectStandardError = true;
+            p.StartInfo.RedirectStandardOutput = true;
+
+            Stopwatch stopWatch = Stopwatch.StartNew();
+                        
+            p.Start();
+            p.WaitForExit();
+
+            //ensure that the process takes longer than or equal to 'timeout'
+            Assert.True(stopWatch.ElapsedMilliseconds >= timeout);
+        }
+
+        [Theory]
+        [InlineData(0)]
         [InlineData(1)]
         [InlineData(50)]
         [InlineData(1000)]
         [PlatformSpecific(TestPlatforms.AnyUnix)] // Tests un-priviledged Ping support on Unix
         public static async Task PacketSizeIsRespected(int payloadSize)
         {
-            IPAddress localAddress = await TestSettings.GetLocalIPAddressAsync();
-            bool ipv4 = localAddress.AddressFamily == AddressFamily.InterNetwork;
-            string arguments = UnixCommandLinePing.ConstructCommandLine(payloadSize, localAddress.ToString(), ipv4);
-            string utilityPath = (localAddress.AddressFamily == AddressFamily.InterNetwork)
-                ? UnixCommandLinePing.Ping4UtilityPath
-                : UnixCommandLinePing.Ping6UtilityPath;
-
-            var p = new Process();
-            p.StartInfo.FileName = utilityPath;
-            p.StartInfo.Arguments = arguments;
-            p.StartInfo.UseShellExecute = false;
+            var stdOutLines = new List<string>();
+            var stdErrLines = new List<string>();
 
+            Process p = ConstructPingProcess(await TestSettings.GetLocalIPAddressAsync(), payloadSize, 1000);
             p.StartInfo.RedirectStandardOutput = true;
-            var stdOutLines = new List<string>();
-            p.OutputDataReceived += new DataReceivedEventHandler(
-                delegate (object sendingProcess, DataReceivedEventArgs outputLine) { stdOutLines.Add(outputLine.Data); });
+            p.OutputDataReceived += delegate (object sendingProcess, DataReceivedEventArgs outputLine) 
+            { 
+                stdOutLines.Add(outputLine.Data); 
+            };
 
             p.StartInfo.RedirectStandardError = true;
-            var stdErrLines = new List<string>();
-            p.ErrorDataReceived += new DataReceivedEventHandler(
-                delegate (object sendingProcess, DataReceivedEventArgs errorLine) { stdErrLines.Add(errorLine.Data); });
+            p.ErrorDataReceived += delegate (object sendingProcess, DataReceivedEventArgs errorLine) 
+            { 
+                stdErrLines.Add(errorLine.Data); 
+            };
 
             p.Start();
             p.BeginOutputReadLine();
             p.BeginErrorReadLine();
 
             // There are multiple issues with ping6 in macOS 10.12 (Sierra), see https://github.com/dotnet/runtime/issues/24682.
-            bool isPing6OnMacSierra = utilityPath.Equals(UnixCommandLinePing.Ping6UtilityPath) &&
-                    RuntimeInformation.IsOSPlatform(OSPlatform.OSX) &&
-                    !PlatformDetection.IsMacOsHighSierraOrHigher;
+            bool isPing6OnMacSierra = IsPing6OnMacSierra(p.StartInfo);
 
             string pingOutput;
             if (!p.WaitForExit(TestSettings.PingTimeout))
@@ -68,7 +82,7 @@ namespace System.Net.NetworkInformation.Tests
                 pingOutput = string.Join("\n", stdOutLines);
                 string stdErr = string.Join("\n", stdErrLines);
                 throw new Exception(
-                    $"[{utilityPath} {arguments}] process did not exit in {TestSettings.PingTimeout} ms.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]");
+                    $"[{p.StartInfo.FileName} {p.StartInfo.Arguments}] process did not exit in {TestSettings.PingTimeout} ms.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]");
             }
 
             // Ensure standard output and error are flushed
@@ -84,7 +98,7 @@ namespace System.Net.NetworkInformation.Tests
 
                 string stdErr = string.Join("\n", stdErrLines);
                 throw new Exception(
-                    $"[{utilityPath} {arguments}] process exit code is {exitCode}.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]");
+                    $"[{p.StartInfo.FileName} {p.StartInfo.Arguments}] process exit code is {exitCode}.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]");
             }
 
             try
@@ -106,10 +120,33 @@ namespace System.Net.NetworkInformation.Tests
             {
                 string stdErr = string.Join("\n", stdErrLines);
                 throw new Exception(
-                    $"Parse error for [{utilityPath} {arguments}] process exit code is {exitCode}.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]", e);
+                    $"Parse error for [{p.StartInfo.FileName} {p.StartInfo.Arguments}] process exit code is {exitCode}.\nStdOut:[{pingOutput}]\nStdErr:[{stdErr}]", e);
             }
         }
 
+        private static bool IsPing6OnMacSierra(ProcessStartInfo startInfo)
+        {
+            return startInfo.FileName.Equals(UnixCommandLinePing.Ping6UtilityPath) &&
+                    RuntimeInformation.IsOSPlatform(OSPlatform.OSX) &&
+                    !PlatformDetection.IsMacOsHighSierraOrHigher;
+        }
+
+        private static Process ConstructPingProcess(IPAddress localAddress, int payloadSize, int timeout)
+        {
+            bool ipv4 = localAddress.AddressFamily == AddressFamily.InterNetwork;
+            string arguments = UnixCommandLinePing.ConstructCommandLine(payloadSize, timeout, localAddress.ToString(), ipv4);
+            string utilityPath = (localAddress.AddressFamily == AddressFamily.InterNetwork)
+                ? UnixCommandLinePing.Ping4UtilityPath
+                : UnixCommandLinePing.Ping6UtilityPath;
+
+            var p = new Process();
+            p.StartInfo.FileName = utilityPath;
+            p.StartInfo.Arguments = arguments;
+            p.StartInfo.UseShellExecute = false;
+
+            return p;
+        }
+
         private static int ParseReturnedPacketSize(string pingOutput)
         {
             int indexOfBytesFrom = pingOutput.IndexOf("bytes from");