fix null ref in SqlDataReader

[SimonCropp]

The bug: At SqlDataReader.cs:2140, when buffer is null and bufferIndex < 0, the condition (bufferIndex < 0) is true, so execution enters the block. But buffer.Length in the exception constructor throws NullReferenceException since buffer is null.

This only affects the PLP + SequentialAccess path in GetChars. The other GetChars/GetBytes paths at lines 1760 and 1891 don't have the buffer != null && guard, so they'd NRE on the condition itself (different issue, but buffer is expected non-null there).

The fix (SqlDataReader.cs:2140): Changed buffer.Length to buffer?.Length ?? 0 so it reports size 0 when buffer is null.

The test (DataReaderTest.cs): Added GetCharsSequentialAccess_NullBufferNegativeBufferIndex_ThrowsArgumentOutOfRange which calls reader.GetChars(0, 0, null, -1, 0) with CommandBehavior.SequentialAccess on an NVARCHAR(MAX) column and asserts it throws ArgumentOutOfRangeException with paramName: "bufferIndex" — without the fix this throws NullReferenceException instead.

[apoorvdeshmukh]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Fixes a NullReferenceException in SqlDataReader.GetChars when called in the PLP + CommandBehavior.SequentialAccess path with buffer == null and a negative bufferIndex, ensuring the intended ArgumentOutOfRangeException is thrown instead.

Changes:

• Update SqlDataReader.GetChars (PLP + SequentialAccess) to avoid dereferencing buffer.Length when buffer is null while constructing the argument exception.
• Add a regression test covering GetChars with SequentialAccess + PLP, buffer == null, and negative bufferIndex.
• Add additional tests for non-sequential GetChars and GetBytes null-buffer + negative bufferIndex behavior (currently inconsistent with implementation).

Reviewed changes

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

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs	Prevents NRE when building the invalid bufferIndex exception in the PLP + SequentialAccess GetChars path; also adds some redundant null-guards in exception-path validation.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs	Adds a regression test for the sequential PLP scenario and two additional tests that currently don’t match product behavior when buffer == null.

[Copilot]

This test expects GetChars(..., buffer: null, bufferIndex: -1, ...) to throw, but the current GetChars implementation returns the field length early when buffer is null (see SqlDataReader.GetChars:2205-2209), so bufferIndex is never validated on this path. Either update the test to assert the returned length when buffer is null, or change GetChars to validate bufferIndex even when buffer is null (and ensure behavior is consistent across sequential/non-sequential paths).

[Copilot]

This test expects GetBytes(..., buffer: null, bufferIndex: -1, ...) to throw, but GetBytes returns the total byte count immediately when buffer is null (see SqlDataReader.TryGetBytesInternal:1728-1738 and 1849-1854). As written, bufferIndex isn't validated on this path, so this assertion will fail unless the product code is changed intentionally.

[Copilot]

In this catch block, buffer cannot be null because the method returns early when buffer == null (line 1850). The added buffer != null && guards and buffer?.Length ?? 0 are therefore redundant and make the validation logic harder to follow. Consider simplifying back to direct buffer.Length checks here and keeping the null-handling fix scoped to the PLP + SequentialAccess GetChars path where buffer can actually be null.

[Copilot]

Similarly here, buffer cannot be null because this code path returns early when buffer == null (line 2205). The null-guards in the exception-path validation are redundant; consider removing them to reduce diff noise and keep the validation logic consistent.