From 00d45e87ab45b5d7fab334baf65631a64ae20aaf Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 17 May 2021 16:59:40 -0700 Subject: [PATCH] Fixes corrupted connection issue when an exception occurs during RPC execution with TVP types (#1068) --- build.proj | 2 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 6 -- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 7 -- .../ManualTests/SQL/ParameterTest/TvpTest.cs | 96 ++++++++++++++++++- 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/build.proj b/build.proj index 9d59894bf0..763e975df1 100644 --- a/build.proj +++ b/build.proj @@ -38,7 +38,7 @@ - + diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index b32a6e3174..5c526e9c30 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -9110,13 +9110,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo } catch (Exception e) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - FailureCleanup(stateObj, e); - throw; } FinalizeExecuteRPC(stateObj); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 0d3c877b7f..328867ddca 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -10449,14 +10449,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo } catch (Exception e) { - // UNDONE - should not be catching all exceptions!!! - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - FailureCleanup(stateObj, e); - throw; } FinalizeExecuteRPC(stateObj); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs index 7c4e81cfd5..084fe11789 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs @@ -17,6 +17,7 @@ using System.Transactions; using Microsoft.Data.SqlClient.Server; using Xunit; +using System.Linq; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { @@ -82,6 +83,99 @@ public void TestPacketNumberWraparound() Assert.True(enumerator.MaxCount == enumerator.Count); } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + public void TestConnectionIsSafeToReuse() + { + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + + // Bad Scenario - exception expected. + try + { + List list = new() + { + new Item(0), + null, + new Item(2), + new Item(3), + new Item(4), + new Item(5) + }; + + IEnumerable Ids = list.Select(x => x.id.Value).Distinct(); + + var sqlParam = new SqlParameter("ids", SqlDbType.Structured) + { + TypeName = "dbo.TableOfIntId", + SqlValue = Ids.Select(x => + { + SqlDataRecord rec = new(new[] { new SqlMetaData("Id", SqlDbType.Int) }); + rec.SetInt32(0, x); + return rec; + }) + }; + + var parameters = new List() { sqlParam }; + const string SQL = @"SELECT * FROM information_schema.COLUMNS cols INNER JOIN @ids Ids on Ids.id = cols.ORDINAL_POSITION"; + using SqlCommand cmd = new(SQL, connection); + cmd.CommandTimeout = 100; + AddCommandParameters(cmd, parameters); + new SqlDataAdapter(cmd).Fill(new("BadFunc")); + Assert.False(true, "Expected exception did not occur"); + } + catch (Exception e) + { + // Ignore this exception as it's deliberately introduced. + Assert.True(e.Message.Contains("Object reference not set to an instance of an object"), "Expected exception did not occur"); + } + + // Good Scenario - No failure expected. + try + { + const string SQL = @"SELECT * FROM information_schema.tables WHERE TABLE_NAME = @TableName"; + var parameters = new List() { new SqlParameter("@TableName", "Temp") }; + using SqlCommand cmd = new(SQL, connection); + cmd.CommandTimeout = 100; + AddCommandParameters(cmd, parameters); + new SqlDataAdapter(cmd).Fill(new("GoodFunc")); + } + catch (Exception e) + { + Assert.False(true, $"Unexpected error occurred: {e.Message}"); + } + } + + private class Item + { + public Item(int? v) + { + id = v; + } + public int? id { get; set; } + } + + static internal void AddCommandParameters(SqlCommand command, IEnumerable parameters) + { + if (parameters == null) + return; + + foreach (SqlParameter p in parameters) + { + if (p == null) + continue; + + if (p.Value == null) + { + var clone = (SqlParameter)((ICloneable)p).Clone(); + clone.Value = DBNull.Value; + command.Parameters.Add(clone); + } + else + { + command.Parameters.Add(p); + } + } + } + public TvpTest() { _connStr = DataTestUtility.TCPConnectionString; @@ -693,7 +787,7 @@ private bool AllowableDifference(byte[] source, object result, StePermutation me // allowable max-length adjustments if (maxLength == resultBytes.Length) { // a bit optimistic, but what the heck. - // truncation + // truncation if (maxLength <= source.Length) { returnValue = true;