Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix | Fixing issue for SQL named Instances in Managed SNI using named pipes. #2142

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
56a19de
Implement support for NamePiped Instance in Managed SNI.
arellegue Aug 31, 2023
730e0c5
Removed path sepator prefix for default pipe name.
arellegue Aug 31, 2023
731048a
Added ReadOnlySpan to access string array.
arellegue Sep 1, 2023
4dbd383
Enhance InstanceNameTest to get NormalizedPipePath using either m_nor…
arellegue Sep 25, 2023
c948a59
Removed nuget package source from Nuget.config.
arellegue Sep 25, 2023
370aa8c
Removed SkipOnTargetFramework from NamedPipeInstanceNormalizedPipePa…
arellegue Sep 27, 2023
6d3e7c4
Added TCPInstanceConnectionString and NPInstanceConnectionString to c…
arellegue Sep 27, 2023
db07788
Include newly added config properties to SqlDbManager class.
arellegue Oct 4, 2023
b3660e4
Add condition to create Northwind database in named instance sql server.
arellegue Oct 4, 2023
6e7e24b
Use more meaningful name in third command line argument.
arellegue Oct 5, 2023
21d8eaf
Remove third command line argument. Control the Northwind creation in…
arellegue Oct 5, 2023
fc4f20e
Add ConditionalFact to test if NP Instance connection string is setup…
arellegue Oct 12, 2023
7a5297d
Revert all changes for newly added NamedInstance unit test. There is …
arellegue Oct 13, 2023
0d5a1e2
Revert config.default.json.
arellegue Oct 13, 2023
56903a9
Added 500ms delay after creating a user in SqlCredentialTest.
arellegue Oct 20, 2023
3bee9cf
Fixed Login exception error in OldCredentialsShouldFail unit test.
arellegue Oct 21, 2023
ee1d035
Add 5 seconds delay in createTestUser.
arellegue Oct 23, 2023
3724a46
Revert OldCredentialsShouldFail.
arellegue Oct 23, 2023
102e5b2
Removed unecessary comment.
arellegue Oct 23, 2023
b3a91cc
Removed blank line.
arellegue Oct 23, 2023
81f5f73
Removed blank line in SqlCredentialTest.
arellegue Oct 24, 2023
b5a581f
Removed using System.Data from InstanceNameTest class.
arellegue Oct 25, 2023
a96a5b9
Delete commented out InferLocalServerName();
arellegue Oct 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public SNINpHandle(string serverName, string pipeName, TimeoutTimer timeout, boo
{
int timeoutMilliseconds = timeout.MillisecondsRemainingInt;
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNINpHandle), EventType.INFO,
"Connection Id {0}, Setting server name = {1}, pipe name = {2}. Connecting within the {3} sepecified milliseconds.",
"Connection Id {0}, Setting server name = {1}, pipe name = {2}. Connecting within the {3} specified milliseconds.",
args0: _connectionId,
args1: serverName,
args2: pipeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ internal class DataSource
private const string LocalDbHost_NP = @"np:\\.\pipe\LOCALDB#";
private const string NamedPipeInstanceNameHeader = "mssql$";
private const string DefaultPipeName = "sql\\query";
private const string InstancePrefix = "MSSQL$";
private const string PathSeparator = "\\";

internal enum Protocol { TCP, NP, None, Admin };

Expand Down Expand Up @@ -676,12 +678,36 @@ private bool InferNamedPipesInformation()
// If we have a datasource beginning with a pipe or we have already determined that the protocol is Named Pipe
if (_dataSourceAfterTrimmingProtocol.StartsWith(PipeBeginning, StringComparison.Ordinal) || _connectionProtocol == Protocol.NP)
{
// If the data source is "np:servername"
// If the data source is "np:servername" or "np:servername\instance"
arellegue marked this conversation as resolved.
Show resolved Hide resolved
if (!_dataSourceAfterTrimmingProtocol.Contains(PipeBeginning))
{
PipeHostName = ServerName = _dataSourceAfterTrimmingProtocol;
InferLocalServerName();
PipeName = SNINpHandle.DefaultPipePath;
// 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) && _connectionProtocol == Protocol.NP)
{
string[] tokensByBackSlash = _dataSourceAfterTrimmingProtocol.Split(BackSlashCharacter);
if (tokensByBackSlash.Length == 2)
{
// NamedPipeClientStream object will create the network path using PipeHostName and PipeName
// and can be seen in its _normalizedPipePath variable in the format \\servername\pipe\MSSQL$<instancename>\sql\query
ReadOnlySpan<string> arr = tokensByBackSlash;
PipeHostName = ServerName = arr[0];
InferLocalServerName();
PipeName = $"{InstancePrefix}{arr[1].ToUpper()}{PathSeparator}{DefaultPipeName}";
}
else
{
ReportSNIError(SNIProviders.NP_PROV);
return false;
}
}
else
{
PipeHostName = ServerName = _dataSourceAfterTrimmingProtocol;
InferLocalServerName();
PipeName = SNINpHandle.DefaultPipePath;
}
arellegue marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ internal static object LocalMachineRegistryValue(string subkey, string queryvalu
#if !NET6_0_OR_GREATER
(new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028
#endif
// Non-Windows OS do not have registry
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return null;
}

arellegue marked this conversation as resolved.
Show resolved Hide resolved
try
{
using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
// See the LICENSE file in the project root for more information.

using System;
using System.IO.Pipes;
using System.Net;
using System.Net.Sockets;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Xunit;
Expand All @@ -13,6 +15,7 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class InstanceNameTest
{

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))]
public static void ConnectToSQLWithInstanceNameTest()
{
Expand Down Expand Up @@ -76,13 +79,62 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove
if (!IsValidInstance(hostname, instanceName))
{
builder.DataSource = hostname + "\\" + instanceName;

using SqlConnection connection = new(builder.ConnectionString);
SqlException ex = Assert.Throws<SqlException>(() => connection.Open());
Assert.Contains("Error Locating Server/Instance Specified", ex.Message);
}
}

[PlatformSpecific(TestPlatforms.Windows)]
[SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp)]
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
[InlineData("")]
[InlineData("MSSQLSERVER02")]
public static void NamedPipeInstanceNormalizedPipePathTest(string instance)
David-Engel marked this conversation as resolved.
Show resolved Hide resolved
{
var instancePrefix = "MSSQL$";
var pathSeparator = "\\";
var defaultPipeName = "sql\\sqlquery";

SqlConnectionStringBuilder builder = new(DataTestUtility.NPConnectionString);

Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName));
instanceName = instance;

// Mimic the SNIProxy.InferNamedPipesInformation's logic to initialize the PipeName. It is private so can not be used here.
string pipeName = $"{defaultPipeName}";
if (instanceName != string.Empty)
{
// This is how InferNamedPipesInformation build the pipeName when there's an instance provided.
pipeName = $"{instancePrefix}{instanceName.ToUpper()}{pathSeparator}{defaultPipeName}";
}

NamedPipeClientStream pipeStream = new NamedPipeClientStream(
hostname,
pipeName,
PipeDirection.InOut,
PipeOptions.Asynchronous | PipeOptions.WriteThrough);

string normalizedPipePath = pipeStream
arellegue marked this conversation as resolved.
Show resolved Hide resolved
.GetType()
.GetField("m_normalizedPipePath", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(pipeStream).ToString();

// Check if the normalized pipe path parsed by NamedPipeClientStream object from supplied
// host and pipename has a valid format
if (instanceName != string.Empty)
{
// Secondary NamedPipe Instance normalized pipe path format check
Assert.Matches(@"\\\\.*\\pipe\\MSSQL\$.*\\sql\\sqlquery", normalizedPipePath);
}
else
{
// Default NamedPipe Instance normalized pipe path format check
Assert.Matches(@"\\\\.*\\pipe\\sql\\sqlquery", normalizedPipePath);
}
}

private static bool IsBrowserAlive(string browserHostname)
{
const byte ClntUcastEx = 0x03;
Expand Down