Skip to content

Commit c23fed6

Browse files
priyankatiwari08Copilot
priyankatiwari08
and
Copilot
authored
Fixing NullReferenceException issue with SqlDataAdapter (#3749)
* Fixing NullReferenceException issue with SqlDataAdapter * Added tests for this scenario * Update src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj Co-authored-by: Copilot <[email protected]> * addressed PR comments related fixes. * added this test in AE TestSetup * Update src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs Co-authored-by: Copilot <[email protected]> * Update src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs Co-authored-by: Copilot <[email protected]> * Commits to ensure table name is properly escaped to prevent potential SQL injection. * . * . --------- Co-authored-by: Copilot <[email protected]>
1 parent 9bce6ad commit c23fed6

File tree

3 files changed

+259
-1
lines changed

3 files changed

+259
-1
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1293,7 +1293,7 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo(
12931293
// not present as the rpcName, as is the case with non-_batchRPCMode. So
12941294
// input parameters start at parameters[1]. parameters[0] is the actual
12951295
// T-SQL Statement. rpcName is sp_executesql.
1296-
if (_RPCList[i].systemParams.Length > 1)
1296+
if (_RPCList[i].systemParams != null && _RPCList[i].systemParams.Length > 1)
12971297
{
12981298
_RPCList[i].needsFetchParameterEncryptionMetadata = true;
12991299

‎src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/SqlDataAdapterBatchUpdateTests.cs‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Data;
7+
using System.Threading.Tasks;
8+
using System.Collections.Generic;
9+
using Microsoft.Data.SqlClient;
10+
using Microsoft.Data.SqlClient.ManualTesting.Tests.AlwaysEncrypted.Setup;
11+
using Xunit;
12+
13+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests.AlwaysEncrypted
14+
{
15+
public sealed class SqlDataAdapterBatchUpdateTests : IClassFixture<SQLSetupStrategyCertStoreProvider>, IDisposable
16+
{
17+
private readonly SQLSetupStrategy _fixture;
18+
private readonly Dictionary<string, string> tableNames = new();
19+
20+
public SqlDataAdapterBatchUpdateTests(SQLSetupStrategyCertStoreProvider context)
21+
{
22+
_fixture = context;
23+
24+
// Provide table names to mirror repo patterns.
25+
// If your fixture already exposes specific names for BuyerSeller and procs, wire them here.
26+
// Otherwise use literal names as below.
27+
tableNames["BuyerSeller"] = "BuyerSeller";
28+
tableNames["ProcInsertBuyerSeller"] = "InsertBuyerSeller";
29+
tableNames["ProcUpdateBuyerSeller"] = "UpdateBuyerSeller";
30+
}
31+
32+
// ---------- TESTS ----------
33+
34+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsTargetReadyForAeWithKeyStore))]
35+
[ClassData(typeof(AEConnectionStringProvider))]
36+
public async Task AdapterUpdate_BatchSizeGreaterThanOne_Succeeds(string connectionString)
37+
{
38+
// Arrange
39+
// Ensure baseline rows exist
40+
TruncateTables("BuyerSeller", connectionString);
41+
PopulateTable("BuyerSeller", new (int id, string s1, string s2)[] {
42+
(1, "123-45-6789", "987-65-4321"),
43+
(2, "234-56-7890", "876-54-3210"),
44+
(3, "345-67-8901", "765-43-2109"),
45+
(4, "456-78-9012", "654-32-1098"),
46+
}, connectionString);
47+
48+
using var conn = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
49+
await conn.OpenAsync();
50+
51+
using var adapter = CreateAdapter(conn, updateBatchSize: 10); // failure repro: > 1
52+
var dataTable = BuildBuyerSellerDataTable();
53+
LoadCurrentRowsIntoDataTable(dataTable, conn);
54+
55+
// Mutate values for update
56+
MutateForUpdate(dataTable);
57+
58+
// Act - With batch updates (UpdateBatchSize > 1), this previously threw NullReferenceException due to null systemParams in batch RPC mode
59+
var updated = await Task.Run(() => adapter.Update(dataTable));
60+
61+
// Assert
62+
Assert.Equal(dataTable.Rows.Count, updated);
63+
64+
}
65+
66+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsTargetReadyForAeWithKeyStore))]
67+
[ClassData(typeof(AEConnectionStringProvider))]
68+
public async Task AdapterUpdate_BatchSizeOne_Succeeds(string connectionString)
69+
{
70+
// Arrange
71+
TruncateTables("BuyerSeller", connectionString);
72+
PopulateTable("BuyerSeller", new (int id, string s1, string s2)[] {
73+
(1, "123-45-6789", "987-65-4321"),
74+
(2, "234-56-7890", "876-54-3210"),
75+
(3, "345-67-8901", "765-43-2109"),
76+
(4, "456-78-9012", "654-32-1098"),
77+
}, connectionString);
78+
79+
using var conn = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
80+
await conn.OpenAsync();
81+
82+
using var adapter = CreateAdapter(conn, updateBatchSize: 1); // success path
83+
var dataTable = BuildBuyerSellerDataTable();
84+
LoadCurrentRowsIntoDataTable(dataTable, conn);
85+
86+
MutateForUpdate(dataTable);
87+
88+
// Act (should not throw)
89+
var updatedRows = await Task.Run(() => adapter.Update(dataTable));
90+
91+
// Assert
92+
Assert.Equal(dataTable.Rows.Count, updatedRows);
93+
94+
}
95+
96+
// ---------- HELPERS ----------
97+
98+
private SqlDataAdapter CreateAdapter(SqlConnection connection, int updateBatchSize)
99+
{
100+
// Insert
101+
var insertCmd = new SqlCommand(tableNames["ProcInsertBuyerSeller"], connection)
102+
{
103+
CommandType = CommandType.StoredProcedure
104+
};
105+
insertCmd.Parameters.AddRange(new[]
106+
{
107+
new SqlParameter("@BuyerSellerID", SqlDbType.Int) { SourceColumn = "BuyerSellerID" },
108+
new SqlParameter("@SSN1", SqlDbType.VarChar, 255) { SourceColumn = "SSN1" },
109+
new SqlParameter("@SSN2", SqlDbType.VarChar, 255) { SourceColumn = "SSN2" },
110+
});
111+
insertCmd.UpdatedRowSource = UpdateRowSource.None;
112+
113+
// Update
114+
var updateCmd = new SqlCommand(tableNames["ProcUpdateBuyerSeller"], connection)
115+
{
116+
CommandType = CommandType.StoredProcedure
117+
};
118+
updateCmd.Parameters.AddRange(new[]
119+
{
120+
new SqlParameter("@BuyerSellerID", SqlDbType.Int) { SourceColumn = "BuyerSellerID" },
121+
new SqlParameter("@SSN1", SqlDbType.VarChar, 255) { SourceColumn = "SSN1" },
122+
new SqlParameter("@SSN2", SqlDbType.VarChar, 255) { SourceColumn = "SSN2" },
123+
});
124+
updateCmd.UpdatedRowSource = UpdateRowSource.None;
125+
126+
return new SqlDataAdapter
127+
{
128+
InsertCommand = insertCmd,
129+
UpdateCommand = updateCmd,
130+
UpdateBatchSize = updateBatchSize
131+
};
132+
}
133+
134+
private DataTable BuildBuyerSellerDataTable()
135+
{
136+
var dt = new DataTable(tableNames["BuyerSeller"]);
137+
dt.Columns.AddRange(new[]
138+
{
139+
new DataColumn("BuyerSellerID", typeof(int)),
140+
new DataColumn("SSN1", typeof(string)),
141+
new DataColumn("SSN2", typeof(string)),
142+
});
143+
dt.PrimaryKey = new[] { dt.Columns["BuyerSellerID"] };
144+
return dt;
145+
}
146+
147+
private void LoadCurrentRowsIntoDataTable(DataTable dt, SqlConnection conn)
148+
{
149+
using var cmd = new SqlCommand($"SELECT BuyerSellerID, SSN1, SSN2 FROM [dbo].[{tableNames["BuyerSeller"]}] ORDER BY BuyerSellerID", conn);
150+
using var reader = cmd.ExecuteReader();
151+
while (reader.Read())
152+
{
153+
dt.Rows.Add(reader.GetInt32(0), reader.GetString(1), reader.GetString(2));
154+
}
155+
}
156+
157+
private void MutateForUpdate(DataTable dt)
158+
{
159+
int i = 0;
160+
var fixedTime = new DateTime(2023, 01, 01, 12, 34, 56); // Use any fixed value
161+
string timeStr = fixedTime.ToString("HHmm");
162+
foreach (DataRow row in dt.Rows)
163+
{
164+
i++;
165+
row["SSN1"] = $"{i:000}-11-{timeStr}";
166+
row["SSN2"] = $"{i:000}-22-{timeStr}";
167+
}
168+
}
169+
170+
internal void TruncateTables(string tableName, string connectionString)
171+
{
172+
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
173+
connection.Open();
174+
SilentRunCommand($@"TRUNCATE TABLE [dbo].[{tableNames[tableName]}]", connection);
175+
}
176+
177+
internal void ExecuteQuery(SqlConnection connection, string commandText)
178+
{
179+
// Mirror AE-enabled command execution style used in repo tests
180+
using var cmd = new SqlCommand(
181+
commandText,
182+
connection: connection,
183+
transaction: null,
184+
columnEncryptionSetting: SqlCommandColumnEncryptionSetting.Enabled);
185+
cmd.ExecuteNonQuery();
186+
}
187+
188+
internal void PopulateTable(string tableName, (int id, string s1, string s2)[] rows, string connectionString)
189+
{
190+
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
191+
connection.Open();
192+
193+
foreach (var (id, s1, s2) in rows)
194+
{
195+
using var cmd = new SqlCommand(
196+
$@"INSERT INTO [dbo].[{tableNames[tableName]}] (BuyerSellerID, SSN1, SSN2) VALUES (@id, @s1, @s2)",
197+
connection,
198+
null,
199+
SqlCommandColumnEncryptionSetting.Enabled);
200+
201+
cmd.Parameters.Add(new SqlParameter("@id", SqlDbType.Int) { Value = id });
202+
cmd.Parameters.Add(new SqlParameter("@s1", SqlDbType.VarChar, 255) { Value = s1 });
203+
cmd.Parameters.Add(new SqlParameter("@s2", SqlDbType.VarChar, 255) { Value = s2 });
204+
205+
cmd.ExecuteNonQuery();
206+
}
207+
}
208+
209+
public string GetOpenConnectionString(string baseConnectionString, bool encryptionEnabled)
210+
{
211+
var builder = new SqlConnectionStringBuilder(baseConnectionString)
212+
{
213+
// TrustServerCertificate can be set based on environment; mirror repo’s AE toggling idiom
214+
ColumnEncryptionSetting = encryptionEnabled
215+
? SqlConnectionColumnEncryptionSetting.Enabled
216+
: SqlConnectionColumnEncryptionSetting.Disabled
217+
};
218+
return builder.ToString();
219+
}
220+
221+
internal void SilentRunCommand(string commandText, SqlConnection connection)
222+
{
223+
try
224+
{ ExecuteQuery(connection, commandText); }
225+
catch (SqlException ex)
226+
{
227+
// Only swallow "object does not exist" (error 208), log others
228+
bool onlyObjectNotExist = true;
229+
foreach (SqlError err in ex.Errors)
230+
{
231+
if (err.Number != 208)
232+
{
233+
onlyObjectNotExist = false;
234+
break;
235+
}
236+
}
237+
if (!onlyObjectNotExist)
238+
{
239+
Console.WriteLine($"SilentRunCommand: Unexpected SqlException during cleanup: {ex}");
240+
}
241+
// Swallow all exceptions, but log unexpected ones
242+
}
243+
}
244+
245+
public void Dispose()
246+
{
247+
foreach (string connectionString in DataTestUtility.AEConnStringsSetup)
248+
{
249+
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
250+
connection.Open();
251+
SilentRunCommand($"DELETE FROM [dbo].[{tableNames["BuyerSeller"]}]", connection);
252+
}
253+
}
254+
}
255+
}

‎src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
<OutputPath>$(BinFolder)$(Configuration).$(Platform).$(AssemblyName)</OutputPath>
1313
<IsTestProject>true</IsTestProject>
1414
</PropertyGroup>
15+
<ItemGroup Condition="'$(TestSet)' == 'AE'">
16+
<Compile Include="AlwaysEncrypted\SqlDataAdapterBatchUpdateTests.cs" />
17+
</ItemGroup>
1518
<ItemGroup Condition="'$(TestSet)' == '' or '$(TestSet)' == 'AE'">
1619
<Compile Include="AlwaysEncrypted\ConversionTests.cs" />
1720
<Compile Include="AlwaysEncrypted\ExceptionsGenericError.cs" />

0 commit comments

Comments
 (0)