[5.0.1] Hang on infinite timeout and managed SNI (#1798)

Author: JRahnama

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Net;
 using System.Net.Security;
 using System.Security.Cryptography.X509Certificates;
 
@@ -194,6 +195,15 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C
             }
         }
 
+        internal static IPAddress[] GetDnsIpAddresses(string serverName)
+        {
+            using (TrySNIEventScope.Create(nameof(GetDnsIpAddresses)))
+            {
+                SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.INFO, "Getting DNS host entries for serverName {0}.", args0: serverName);
+                return Dns.GetHostAddresses(serverName);
+            }
+        }
+
         /// <summary>
         /// Sets last error encountered for SNI
         /// </summary>

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs

@@ -299,14 +299,7 @@ private Socket TryConnectParallel(string hostName, int port, TimeSpan ts, bool i
             Socket availableSocket = null;
             Task<Socket> connectTask;
 
-            Task<IPAddress[]> serverAddrTask = Dns.GetHostAddressesAsync(hostName);
-            bool complete = serverAddrTask.Wait(ts);
-
-            // DNS timed out - don't block
-            if (!complete)
-                return null;
-
-            IPAddress[] serverAddresses = serverAddrTask.Result;
+            IPAddress[] serverAddresses = SNICommon.GetDnsIpAddresses(hostName);
 
             if (serverAddresses.Length > MaxParallelIpAddresses)
             {
@@ -358,14 +351,7 @@ private static Socket Connect(string serverName, int port, TimeSpan timeout, boo
         {
             SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNITCPHandle), EventType.INFO, "IP preference : {0}", Enum.GetName(typeof(SqlConnectionIPAddressPreference), ipPreference));
 
-            Task<IPAddress[]> serverAddrTask = Dns.GetHostAddressesAsync(serverName);
-            bool complete = serverAddrTask.Wait(timeout);
-
-            // DNS timed out - don't block
-            if (!complete)
-                return null;
-
-            IPAddress[] ipAddresses = serverAddrTask.Result;
+            IPAddress[] ipAddresses = SNICommon.GetDnsIpAddresses(serverName);
 
             string IPv4String = null;
             string IPv6String = null;

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SSRP.cs

@@ -170,7 +170,16 @@ private static byte[] SendUDPRequest(string browserHostname, int port, byte[] re
                 Debug.Assert(port >= 0 && port <= 65535, "Invalid port");
                 Debug.Assert(requestPacket != null && requestPacket.Length > 0, "requestPacket should not be null or 0-length array");
 
-                bool isIpAddress = IPAddress.TryParse(browserHostname, out IPAddress address);
+                if (IPAddress.TryParse(browserHostname, out IPAddress address))
+                {
+                    SsrpResult response = SendUDPRequest(new IPAddress[] { address }, port, requestPacket, allIPsInParallel);
+                    if (response != null && response.ResponsePacket != null)
+                        return response.ResponsePacket;
+                    else if (response != null && response.Error != null)
+                        throw response.Error;
+                    else
+                        return null;
+                }
 
                 TimeSpan ts = default;
                 // In case the Timeout is Infinite, we will receive the max value of Int64 as the tick count
@@ -181,26 +190,7 @@ private static byte[] SendUDPRequest(string browserHostname, int port, byte[] re
                     ts = ts.Ticks < 0 ? TimeSpan.FromTicks(0) : ts;
                 }
 
-                IPAddress[] ipAddresses = null;
-                if (!isIpAddress)
-                {
-                    Task<IPAddress[]> serverAddrTask = Dns.GetHostAddressesAsync(browserHostname);
-                    bool taskComplete;
-                    try
-                    {
-                        taskComplete = serverAddrTask.Wait(ts);
-                    }
-                    catch (AggregateException ae)
-                    {
-                        throw ae.InnerException;
-                    }
-
-                    // If DNS took too long, need to return instead of blocking
-                    if (!taskComplete)
-                        return null;
-
-                    ipAddresses = serverAddrTask.Result;
-                }
+                IPAddress[] ipAddresses = SNICommon.GetDnsIpAddresses(browserHostname);
 
                 Debug.Assert(ipAddresses.Length > 0, "DNS should throw if zero addresses resolve");
 
@@ -278,7 +268,7 @@ private static SsrpResult SendUDPRequest(IPAddress[] ipAddresses, int port, byte
                 for (int i = 0; i < ipAddresses.Length; i++)
                 {
                     IPEndPoint endPoint = new IPEndPoint(ipAddresses[i], port);
-                    tasks.Add(Task.Factory.StartNew<SsrpResult>(() => SendUDPRequest(endPoint, requestPacket)));
+                    tasks.Add(Task.Factory.StartNew<SsrpResult>(() => SendUDPRequest(endPoint, requestPacket), cts.Token));
                 }
 
                 List<Task<SsrpResult>> completedTasks = new();

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs

@@ -88,6 +88,21 @@ public static void EnvironmentHostNameSPIDTest()
             Assert.True(false, "No non-empty hostname found for the application");
         }
 
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static async void ConnectionTimeoutInfiniteTest()
+        {
+            // Exercise the special-case infinite connect timeout code path
+            SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString)
+            {
+                ConnectTimeout = 0 // Infinite
+            };
+
+            using SqlConnection conn = new(builder.ConnectionString);
+            CancellationTokenSource cts = new(30000);
+            // Will throw TaskCanceledException and fail the test in the event of a hang
+            await conn.OpenAsync(cts.Token);
+        }
+
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
         public static void ConnectionTimeoutTestWithThread()
         {

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Net;
 using System.Net.Sockets;
 using System.Text;
 using System.Threading.Tasks;
@@ -12,17 +13,17 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
 {
     public static class InstanceNameTest
     {
-        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))]
         public static void ConnectToSQLWithInstanceNameTest()
         {
             SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString);
 
             bool proceed = true;
             string dataSourceStr = builder.DataSource.Replace("tcp:", "");
             string[] serverNamePartsByBackSlash = dataSourceStr.Split('\\');
+            string hostname = serverNamePartsByBackSlash[0];
             if (!dataSourceStr.Contains(",") && serverNamePartsByBackSlash.Length == 2)
             {
-                string hostname = serverNamePartsByBackSlash[0];
                 proceed = !string.IsNullOrWhiteSpace(hostname) && IsBrowserAlive(hostname);
             }
 
@@ -31,6 +32,17 @@ public static void ConnectToSQLWithInstanceNameTest()
                 using SqlConnection connection = new(builder.ConnectionString);
                 connection.Open();
                 connection.Close();
+
+                if (builder.Encrypt != SqlConnectionEncryptOption.Strict)
+                {
+                    // Exercise the IP address-specific code in SSRP
+                    IPAddress[] addresses = Dns.GetHostAddresses(hostname);
+                    builder.DataSource = builder.DataSource.Replace(hostname, addresses[0].ToString());
+                    builder.TrustServerCertificate = true;
+                    using SqlConnection connection2 = new(builder.ConnectionString);
+                    connection2.Open();
+                    connection2.Close();
+                }
             }
         }