diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs index da6885062d..36f834af53 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs @@ -116,7 +116,7 @@ internal static SniHandle CreateConnectionHandle( return sniHandle; } - private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string serverSPN) + internal static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string serverSPN) { Debug.Assert(!string.IsNullOrWhiteSpace(dataSource.ServerName)); if (!string.IsNullOrWhiteSpace(serverSPN)) @@ -132,14 +132,19 @@ private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string } else if (!string.IsNullOrWhiteSpace(dataSource.InstanceName)) { - postfix = dataSource.ResolvedProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName; + // Named Pipes use the instance name in the SPN (MSSQLSvc/host:instance). + // All other protocols (TCP, None, Admin) use the port resolved by SSRP + // (MSSQLSvc/host:port). Protocol.None is the default when no prefix is + // specified in the data source (e.g. "server\instance"), and it is treated + // as TCP for connection purposes. See GitHub issue #3566. + postfix = dataSource.ResolvedProtocol == DataSource.Protocol.NP ? dataSource.InstanceName : dataSource.ResolvedPort.ToString(); } SqlClientEventSource.Log.TryTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerName {0}, InstanceName {1}, Port {2}, postfix {3}", dataSource?.ServerName, dataSource?.InstanceName, dataSource?.Port, postfix); return GetSqlServerSPNs(hostName, postfix, dataSource.ResolvedProtocol); } - private static ResolvedServerSpn GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) + internal static ResolvedServerSpn GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) { Debug.Assert(!string.IsNullOrWhiteSpace(hostNameOrAddress)); IPHostEntry hostEntry = null; @@ -607,8 +612,8 @@ private bool InferNamedPipesInformation() // If the data source starts with "np:servername" if (!_dataSourceAfterTrimmingProtocol.Contains(PipeBeginning)) { - // Assuming that user did not change default NamedPipe name, if the datasource is in the format servername\instance, - // separate servername and instance and prepend instance with MSSQL$ and append default pipe path + // Assuming that user did not change default NamedPipe name, if the datasource is in the format servername\instance, + // separate servername and instance and prepend instance with MSSQL$ and append default pipe path // https://learn.microsoft.com/en-us/sql/tools/configuration-manager/named-pipes-properties?view=sql-server-ver16 if (_dataSourceAfterTrimmingProtocol.Contains(PathSeparator) && ResolvedProtocol == Protocol.NP) { diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniProxyGetSqlServerSPNsTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniProxyGetSqlServerSPNsTest.cs new file mode 100644 index 0000000000..16f0e30215 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniProxyGetSqlServerSPNsTest.cs @@ -0,0 +1,117 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#if NET + +using Microsoft.Data.SqlClient.ManagedSni; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ManagedSni +{ + public class SniProxyGetSqlServerSPNsTest + { + /// + /// Verifies that when connecting to a named instance without a protocol prefix + /// (Protocol.None), the SPN uses the resolved port number from SSRP rather than + /// the instance name. This is a regression test for GitHub issue #3566. + /// + [Fact] + public void GetSqlServerSPNs_ProtocolNone_WithResolvedPort_UsesPortNotInstanceName() + { + // Arrange: parse "server\instance" which sets Protocol.None and IsSsrpRequired + DataSource dataSource = DataSource.ParseServerName(@"server\instance"); + Assert.NotNull(dataSource); + Assert.Equal(DataSource.Protocol.None, dataSource.ResolvedProtocol); + Assert.Equal("instance", dataSource.InstanceName); + Assert.Equal(-1, dataSource.Port); + + // Simulate SSRP resolution setting the port (as CreateTcpHandle would do) + dataSource.ResolvedPort = 12345; + + // Act + ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty); + + // Assert: SPN should contain the resolved port, NOT the instance name + Assert.Contains(":12345", spn.Primary); + Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase); + } + + /// + /// Verifies that when connecting with an explicit tcp: prefix (Protocol.TCP), + /// the SPN uses the resolved port number. This was the original fix for #2187. + /// + [Fact] + public void GetSqlServerSPNs_ProtocolTcp_WithResolvedPort_UsesPort() + { + // Arrange: parse "tcp:server\instance" which sets Protocol.TCP + DataSource dataSource = DataSource.ParseServerName(@"tcp:server\instance"); + Assert.NotNull(dataSource); + Assert.Equal(DataSource.Protocol.TCP, dataSource.ResolvedProtocol); + + dataSource.ResolvedPort = 54321; + + // Act + ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty); + + // Assert + Assert.Contains(":54321", spn.Primary); + Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase); + } + + /// + /// Verifies that when connecting with Named Pipes protocol, the SPN uses + /// the instance name rather than a port number. + /// + [Fact] + public void GetSqlServerSPNs_ProtocolNp_WithInstanceName_UsesInstanceName() + { + // Named Pipes data sources go through a different parsing path + // (InferNamedPipesInformation) that doesn't populate InstanceName, + // so we test the lower-level overload directly. + ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs("server", "myinstance", DataSource.Protocol.NP); + + Assert.Contains(":myinstance", spn.Primary); + Assert.Null(spn.Secondary); + } + + /// + /// Verifies that when a custom ServerSPN is provided in the connection string, + /// it is used as-is regardless of protocol or instance name. + /// + [Fact] + public void GetSqlServerSPNs_CustomSpnProvided_UsesCustomSpn() + { + DataSource dataSource = DataSource.ParseServerName(@"server\instance"); + Assert.NotNull(dataSource); + dataSource.ResolvedPort = 12345; + + string customSpn = "MSSQLSvc/myserver.domain.com:1433"; + ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: customSpn); + + Assert.Equal(customSpn, spn.Primary); + Assert.Null(spn.Secondary); + } + + /// + /// Verifies that when connecting with admin: prefix (DAC), the SPN uses + /// the resolved port number (DAC also resolves via SSRP). + /// + [Fact] + public void GetSqlServerSPNs_ProtocolAdmin_WithResolvedPort_UsesPort() + { + DataSource dataSource = DataSource.ParseServerName(@"admin:server\instance"); + Assert.NotNull(dataSource); + Assert.Equal(DataSource.Protocol.Admin, dataSource.ResolvedProtocol); + + dataSource.ResolvedPort = 11111; + + ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty); + + Assert.Contains(":11111", spn.Primary); + Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase); + } + } +} + +#endif