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 68.10%. Comparing base (60d4b92) to head (c2a9aee).
⚠️ Report is 2 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (60d4b92) and HEAD (c2a9aee). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (60d4b92)	HEAD (c2a9aee)
CI-SqlClient	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4082      +/-   ##
==========================================
- Coverage   73.22%   68.10%   -5.13%     
==========================================
  Files         280      275       -5     
  Lines       43000    66925   +23925     
==========================================
+ Hits        31486    45578   +14092     
- Misses      11514    21347    +9833     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	68.10% <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.

[paulmedynski]

This project doesn't use xUnit. Including these was causing VS Code test explorer to attempt test discovery, which failed consistently when builds occurred elsewhere.

[paulmedynski]

The test class is gated by [PlatformSpecific(TestPlatforms.Windows)], but that only applies at test execution time. Test discovery occurs regardless of attributes like these, and in this case would fail on Unix due to the Windows-specific registry calls below.

[paulmedynski]

Fixed broken conditions here, that would pass the test if the Read() failed.

[paulmedynski]

New tests that expose the bug behaviour. All of these tests fail without the change to SqlDataReader.

[paulmedynski]

This makes it easy to run many tests (like the new ones above) with a single command:

MDS_TCPConnectionString="Server=tcp:127.0.0.1,1434;Database=Northwind;User ID=sa;Password=****;Encrypt=false;TrustServerCertificate=true" dotnet test --no-build src/Microsoft.Data.SqlClient/tests/ManualTests/ -f net10.0 --filter IsDBNull

No need to jump through the config file hoops.

[cheenamalhotra]

Please document the same in BUILDGUIDE so its easier for developers to know about these variables that can be used for local development.