Fix | Fix unit test for SPN to include port number with Managed SNI

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

@@ -653,10 +653,10 @@ private bool InferConnectionDetails()
 
                 Port = port;
             }
-            // Instance Name Handling. Only if we found a '\' and we did not find a port in the Data Source
-            else if (backSlashIndex > -1)
+            // Instance Name Handling. 
+            if (backSlashIndex > -1)
             {
-                // This means that there will not be any part separated by comma.
+                // This means that there is a part separated by '\'
                 InstanceName = tokensByCommaAndSlash[1].Trim();
 
                 if (string.IsNullOrWhiteSpace(InstanceName))

src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

@@ -979,9 +979,6 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i
             port = -1;
             instanceName = string.Empty;
 
-            if (dataSource.Contains(",") && dataSource.Contains("\\"))
-                return false;
-
             if (dataSource.Contains(":"))
             {
                 dataSource = dataSource.Substring(dataSource.IndexOf(":", StringComparison.Ordinal) + 1);
@@ -993,7 +990,8 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i
                 {
                     return false;
                 }
-                dataSource = dataSource.Substring(0, dataSource.IndexOf(",", StringComparison.Ordinal) - 1);
+                // IndexOf is zero-based, no need to subtract one
+                dataSource = dataSource.Substring(0, dataSource.IndexOf(",", StringComparison.Ordinal));
             }
 
             if (dataSource.Contains("\\"))

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

@@ -84,48 +84,96 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove
             }
         }
 
-        // Note: This Unit test was tested in a domain-joined VM connecting to a remote
-        //       SQL Server using Kerberos in the same domain.
-        [ActiveIssue("27824")] // When specifying instance name and port number, this method call always returns false
-        [ConditionalFact(nameof(IsKerberos))]
-        public static void PortNumberInSPNTest()
+#if NETCOREAPP
+        [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field
+        [ConditionalTheory(nameof(IsSPNPortNumberTestForNP))]
+        [InlineData("")]
+        [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well
+        public static void PortNumberInSPNTestForNP(string port)
         {
-            string connStr = DataTestUtility.TCPConnectionString;
-            // If config.json.SupportsIntegratedSecurity = true, replace all keys defined below with Integrated Security=true 
+            string connectionString = DataTestUtility.NPConnectionString;
+            SqlConnectionStringBuilder builder = new(connectionString);
+
+            if (!string.IsNullOrWhiteSpace(port))
+                builder.DataSource = $"{builder.DataSource},{port}";
+
+            PortNumberInSPNTest(builder.ConnectionString);
+        }
+
+        [ConditionalTheory(nameof(IsSPNPortNumberTestForTCP))]
+        [InlineData("")]
+        [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well
+        public static void PortNumberInSPNTestForTCP(string port)
+        {
+            string connectionString = DataTestUtility.TCPConnectionString;
+            SqlConnectionStringBuilder builder = new(connectionString);
+
+            if (!string.IsNullOrWhiteSpace(port))
+                builder.DataSource = $"{builder.DataSource},{port}";
+
+            PortNumberInSPNTest(builder.ConnectionString);
+        }
+#endif
+
+        private static void PortNumberInSPNTest(string connectionString)
+        {
+            // If config.json.Supports IntegratedSecurity = true, replace all keys defined below with Integrated Security=true 
             if (DataTestUtility.IsIntegratedSecuritySetup())
             {
                 string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection" };
-                connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys) + $"Integrated Security=true";
+                connectionString = DataTestUtility.RemoveKeysInConnStr(connectionString, removeKeys) + $"Integrated Security=true";
+            }
+
+            SqlConnectionStringBuilder builder = new(connectionString);
+
+            string hostname = "";
+            string instanceName = "";
+            int port = -1;
+
+            // Named pipe protocol data source does not support port number
+            string dataSource = builder.DataSource.ToUpper();
+            if ((dataSource.Contains(@"\MSSQL$") && builder.DataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) ||
+               (dataSource.Contains(@"NP:") && dataSource.Contains(",")))
+            {
+                port = -2;
             }
+            Assert.False(port == -2, "Named pipe protocol in data source does not support port number.");
 
-            SqlConnectionStringBuilder builder = new(connStr);
+            DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName);
+
+            Assert.False(string.IsNullOrEmpty(hostname), "Hostname must be included in the data source.");
+            Assert.False(string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source.");
 
-            Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName), "Data source to be parsed must contain a host name and instance name");
+            bool isBrowserRunning = IsBrowserAlive(hostname);
+            Assert.True(isBrowserRunning, "Browser service is not running.");
 
-            bool condition = IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName);
-            Assert.True(condition, "Browser service is not running or instance name is invalid");
+            bool isInstanceExisting = IsValidInstance(hostname, instanceName);
+            Assert.True(isInstanceExisting, "Instance name is invalid.");
 
-            if (condition)
+            if (isBrowserRunning && isInstanceExisting)
             {
-                using SqlConnection connection = new(builder.ConnectionString);
+                // Create a connection object to ensure SPN info is available via reflection
+                SqlConnection connection = new(builder.ConnectionString);
                 connection.Open();
-                using SqlCommand command = new("SELECT auth_scheme, local_tcp_port from sys.dm_exec_connections where session_id = @@spid", connection);
-                using SqlDataReader reader = command.ExecuteReader();
-                Assert.True(reader.Read(), "Expected to receive one row data");
-                Assert.Equal("KERBEROS", reader.GetString(0));
-                int localTcpPort = reader.GetInt32(1);
-
-                int spnPort = -1;
-                string spnInfo = GetSPNInfo(builder.DataSource, out spnPort);
-
-                // sample output to validate = MSSQLSvc/machine.domain.tld:spnPort"
-                Assert.Contains($"MSSQLSvc/{hostname}", spnInfo);
-                // the local_tcp_port should be the same as the inferred SPN port from instance name
-                Assert.Equal(localTcpPort, spnPort);
+
+                // Get the SPN info using reflection
+                string spnInfo = GetSPNInfo(builder.DataSource);
+
+                // The expected output to validate is supposed to be in the format "MSSQLSvc/machine.domain.tld:spnPort".
+                // So, we want to get the port number from the SPN and ensure it is a valid port number.
+                string[] spnStrs = spnInfo.Split(':');
+                int portInSPN = 0;
+                if (spnStrs.Length > 1)
+                {
+                    int.TryParse(spnStrs[1], out portInSPN);
+                }
+                Assert.True(portInSPN > 0, "The expected SPN must include a valid port number.");
+
+                connection.Close();
             }
         }
 
-        private static string GetSPNInfo(string datasource, out int out_port)
+        private static string GetSPNInfo(string dataSource)
         {
             Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection));
 
@@ -156,26 +204,27 @@ private static string GetSPNInfo(string datasource, out int out_port)
             // Instantiate SNIProxy
             object sniProxy = sniProxyCtor.Invoke(new object[] { });
 
-            // Instantiate datasource 
-            object dataSourceObj = dataSourceCtor.Invoke(new object[] { datasource });
+            // Instantiate dataSource 
+            object dataSourceObj = dataSourceCtor.Invoke(new object[] { dataSource });
 
             // Instantiate SSRP
-            object ssrp = SSRPCtor.Invoke(new object[] { });
+            object ssrpObj = SSRPCtor.Invoke(new object[] { });
 
             // Instantiate TimeoutTimer
-            object timeoutTimer = timeoutTimerCtor.Invoke(new object[] { });
+            object timeoutTimerObj = timeoutTimerCtor.Invoke(new object[] { });
 
             // Get TimeoutTimer.StartSecondsTimeout Method
-            MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null);
+            MethodInfo startSecondsTimeout = timeoutTimerObj.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null);
+
             // Create a timeoutTimer that expires in 30 seconds
-            timeoutTimer = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 });
+            timeoutTimerObj = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 });
 
-            // Parse the datasource to separate the server name and instance name
+            // Parse the dataSource to separate the server name and instance name
             MethodInfo ParseServerName = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null);
-            object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { datasource });
+            object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { dataSource });
 
             // Get the GetPortByInstanceName method of SSRP
-            MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null);
+            MethodInfo getPortByInstanceName = ssrpObj.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null);
 
             // Get the server name
             PropertyInfo serverInfo = dataSrcInfo.GetType().GetProperty("ServerName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
@@ -186,9 +235,9 @@ private static string GetSPNInfo(string datasource, out int out_port)
             string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString();
 
             // Get the port number using the GetPortByInstanceName method of SSRP
-            object port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 });
+            object port = getPortByInstanceName.Invoke(ssrpObj, parameters: new object[] { serverName, instanceName, timeoutTimerObj, false, 0 });
 
-            // Set the resolved port property of datasource
+            // Set the resolved port property of dataSource
             PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
             resolvedPortInfo.SetValue(dataSrcInfo, (int)port, null);
 
@@ -202,20 +251,40 @@ private static string GetSPNInfo(string datasource, out int out_port)
             // Example result: MSSQLSvc/machine.domain.tld:port"
             string spnInfo = Encoding.Unicode.GetString(result[0]);
 
-            out_port = (int)port;
-
             return spnInfo;
         }
 
-        private static bool IsKerberos()
+        private static bool IsSPNPortNumberTestForTCP()
+        {
+            return (IsInstanceNameValid(DataTestUtility.TCPConnectionString)
+                 && DataTestUtility.AreConnStringsSetup()
+                 && DataTestUtility.IsUsingManagedSNI()
+                 && DataTestUtility.IsNotAzureServer()
+                 && DataTestUtility.IsNotAzureSynapse());
+        }
+
+        private static bool IsSPNPortNumberTestForNP()
         {
-            return (DataTestUtility.AreConnStringsSetup() 
-                 && DataTestUtility.IsNotLocalhost() 
-                 && DataTestUtility.IsKerberosTest 
-                 && DataTestUtility.IsNotAzureServer() 
+            return (IsInstanceNameValid(DataTestUtility.NPConnectionString)
+                 && DataTestUtility.AreConnStringsSetup()
+                 && DataTestUtility.IsUsingManagedSNI()
+                 && DataTestUtility.IsNotAzureServer()
                  && DataTestUtility.IsNotAzureSynapse());
         }
 
+        private static bool IsInstanceNameValid(string connectionString)
+        {
+            string hostname = "";
+            string instanceName = "";
+            int port = -1;
+
+            SqlConnectionStringBuilder builder = new(connectionString);
+
+            DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName);
+
+            return !string.IsNullOrWhiteSpace(instanceName);
+        }
+
         private static bool IsBrowserAlive(string browserHostname)
         {
             const byte ClntUcastEx = 0x03;