Fixing NullReferenceException issue with SqlDataAdapter by priyankatiwari08 · Pull Request #3749 · dotnet/SqlClient

priyankatiwari08 · 2025-11-06T17:37:13Z

Description

This Pull Request addresses issue #3716 by introducing a null check for systemParams, which stores the system-level parameters for SQL RPC (Remote Procedure Call) operations. In batch scenarios, certain SQL RPC calls may not include system parameters, and this change ensures proper handling in such cases.

Issues

#3716

Copilot

Pull Request Overview

This pull request adds a null check for systemParams before accessing its Length property in the parameter encryption metadata retrieval logic. This defensive programming change prevents a potential NullReferenceException when processing RPC batch commands.

Key Changes

Added null check for _RPCList[i].systemParams before accessing its Length property in TryFetchInputParameterEncryptionInfo method

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs:1296

The added null check for systemParams lacks test coverage. Based on the code analysis, systemParams is initialized through GetRPCObject() which always allocates and initializes the array when systemParamCount > 0. However, a newly constructed _SqlRPC object (as seen in AddBatchCommand line 21) has systemParams initially null. A test case should be added that exercises the batch RPC mode encryption path with an RPC object that has not yet had its systemParams initialized to verify this defensive check works correctly.

                    if (_RPCList[i].systemParams != null && _RPCList[i].systemParams.Length > 1)

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

paulmedynski

Need some tests for this.

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj

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

apoorvdeshmukh

There are build failures while compiling the tests.

...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj

…SqlClient.ManualTesting.Tests.csproj Co-authored-by: Copilot <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

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

…dotnet/SqlClient into dev/prtiwar/SqlDataAdapterIssue

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj

…apterIssue r Please enter a commit message to explain why this merge is necessary, Merging with main

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

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

priyankatiwari08 · 2025-12-04T09:01:23Z

@dotnet-policy-service agree company="Microsoft"

codecov · 2025-12-04T10:19:36Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.84%. Comparing base (995df25) to head (6137517).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
.../Microsoft/Data/SqlClient/SqlCommand.Encryption.cs	0.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (995df25) and HEAD (6137517). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (995df25) HEAD (6137517)

addons 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3749      +/-   ##
==========================================
- Coverage   76.61%   69.84%   -6.77%     
==========================================
  Files         274      268       -6     
  Lines       43393    66432   +23039     
==========================================
+ Hits        33247    46402   +13155     
- Misses      10146    20030    +9884

Flag	Coverage Δ
addons	`?`
netcore	`69.86% <0.00%> (-6.82%)`	⬇️
netfx	`69.22% <0.00%> (-6.97%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…rtiwar/SqlDataAdapterIssue

This reverts commit c23fed6.

…" (#3854) This reverts commit c23fed6.

Fixing NullReferenceException issue with SqlDataAdapter

0488e8c

priyankatiwari08 requested a review from a team as a code owner November 6, 2025 17:37

Copilot AI review requested due to automatic review settings November 6, 2025 17:37

github-project-automation bot added this to SqlClient Board Nov 6, 2025

github-project-automation bot moved this to To triage in SqlClient Board Nov 6, 2025

Copilot AI reviewed Nov 6, 2025

View reviewed changes

apoorvdeshmukh reviewed Nov 6, 2025

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Show resolved Hide resolved

priyankatiwari08 added the P1 label Nov 6, 2025

paulmedynski approved these changes Nov 6, 2025

View reviewed changes

paulmedynski requested changes Nov 6, 2025

View reviewed changes

paulmedynski self-assigned this Nov 6, 2025

Added tests for this scenario

b6bdbaa

priyankatiwari08 requested review from apoorvdeshmukh, Copilot and paulmedynski November 11, 2025 12:28

Copilot started reviewing on behalf of priyankatiwari08 November 11, 2025 12:51 View session

Copilot finished reviewing on behalf of priyankatiwari08 November 11, 2025 12:53

Copilot AI reviewed Nov 11, 2025

View reviewed changes

apoorvdeshmukh requested changes Nov 11, 2025

View reviewed changes

...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj Outdated Show resolved Hide resolved

Update src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.…

92fc6cb

…SqlClient.ManualTesting.Tests.csproj Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings November 11, 2025 17:14

Copilot started reviewing on behalf of priyankatiwari08 November 11, 2025 17:15 View session

Copilot finished reviewing on behalf of priyankatiwari08 November 11, 2025 17:18

Copilot AI reviewed Nov 11, 2025

View reviewed changes

apoorvdeshmukh self-assigned this Nov 11, 2025

cheenamalhotra moved this from To triage to In review in SqlClient Board Nov 17, 2025

priyankatiwari08 added 2 commits November 18, 2025 23:26

addressed PR comments related fixes.

ae05d9d

Merge branch 'dev/prtiwar/SqlDataAdapterIssue' of https://github.com/…

a8bda9a

…dotnet/SqlClient into dev/prtiwar/SqlDataAdapterIssue

priyankatiwari08 requested a review from Copilot November 18, 2025 17:59

Copilot started reviewing on behalf of priyankatiwari08 November 18, 2025 17:59 View session

Copilot started reviewing on behalf of priyankatiwari08 November 19, 2025 07:33 View session

Copilot finished reviewing on behalf of priyankatiwari08 November 19, 2025 07:35

Copilot AI reviewed Nov 19, 2025

View reviewed changes

...crosoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj Outdated Show resolved Hide resolved

.

e926fb5

priyankatiwari08 requested a review from apoorvdeshmukh November 19, 2025 09:16

paulmedynski approved these changes Nov 19, 2025

View reviewed changes

cheenamalhotra removed the P1 label Nov 21, 2025

apoorvdeshmukh approved these changes Nov 28, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into dev/prtiwar/SqlDataAd…

6137517

…apterIssue r Please enter a commit message to explain why this merge is necessary, Merging with main

Copilot AI review requested due to automatic review settings December 2, 2025 10:44

Copilot started reviewing on behalf of priyankatiwari08 December 2, 2025 10:44 View session

Copilot finished reviewing on behalf of priyankatiwari08 December 2, 2025 10:48

Copilot AI reviewed Dec 2, 2025

View reviewed changes

Merge branch 'main' of https://github.com/dotnet/SqlClient into dev/p…

7887c01

…rtiwar/SqlDataAdapterIssue

priyankatiwari08 merged commit c23fed6 into main Dec 9, 2025
250 checks passed

priyankatiwari08 deleted the dev/prtiwar/SqlDataAdapterIssue branch December 9, 2025 17:10

github-project-automation bot moved this from In review to Done in SqlClient Board Dec 9, 2025

paulmedynski added this to the 7.0.0-preview4 milestone Dec 10, 2025

apoorvdeshmukh added a commit that referenced this pull request Dec 18, 2025

Revert "Fixing NullReferenceException issue with SqlDataAdapter (#3749)"

c5ac3ff

This reverts commit c23fed6.

apoorvdeshmukh added a commit that referenced this pull request Dec 18, 2025

Revert "Fixing NullReferenceException issue with SqlDataAdapter (#3749)…

07f524c

…" (#3854) This reverts commit c23fed6.

cheenamalhotra removed this from the 7.0.0-preview4 milestone Jan 9, 2026

paulmedynski added this to the 7.0.0-preview4 milestone Feb 11, 2026

Conversation

priyankatiwari08 commented Nov 6, 2025

Description

Issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Uh oh!

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

apoorvdeshmukh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

priyankatiwari08 commented Dec 4, 2025

Uh oh!

codecov bot commented Dec 4, 2025

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants