From e99b5747971e4909b796cfe55d86fda97d845410 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Tue, 8 Jun 2021 15:07:33 -0700 Subject: [PATCH 1/8] Update SqlCommand.cs --- .../netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs index 24a82db701..3da0c9cab5 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -3854,7 +3854,7 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo(int timeout, "The number of decribe parameter encryption RPC requests is more than the number of original RPC requests."); } //Always Encrypted generally operates only on parameterized queries. However enclave based Always encrypted also supports unparameterized queries - else if (ShouldUseEnclaveBasedWorkflow || (0 != GetParameterCount(_parameters))) + else if (ShouldUseEnclaveBasedWorkflow && (0 != GetParameterCount(_parameters))) { // Fetch params for a single batch inputParameterEncryptionNeeded = true; From c8fb592a690c59ebf4aecb09782871bf9f761cd0 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 11 Jun 2021 13:02:11 -0700 Subject: [PATCH 2/8] only execute sp_describe_parameter_encryption if parameters not empty --- doc/samples/AzureKeyVaultProviderExample_2_0.cs | 4 +--- .../netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/samples/AzureKeyVaultProviderExample_2_0.cs b/doc/samples/AzureKeyVaultProviderExample_2_0.cs index 95733310a8..f241966458 100644 --- a/doc/samples/AzureKeyVaultProviderExample_2_0.cs +++ b/doc/samples/AzureKeyVaultProviderExample_2_0.cs @@ -242,7 +242,5 @@ public CustomerRecord(int id, string fName, string lName) } } } - - // - } +// diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs index 3da0c9cab5..96834fe60f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -3854,7 +3854,7 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo(int timeout, "The number of decribe parameter encryption RPC requests is more than the number of original RPC requests."); } //Always Encrypted generally operates only on parameterized queries. However enclave based Always encrypted also supports unparameterized queries - else if (ShouldUseEnclaveBasedWorkflow && (0 != GetParameterCount(_parameters))) + else if ((ShouldUseEnclaveBasedWorkflow && 0 != GetParameterCount(_parameters)) || (0 != GetParameterCount(_parameters))) { // Fetch params for a single batch inputParameterEncryptionNeeded = true; From d61e3b8a318cad3a3f3defc074402b76a8fe28f5 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 11 Jun 2021 14:42:38 -0700 Subject: [PATCH 3/8] add test --- .../ManualTests/AlwaysEncrypted/ApiShould.cs | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index b92a642ae8..6e16810661 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -878,6 +878,39 @@ public void TestExecuteReaderWithCommandBehavior(string connection, CommandBehav }); } + + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] + [ClassData(typeof(AEConnectionStringProvider))] + public void TestEnclaveStoredProcedureWithoutParameters(string connectionString) + { + using (SqlConnection sqlConnection = new(connectionString)) + { + sqlConnection.Open(); + + using (SqlCommand sqlCommand = new("", sqlConnection, transaction: null, + columnEncryptionSetting: SqlCommandColumnEncryptionSetting.Enabled)) + { + string storedProcedureName = DataTestUtility.GetUniqueName("EnclaveWithoutParams"); + + try + { + sqlCommand.CommandText = $"CREATE PROCEDURE {storedProcedureName} AS SELECT * FROM [{_tableName}]"; + sqlCommand.ExecuteNonQuery(); + + sqlCommand.CommandText = storedProcedureName; + sqlCommand.CommandType = CommandType.StoredProcedure; + SqlDataReader reader = sqlCommand.ExecuteReader(); + + Assert.Equal(3, reader.VisibleFieldCount); + } + finally + { + DropHelperProcedures(new[] { storedProcedureName }, connectionString); + } + } + } + } + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] [ClassData(typeof(AEConnectionStringProvider))] public void TestPrepareWithExecuteNonQuery(string connection) @@ -2262,7 +2295,8 @@ public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string c connection.Open(); using SqlCommand command = CreateCommandThatRequiresSystemKeyStoreProvider(connection); connection.RegisterColumnEncryptionKeyStoreProvidersOnConnection(customKeyStoreProviders); - command.ExecuteReader(); + SqlDataReader reader = command.ExecuteReader(); + Assert.Equal(3, reader.VisibleFieldCount); } using (SqlConnection connection = new(connectionString)) @@ -2270,7 +2304,8 @@ public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string c connection.Open(); using SqlCommand command = CreateCommandThatRequiresSystemKeyStoreProvider(connection); command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(customKeyStoreProviders); - command.ExecuteReader(); + SqlDataReader reader = command.ExecuteReader(); + Assert.Equal(3, reader.VisibleFieldCount); } } From 1234484238db9691ce1c36917374ab25cb119404 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 11 Jun 2021 15:51:38 -0700 Subject: [PATCH 4/8] improve test coverage --- .../ManualTests/AlwaysEncrypted/ApiShould.cs | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index 6e16810661..5cd0536ab0 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -881,34 +881,48 @@ public void TestExecuteReaderWithCommandBehavior(string connection, CommandBehav [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] [ClassData(typeof(AEConnectionStringProvider))] - public void TestEnclaveStoredProcedureWithoutParameters(string connectionString) + public void TestEnclaveStoredProcedure(string connectionString) { - using (SqlConnection sqlConnection = new(connectionString)) + using SqlConnection sqlConnection = new(connectionString); + sqlConnection.Open(); + + using SqlCommand sqlCommand = new("", sqlConnection, transaction: null, + columnEncryptionSetting: SqlCommandColumnEncryptionSetting.Enabled); + + string procWithoutParams = DataTestUtility.GetUniqueName("EnclaveWithoutParams", withBracket: false); + string procWithParam = DataTestUtility.GetUniqueName("EnclaveWithParams", withBracket: false); + + try { - sqlConnection.Open(); + sqlCommand.CommandText = $"CREATE PROCEDURE {procWithoutParams} AS SELECT * FROM [{_tableName}];"; + sqlCommand.ExecuteNonQuery(); + sqlCommand.CommandText = $"CREATE PROCEDURE {procWithParam} @id INT AS SELECT * FROM [{_tableName}] WHERE CustomerId = @id"; + sqlCommand.ExecuteNonQuery(); - using (SqlCommand sqlCommand = new("", sqlConnection, transaction: null, - columnEncryptionSetting: SqlCommandColumnEncryptionSetting.Enabled)) + sqlCommand.CommandText = procWithoutParams; + sqlCommand.CommandType = CommandType.StoredProcedure; + using (SqlDataReader reader = sqlCommand.ExecuteReader()) { - string storedProcedureName = DataTestUtility.GetUniqueName("EnclaveWithoutParams"); + Assert.Equal(3, reader.VisibleFieldCount); + } - try - { - sqlCommand.CommandText = $"CREATE PROCEDURE {storedProcedureName} AS SELECT * FROM [{_tableName}]"; - sqlCommand.ExecuteNonQuery(); + sqlCommand.CommandText = procWithParam; + sqlCommand.CommandType = CommandType.StoredProcedure; + Exception ex = Assert.Throws(() => sqlCommand.ExecuteReader()); + string expectedMsg = $"Procedure or function '{procWithParam}' expects parameter '@id', which was not supplied."; - sqlCommand.CommandText = storedProcedureName; - sqlCommand.CommandType = CommandType.StoredProcedure; - SqlDataReader reader = sqlCommand.ExecuteReader(); + Assert.Equal(expectedMsg, ex.Message); - Assert.Equal(3, reader.VisibleFieldCount); - } - finally - { - DropHelperProcedures(new[] { storedProcedureName }, connectionString); - } + sqlCommand.Parameters.AddWithValue("@id", 0); + using (SqlDataReader reader = sqlCommand.ExecuteReader()) + { + Assert.Equal(3, reader.VisibleFieldCount); } } + finally + { + DropHelperProcedures(new[] { procWithoutParams, procWithParam }, connectionString); + } } [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] From 1c7b8aa04f1eef0a017f0f56b8a6574e54c1d09e Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Mon, 14 Jun 2021 17:06:04 -0700 Subject: [PATCH 5/8] only go through parameters if not null --- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 13 ++++++++++++- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 13 ++++++++++++- .../tests/ManualTests/AlwaysEncrypted/ApiShould.cs | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs index 96834fe60f..fc97b789d5 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -5829,11 +5829,22 @@ private SqlParameter BuildStoredProcedureStatementForColumnEncryption(string sto { Debug.Assert(CommandType == CommandType.StoredProcedure, "BuildStoredProcedureStatementForColumnEncryption() should only be called for stored procedures"); Debug.Assert(!string.IsNullOrWhiteSpace(storedProcedureName), "storedProcedureName cannot be null or empty in BuildStoredProcedureStatementForColumnEncryption"); - Debug.Assert(parameters != null, "parameters cannot be null in BuildStoredProcedureStatementForColumnEncryption"); StringBuilder execStatement = new StringBuilder(); execStatement.Append(@"EXEC "); + if (parameters is null) + { + execStatement.Append(ParseAndQuoteIdentifier(storedProcedureName, false)); + return new SqlParameter( + null, + ((execStatement.Length << 1) <= TdsEnums.TYPE_SIZE_LIMIT) ? SqlDbType.NVarChar : SqlDbType.NText, + execStatement.Length) + { + Value = execStatement.ToString() + }; + } + // Find the return value parameter (if any). SqlParameter returnValueParameter = null; foreach (SqlParameter parameter in parameters) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs index b0f8dc3d50..3f9e28ed89 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -6781,11 +6781,22 @@ private SqlParameter BuildStoredProcedureStatementForColumnEncryption(string sto { Debug.Assert(CommandType == CommandType.StoredProcedure, "BuildStoredProcedureStatementForColumnEncryption() should only be called for stored procedures"); Debug.Assert(!string.IsNullOrWhiteSpace(storedProcedureName), "storedProcedureName cannot be null or empty in BuildStoredProcedureStatementForColumnEncryption"); - Debug.Assert(parameters != null, "parameters cannot be null in BuildStoredProcedureStatementForColumnEncryption"); StringBuilder execStatement = new StringBuilder(); execStatement.Append(@"EXEC "); + if (parameters is null) + { + execStatement.Append(ParseAndQuoteIdentifier(storedProcedureName, false)); + return new SqlParameter( + null, + ((execStatement.Length << 1) <= TdsEnums.TYPE_SIZE_LIMIT) ? SqlDbType.NVarChar : SqlDbType.NText, + execStatement.Length) + { + Value = execStatement.ToString() + }; + } + // Find the return value parameter (if any). SqlParameter returnValueParameter = null; foreach (SqlParameter parameter in parameters) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index 5cd0536ab0..4488f26b6c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -881,7 +881,7 @@ public void TestExecuteReaderWithCommandBehavior(string connection, CommandBehav [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] [ClassData(typeof(AEConnectionStringProvider))] - public void TestEnclaveStoredProcedure(string connectionString) + public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectionString) { using SqlConnection sqlConnection = new(connectionString); sqlConnection.Open(); From 208124f0547d3bdba5d114f7f899d2beb86f0593 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Mon, 14 Jun 2021 17:10:58 -0700 Subject: [PATCH 6/8] revert previous attempt --- .../netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs index fc97b789d5..69fabd3387 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -3854,7 +3854,7 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo(int timeout, "The number of decribe parameter encryption RPC requests is more than the number of original RPC requests."); } //Always Encrypted generally operates only on parameterized queries. However enclave based Always encrypted also supports unparameterized queries - else if ((ShouldUseEnclaveBasedWorkflow && 0 != GetParameterCount(_parameters)) || (0 != GetParameterCount(_parameters))) + else if (ShouldUseEnclaveBasedWorkflow || (0 != GetParameterCount(_parameters))) { // Fetch params for a single batch inputParameterEncryptionNeeded = true; From 6d19e812295a94a3f5bf64531025100c21f3e692 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Thu, 8 Jul 2021 12:20:41 -0700 Subject: [PATCH 7/8] Update ApiShould.cs --- .../tests/ManualTests/AlwaysEncrypted/ApiShould.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index 4488f26b6c..acebfdf84b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -878,7 +878,6 @@ public void TestExecuteReaderWithCommandBehavior(string connection, CommandBehav }); } - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] [ClassData(typeof(AEConnectionStringProvider))] public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectionString) @@ -894,16 +893,16 @@ public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectio try { - sqlCommand.CommandText = $"CREATE PROCEDURE {procWithoutParams} AS SELECT * FROM [{_tableName}];"; + sqlCommand.CommandText = $"CREATE PROCEDURE {procWithoutParams} AS SELECT FirstName, LastName FROM [{_tableName}];"; sqlCommand.ExecuteNonQuery(); - sqlCommand.CommandText = $"CREATE PROCEDURE {procWithParam} @id INT AS SELECT * FROM [{_tableName}] WHERE CustomerId = @id"; + sqlCommand.CommandText = $"CREATE PROCEDURE {procWithParam} @id INT AS SELECT FirstName, LastName FROM [{_tableName}] WHERE CustomerId = @id"; sqlCommand.ExecuteNonQuery(); sqlCommand.CommandText = procWithoutParams; sqlCommand.CommandType = CommandType.StoredProcedure; using (SqlDataReader reader = sqlCommand.ExecuteReader()) { - Assert.Equal(3, reader.VisibleFieldCount); + Assert.Equal(2, reader.VisibleFieldCount); } sqlCommand.CommandText = procWithParam; @@ -916,7 +915,7 @@ public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectio sqlCommand.Parameters.AddWithValue("@id", 0); using (SqlDataReader reader = sqlCommand.ExecuteReader()) { - Assert.Equal(3, reader.VisibleFieldCount); + Assert.Equal(2, reader.VisibleFieldCount); } } finally From 099aedbfa46d2afb2fbc29a1679e31109034b0ac Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Thu, 8 Jul 2021 12:21:40 -0700 Subject: [PATCH 8/8] Update ApiShould.cs --- .../tests/ManualTests/AlwaysEncrypted/ApiShould.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index acebfdf84b..d27b985864 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -897,12 +897,13 @@ public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectio sqlCommand.ExecuteNonQuery(); sqlCommand.CommandText = $"CREATE PROCEDURE {procWithParam} @id INT AS SELECT FirstName, LastName FROM [{_tableName}] WHERE CustomerId = @id"; sqlCommand.ExecuteNonQuery(); + int expectedFields = 2; sqlCommand.CommandText = procWithoutParams; sqlCommand.CommandType = CommandType.StoredProcedure; using (SqlDataReader reader = sqlCommand.ExecuteReader()) { - Assert.Equal(2, reader.VisibleFieldCount); + Assert.Equal(expectedFields, reader.VisibleFieldCount); } sqlCommand.CommandText = procWithParam; @@ -915,7 +916,7 @@ public void TestEnclaveStoredProceduresWithAndWithoutParameters(string connectio sqlCommand.Parameters.AddWithValue("@id", 0); using (SqlDataReader reader = sqlCommand.ExecuteReader()) { - Assert.Equal(2, reader.VisibleFieldCount); + Assert.Equal(expectedFields, reader.VisibleFieldCount); } } finally