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.

Sign up for GitHub

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

Jump to bottom

Fix | Take out the ignoreSniOpenTimeout in open connection #2067

Merged
merged 12 commits into from
Jul 20, 2023
Merged

Fix | Take out the ignoreSniOpenTimeout in open connection #2067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5c505a1
Add SqlConnectionEncryptOptionConverter class which is used to conver…
arellegue Jun 8, 2023
9b9af7e
Merge branch 'main' of https://github.com/arellegue/SqlClient
arellegue Jun 8, 2023
cde0430
Fixed Issue #1126 .NET Core SqlConnection ConnectTimout 15 less than …
arellegue Jun 13, 2023
1f41e2d
Merge branch 'dotnet:main' into main
arellegue Jun 16, 2023
deac068
Revert "Add SqlConnectionEncryptOptionConverter class which is used t…
arellegue Jun 16, 2023
bf78858
Revert "Add SqlConnectionEncryptOptionConverter class which is used t…
arellegue Jun 16, 2023
2028759
Revert "Fixed Issue #1126 .NET Core SqlConnection ConnectTimout 15 le…
arellegue Jun 16, 2023
ede78dc
Merge branch 'main' of https://github.com/arellegue/SqlClient
arellegue Jun 16, 2023
64ece1d
Removed ignoreSniOpenTimeout in Connect function chain of calls.
arellegue Jun 19, 2023
a9feac9
Merge branch 'main' into Issue_1126_NET_Core_SqlConnection_ConnectTim…
arellegue Jul 11, 2023
f4538d3
Put back missing } after resolving conflict so project will build.
arellegue Jul 12, 2023
e76d99f
Remove ignoreSniOpenTimeout from .NetFx too.
arellegue Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ private static bool IsErrorStatus(SecurityStatusPalErrorCode errorCode)
/// Create a SNI connection handle
/// </summary>
/// <param name="fullServerName">Full server name from connection string</param>
/// <param name="ignoreSniOpenTimeout">Ignore open timeout</param>
/// <param name="timerExpire">Timer expiration</param>
/// <param name="instanceName">Instance name</param>
/// <param name="spnBuffer">SPN</param>
Expand All @@ -148,7 +147,6 @@ private static bool IsErrorStatus(SecurityStatusPalErrorCode errorCode)
/// <returns>SNI handle</returns>
internal static SNIHandle CreateConnectionHandle(
string fullServerName,
bool ignoreSniOpenTimeout,
long timerExpire,
out byte[] instanceName,
ref byte[][] spnBuffer,
Expand Down
4 changes: 0 additions & 4 deletions ...Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1543,7 +1543,6 @@ private bool IsDoNotRetryConnectError(SqlException exc)
AttemptOneLogin(serverInfo,
newPassword,
newSecurePassword,
!connectionOptions.MultiSubnetFailover, // ignore timeout for SniOpen call unless MSF
connectionOptions.MultiSubnetFailover ? intervalTimer : timeout);

if (connectionOptions.MultiSubnetFailover && null != ServerProvidedFailOverPartner)
Expand Down Expand Up @@ -1782,7 +1781,6 @@ TimeoutTimer timeout
currentServerInfo,
newPassword,
newSecurePassword,
false, // Use timeout in SniOpen
intervalTimer,
withFailover: true
);
Expand Down Expand Up @@ -1910,7 +1908,6 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup,
ServerInfo serverInfo,
string newPassword,
SecureString newSecurePassword,
bool ignoreSniOpenTimeout,
TimeoutTimer timeout,
bool withFailover = false)
{
Expand All @@ -1921,7 +1918,6 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup,

_parser.Connect(serverInfo,
this,
ignoreSniOpenTimeout,
timeout.LegacyTimerExpire,
ConnectionOptions,
withFailover);
Expand Down
3 changes: 0 additions & 3 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)
internal void Connect(
ServerInfo serverInfo,
SqlInternalConnectionTds connHandler,
bool ignoreSniOpenTimeout,
long timerExpire,
SqlConnectionString connectionOptions,
bool withFailover)
Expand Down Expand Up @@ -445,7 +444,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)
// AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
_physicalStateObj.CreatePhysicalSNIHandle(
serverInfo.ExtendedServerName,
ignoreSniOpenTimeout,
timerExpire,
out instanceName,
ref _sniSpnBuffer,
Expand Down Expand Up @@ -544,7 +542,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)

