Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 24 additions & 16 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;
using System;
using System.Threading;
using System.Collections;
using System.IO;

namespace System.Data.SqlClient.SNI
{
Expand Down Expand Up @@ -575,7 +572,6 @@ internal class DataSource

private const char CommaSeparator = ',';
private const char BackSlashSeparator = '\\';
private const char ForwardSlashSeparator = '/';
private const string DefaultHostName = "localhost";
private const string DefaultSqlServerInstanceName = "mssqlserver";
private const string PipeBeginning = @"\\";
Expand Down Expand Up @@ -786,32 +782,44 @@ private bool InferNamedPipesInformation()

try
{
Uri uri = new Uri(_dataSourceAfterTrimmingProtocol);
if (string.IsNullOrEmpty(uri.Host))
string[] tokensByBackSlash = _dataSourceAfterTrimmingProtocol.Split(BackSlashSeparator);

// The datasource is of the format \\host\pipe\sql\query [0]\[1]\[2]\[3]\[4]\[5]
// It would at least have 6 parts.
// Another valid Sql named pipe for an named instance is \\.\pipe\MSSQL$MYINSTANCE\sql\query
if (tokensByBackSlash.Length < 6)
{
ReportSNIError(SNIProviders.NP_PROV);
return false;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for leading backslashes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard this, there's a check for it higher up.

string[] absolutePathParts = uri.AbsolutePath.Split(ForwardSlashSeparator);
string host = tokensByBackSlash[2];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do string host = tokensByBackSlash[2].Trim();?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Np:\.\pipe\sql\query would lead to a successful connection but np:\. \pipe\sql\query doesn't lead to a successful connection in Native SNI.
We shouldnt trim the pipe parts.


//Check if the "pipe" keyword is the first part of path
if (PipeToken.CompareTo(absolutePathParts[1]) != 0)
if (string.IsNullOrEmpty(host))
{
ReportSNIError(SNIProviders.NP_PROV);
return false;
}

// There should be at least 4 parts in the pipename e.g /pipe/sql/query [0]/[1]/[2]/[3]
// Another valid Sql named pipe for an named instance is \\.\pipe\MSSQL$MYINSTANCE\sql\query
if (absolutePathParts.Length < 4)
//Check if the "pipe" keyword is the first part of path
if (!PipeToken.Equals(tokensByBackSlash[3]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also if (!PipeToken.Equals(tokensByBackSlash[3].Trim()))?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No trimming of pipe paths.

{
ReportSNIError(SNIProviders.NP_PROV);
return false;
}

PipeName = uri.AbsolutePath.Substring(PipeToken.Length + 2);
ServerName = IsLocalHost(uri.Host) ? Environment.MachineName : uri.Host;
StringBuilder pipeNameBuilder = new StringBuilder();

for ( int i = 4; i < tokensByBackSlash.Length-1; i++)
{
pipeNameBuilder.Append(tokensByBackSlash[i]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do pipeNameBuilder.Append(tokensByBackSlash[i].Trim()); ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

pipeNameBuilder.Append(Path.PathSeparator);
}
// Append the last part without a "/"
pipeNameBuilder.Append(tokensByBackSlash[tokensByBackSlash.Length - 1]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if connection string is \\.\pipe\sql\query\ ?

Copy link
Copy Markdown
Contributor Author

@saurabh500 saurabh500 Mar 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlClient should fail to connect in this case.
With the change above a blank will be appended after the last \ and there will be a failure to connect


PipeName = pipeNameBuilder.ToString();
ServerName = IsLocalHost(host) ? Environment.MachineName : host;
}
catch (UriFormatException)
{
Expand All @@ -820,7 +828,7 @@ private bool InferNamedPipesInformation()
}

// DataSource is something like "\\pipename"
if (ConnectionProtocol != DataSource.Protocol.None)
if (ConnectionProtocol == DataSource.Protocol.None)
{
ConnectionProtocol = DataSource.Protocol.NP;
}
Expand Down
21 changes: 21 additions & 0 deletions src/System.Data.SqlClient/tests/FunctionalTests/ExceptionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,27 @@ public void IndependentConnectionExceptionTestExecuteReader()
}
}

[Theory]
[InlineData(@"np:\\.\pipe\sqlbad\query")]
[InlineData(@"np:\\.\pipe\MSSQL$NonExistentInstance\sql\query")]
[InlineData(@"\\.\pipe\sqlbad\query")]
[InlineData(@"\\.\pipe\MSSQL$NonExistentInstance\sql\query")]
[InlineData(@"np:\\localhost\pipe\sqlbad\query")]
[InlineData(@"np:\\localhost\pipe\MSSQL$NonExistentInstance\sqlbad\query")]
[InlineData(@"\\localhost\pipe\sqlbad\query")]
[InlineData(@"\\localhost\pipe\MSSQL$NonExistentInstance\sqlbad\query")]
[PlatformSpecific(TestPlatforms.Windows)] // Named pipes with the given input strings are not supported on Unix
public void NamedPipeTest(string dataSource)
{
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
builder.DataSource = dataSource;
builder.ConnectTimeout = 1;
using(SqlConnection connection = new SqlConnection(builder.ConnectionString))
{
VerifyConnectionFailure<SqlException>(() => connection.Open(), "(provider: Named Pipes Provider, error: 11 - Timeout error)");
}
}

private void GenerateConnectionException(string connectionString)
{
using (SqlConnection sqlConnection = new SqlConnection(connectionString))
Expand Down