Fix IsDBNull + stream read bug that skips column data

[paulmedynski]

Fix: IsDBNull/IsDBNullAsync breaks subsequent streaming reads in SequentialAccess mode

Fixes #3057

Problem

When using CommandBehavior.SequentialAccess, calling IsDBNull() or IsDBNullAsync() on a column before reading it via a streaming type (GetFieldValue<Stream>, GetFieldValue<TextReader>, GetFieldValue<XmlReader>, or their async variants) causes:

• Debug builds: DebugAssertException — "Already read past column"
• Release builds: Silent data loss — the stream returns 0 bytes

Root Cause

TryReadColumnInternal in SqlDataReader.cs has two code paths for sequential access depending on whether the column header has already been read:

Path A — header already read (i < _nextColumnHeaderToRead):

if ((i == _sharedState._nextColumnDataToRead) && (!readHeaderOnly))
{
    return TryReadColumnData();  // consumes ALL column data from the wire
}

Path B — header not yet read (_nextColumnHeaderToRead == i):

if (!readHeaderOnly && !forStreaming)
{
    result = _parser.TryReadSqlValue(…);
    _sharedState._nextColumnDataToRead++;
}

Path B correctly checks both !readHeaderOnly and !forStreaming before consuming data. Path A only checks !readHeaderOnly, ignoring forStreaming.

When IsDBNull/IsDBNullAsync is called first, it reads the column header (advancing _nextColumnHeaderToRead) but leaves data on the wire. The subsequent streaming GetFieldValue<Stream> call enters Path A (since the header is already read) and eagerly consumes all data via TryReadColumnData(), leaving nothing for the SqlSequentialStream to read.

Fix

Add && (!forStreaming) to Path A's condition to match Path B's behavior:

-if ((i == _sharedState._nextColumnDataToRead) && (!readHeaderOnly))
+if ((i == _sharedState._nextColumnDataToRead) && (!readHeaderOnly) && (!forStreaming))

This ensures that when a streaming type is requested after IsDBNull has already read the column header, the column data stays on the wire for the sequential stream to consume incrementally.

Compatibility

• The fix only affects calls where forStreaming == true, which is limited to GetFieldValue<T>/GetFieldValueAsync<T> with T being Stream, TextReader, or XmlReader.
• All other callers pass forStreaming: false (the default) — zero behavior change.
• The previous behavior was always broken (crash or silent data loss), so no application could depend on it.

Tests

Added 7 new test methods (28 test cases) covering all combinations of:

IsDBNull variant	GetFieldValue variant	Data type
IsDBNullAsync	GetFieldValueAsync<TextReader>	nvarchar
IsDBNullAsync	GetFieldValueAsync<XmlReader>	xml
IsDBNull (sync)	GetFieldValue<Stream> (sync)	varbinary
IsDBNull (sync)	GetFieldValue<TextReader> (sync)	nvarchar
IsDBNull (sync)	GetFieldValue<XmlReader> (sync)	xml
IsDBNull (sync)	GetFieldValueAsync<Stream> (async)	varbinary
IsDBNullAsync	GetFieldValue<Stream> (sync)	varbinary

Each test method has 4 inline data cases: null/non-null × checkNull true/false.

Verification:

Condition	Without fix	With fix
checkNull: True (bug path)	16 failed	0 failed
checkNull: False (control)	17 passed	33 passed

Checklist

• Tests added or updated
• Verified against customer repro
• No breaking changes introduced
• Public API changes documented (N/A — no public API changes)

[Copilot]

Pull request overview

Fixes a SqlDataReader sequential-access streaming regression where calling IsDBNull/IsDBNullAsync before GetFieldValue<T> for streaming types could consume the column payload and cause empty streams / asserts.

Changes:

• Updates SqlDataReader.TryReadColumnInternal to avoid eagerly consuming column data when forStreaming == true and the column header was already read.
• Adds new manual test coverage for IsDBNull/IsDBNullAsync + streaming GetFieldValue<T> combinations under CommandBehavior.SequentialAccess.
• Adds test-utility configurability (env var override) and hardens an Always Encrypted CSP test’s data discovery on non-Windows.

Reviewed changes

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

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs	Adjusts sequential-access read logic to preserve wire data for streaming readers after IsDBNull*.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderStreamsTest.cs	Adds manual regression tests for streaming reads after IsDBNull/IsDBNullAsync in sequential access.
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs	Adds env-var override support for test config values.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/CspProviderExt.cs	Prevents registry access during test discovery on non-Windows.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.26%. Comparing base (8cd5dda) to head (d283d14).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4082      +/-   ##
==========================================
- Coverage   65.99%   64.26%   -1.73%     
==========================================
  Files         276      271       -5     
  Lines       42951    65746   +22795     
==========================================
+ Hits        28344    42252   +13908     
- Misses      14607    23494    +8887     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	64.26% <100.00%> (?)

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.

[Copilot]

Pull request overview

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

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).