_physicalStateObj.CreatePhysicalSNIHandle(
serverInfo.ExtendedServerName,
ignoreSniOpenTimeout,
timerExpire, out instanceName,
ref _sniSpnBuffer,
true,
Expand Down
1 change: 0 additions & 1 deletion src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ private void ResetCancelAndProcessAttention()

internal abstract void CreatePhysicalSNIHandle(
string serverName,
bool ignoreSniOpenTimeout,
long timerExpire,
out byte[] instanceName,
ref byte[][] spnBuffer,
Expand Down
3 changes: 1 addition & 2 deletions ...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ protected override uint SNIPacketGetData(PacketHandle packet, byte[] inBuff, ref

internal override void CreatePhysicalSNIHandle(
string serverName,
bool ignoreSniOpenTimeout,
long timerExpire,
out byte[] instanceName,
ref byte[][] spnBuffer,
Expand All @@ -98,7 +97,7 @@ protected override uint SNIPacketGetData(PacketHandle packet, byte[] inBuff, ref
string hostNameInCertificate,
string serverCertificateFilename)
{
SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, ignoreSniOpenTimeout, timerExpire, out instanceName, ref spnBuffer, serverSPN,
SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, timerExpire, out instanceName, ref spnBuffer, serverSPN,
flushCache, async, parallel, isIntegratedSecurity, iPAddressPreference, cachedFQDN, ref pendingDNSInfo, tlsFirst,
hostNameInCertificate, serverCertificateFilename);

Expand Down
3 changes: 1 addition & 2 deletions ...crosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async)

internal override void CreatePhysicalSNIHandle(
string serverName,
bool ignoreSniOpenTimeout,
long timerExpire,
out byte[] instanceName,
ref byte[][] spnBuffer,
Expand Down Expand Up @@ -199,7 +198,7 @@ private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async)
SQLDNSInfo cachedDNSInfo;
bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out cachedDNSInfo);

_sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer[0], ignoreSniOpenTimeout, checked((int)timeout), out instanceName,
_sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer[0], checked((int)timeout), out instanceName,
flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate);
}

Expand Down
6 changes: 1 addition & 5 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1861,7 +1861,6 @@ private bool IsDoNotRetryConnectError(SqlException exc)
AttemptOneLogin(serverInfo,
newPassword,
newSecurePassword,
!isParallel, // ignore timeout for SniOpen call unless MSF , and TNIR
attemptOneLoginTimeout,
isFirstTransparentAttempt: isFirstTransparentAttempt,
disableTnir: disableTnir);
Expand Down Expand Up @@ -2137,7 +2136,6 @@ TimeoutTimer timeout
currentServerInfo,
newPassword,
newSecurePassword,
false, // Use timeout in SniOpen
intervalTimer,
withFailover: true
);
Expand Down Expand Up @@ -2175,7 +2173,6 @@ TimeoutTimer timeout
currentServerInfo,
newPassword,
newSecurePassword,
false, // Use timeout in SniOpen
intervalTimer,
withFailover: true
);
Expand Down Expand Up @@ -2293,7 +2290,7 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup,
}

// Common code path for making one attempt to establish a connection and log in to server.
private void AttemptOneLogin(ServerInfo serverInfo, string newPassword, SecureString newSecurePassword, bool ignoreSniOpenTimeout, TimeoutTimer timeout, bool withFailover = false, bool isFirstTransparentAttempt = true, bool disableTnir = false)
private void AttemptOneLogin(ServerInfo serverInfo, string newPassword, SecureString newSecurePassword, TimeoutTimer timeout, bool withFailover = false, bool isFirstTransparentAttempt = true, bool disableTnir = false)
{
SqlClientEventSource.Log.TryAdvancedTraceEvent("<sc.SqlInternalConnectionTds.AttemptOneLogin|ADV> {0}, timout={1}[msec], server={2}", ObjectID, timeout.MillisecondsRemaining, serverInfo.ExtendedServerName);

Expand All @@ -2303,7 +2300,6 @@ private void AttemptOneLogin(ServerInfo serverInfo, string newPassword, SecureSt

_parser.Connect(serverInfo,
this,
ignoreSniOpenTimeout,
timeout.LegacyTimerExpire,
ConnectionOptions,
withFailover,
Expand Down
3 changes: 0 additions & 3 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)

internal void Connect(ServerInfo serverInfo,
SqlInternalConnectionTds connHandler,
bool ignoreSniOpenTimeout,
long timerExpire,
SqlConnectionString connectionOptions,
bool withFailover,
Expand Down Expand Up @@ -640,7 +639,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)

_physicalStateObj.CreatePhysicalSNIHandle(
serverInfo.ExtendedServerName,
ignoreSniOpenTimeout,
timerExpire,
out instanceName,
_sniSpnBuffer,
Expand Down Expand Up @@ -746,7 +744,6 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)
_physicalStateObj.SniContext = SniContext.Snix_Connect;
_physicalStateObj.CreatePhysicalSNIHandle(
serverInfo.ExtendedServerName,
ignoreSniOpenTimeout,
timerExpire,
out instanceName,
_sniSpnBuffer,
Expand Down
3 changes: 1 addition & 2 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async)

internal void CreatePhysicalSNIHandle(
string serverName,
bool ignoreSniOpenTimeout,
long timerExpire,
out byte[] instanceName,
byte[] spnBuffer,
Expand Down Expand Up @@ -318,7 +317,7 @@ private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async)

_ = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out SQLDNSInfo cachedDNSInfo);

_sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer, ignoreSniOpenTimeout, checked((int)timeout),
_sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer, checked((int)timeout),
out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout,
ipPreference, cachedDNSInfo, hostNameInCertificate);
}
Expand Down
16 changes: 8 additions & 8 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ internal sealed class SNIHandle : SafeHandle
SNINativeMethodWrapper.ConsumerInfo myInfo,
string serverName,
byte[] spnBuffer,
bool ignoreSniOpenTimeout,
int timeout,
out byte[] instanceName,
bool flushCache,
Expand All @@ -174,13 +173,14 @@ internal sealed class SNIHandle : SafeHandle
{
_fSync = fSync;
instanceName = new byte[256]; // Size as specified by netlibs.
if (ignoreSniOpenTimeout)
{
// UNDONE: ITEM12001110 (DB Mirroring Reconnect) Old behavior of not truly honoring timeout presevered
// for non-failover scenarios to avoid breaking changes as part of a QFE. Consider fixing timeout
// handling in next full release and removing ignoreSniOpenTimeout parameter.
timeout = Timeout.Infinite; // -1 == native SNIOPEN_TIMEOUT_VALUE / INFINITE
}
// Option ignoreSniOpenTimeout is no longer available
//if (ignoreSniOpenTimeout)
//{
// // UNDONE: ITEM12001110 (DB Mirroring Reconnect) Old behavior of not truly honoring timeout presevered
// // for non-failover scenarios to avoid breaking changes as part of a QFE. Consider fixing timeout
// // handling in next full release and removing ignoreSniOpenTimeout parameter.
// timeout = Timeout.Infinite; // -1 == native SNIOPEN_TIMEOUT_VALUE / INFINITE
//}

#if NETFRAMEWORK
int transparentNetworkResolutionStateNo = (int)transparentNetworkResolutionState;
Expand Down
93 changes: 93 additions & 0 deletions src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,99 @@ public void ConnectionTestValidCredentialCombination()
Assert.Equal(sqlCredential, conn.Credential);
}


[Theory]
[InlineData(60)]
[InlineData(30)]
[InlineData(15)]
[InlineData(10)]
[InlineData(5)]
[InlineData(1)]
public void ConnectionTimeoutTest(int timeout)
{
// Start a server with connection timeout from the inline data.
using TestTdsServer server = TestTdsServer.StartTestServer(false, false, timeout);
using SqlConnection connection = new SqlConnection(server.ConnectionString);

// Dispose the server to force connection timeout
server.Dispose();

// Measure the actual time it took to timeout and compare it with configured timeout
var start = DateTime.Now;
var end = start;

// Open a connection with the server disposed.
try
{
connection.Open();
}
catch (Exception)
{
end = DateTime.Now;
}

// Calculate actual duration of timeout
TimeSpan s = end - start;
// Did not time out?
if (s.TotalSeconds == 0)
Assert.True(s.TotalSeconds == 0);

// Is actual time out the same as configured timeout or within an additional 3 second threshold because of overhead?
if (s.TotalSeconds > 0)
Assert.True(s.TotalSeconds <= timeout + 3);
}

[Theory]
[InlineData(60)]
[InlineData(30)]
[InlineData(15)]
[InlineData(10)]
[InlineData(5)]
[InlineData(1)]
public async void ConnectionTimeoutTestAsync(int timeout)
{
// Start a server with connection timeout from the inline data.
using TestTdsServer server = TestTdsServer.StartTestServer(false, false, timeout);
using SqlConnection connection = new SqlConnection(server.ConnectionString);

// Dispose the server to force connection timeout
server.Dispose();

// Measure the actual time it took to timeout and compare it with configured timeout
var start = DateTime.Now;
var end = start;

// Open a connection with the server disposed.
try
{
await connection.OpenAsync();
}
catch (Exception)
{
end = DateTime.Now;
}

// Calculate actual duration of timeout
TimeSpan s = end - start;
// Did not time out?
if (s.TotalSeconds == 0)
Assert.True(s.TotalSeconds == 0);

// Is actual time out the same as configured timeout or within an additional 3 second threshold because of overhead?
if (s.TotalSeconds > 0)
Assert.True(s.TotalSeconds <= timeout + 3);
}

[Fact]
public void ConnectionInvalidTimeoutTest()
{
Assert.Throws<ArgumentException>(() =>
{
using TestTdsServer server = TestTdsServer.StartTestServer(false, false, -5);
});

}

[Fact]
public void ConnectionTestWithCultureTH()
{
Expand Down