Skip to content

Fix JsonReaderData buffer refill to respect valid bytes on partial stream reads#38666

Merged
AndriySvyryd merged 4 commits into
mainfrom
copilot/fix-jsonreaderdata-leftover-calculation
Jul 21, 2026
Merged

Fix JsonReaderData buffer refill to respect valid bytes on partial stream reads#38666
AndriySvyryd merged 4 commits into
mainfrom
copilot/fix-jsonreaderdata-leftover-calculation

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

JsonReaderData.ReadBytes treated buffer.Length as valid data during refills, even when _bytesAvailable was smaller after short stream reads. That could copy stale bytes into the next reader window, skew BytesConsumed, and break JSON tokenization.

  • Refill logic now uses valid-byte boundaries

    • Leftover detection changed from totalConsumed < buffer.Length to totalConsumed < _bytesAvailable.
    • Leftover slice now uses only unread valid bytes: buffer.AsSpan(totalConsumed, _bytesAvailable - totalConsumed).
  • Refill reads now handle short reads correctly

    • Replaced single Read(...) calls with a bounded loop that keeps reading until the target span is filled or EOF.
    • Preserves the intended relationship between _bytesAvailable, buffered content, and Utf8JsonReader final-block behavior.
  • Regression coverage for partial-read streams

    • Added a focused test using a stream that intentionally returns small chunks per read.
    • Confirms full parse to EndArray and exact BytesConsumed accounting.
if (_bytesAvailable != 0 && totalConsumed < _bytesAvailable)
{
    var leftover = buffer.AsSpan(totalConsumed, _bytesAvailable - totalConsumed);
    leftover.CopyTo(buffer);
    _bytesAvailable = ReadToEnd(_stream, buffer.AsSpan(leftover.Length)) + leftover.Length;
}

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix leftover calculation in JsonReaderData during stream refill Fix JsonReaderData buffer refill to respect valid bytes on partial stream reads Jul 17, 2026
Copilot AI requested a review from AndriySvyryd July 17, 2026 04:42
@AndriySvyryd
AndriySvyryd requested a review from Copilot July 17, 2026 19:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes JsonReaderData.ReadBytes refill behavior when a stream returns short/partial reads by ensuring only valid bytes are treated as available, preventing stale buffer data from being reused and skewing BytesConsumed / tokenization.

Changes:

  • Adjusted refill “leftover” calculation to use _bytesAvailable (valid bytes) rather than buffer.Length.
  • Replaced single Stream.Read(...) calls with a bounded read loop (ReadToEnd) that fills the target span until EOF.
  • Added a regression test using a stream that intentionally returns small chunks per read, asserting correct end-to-end parsing and BytesConsumed.

Reviewed changes

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

File Description
src/EFCore/Storage/Json/JsonReaderData.cs Fixes buffer refill logic to respect valid-byte boundaries and handle partial stream reads via a read-to-fill loop.
test/EFCore.Tests/Storage/Json/JsonReaderDataTest.cs Adds regression coverage for partial-read streams to validate BytesConsumed correctness through EndArray.

@AndriySvyryd
AndriySvyryd marked this pull request as ready for review July 18, 2026 01:13
@AndriySvyryd
AndriySvyryd requested a review from a team as a code owner July 18, 2026 01:13
@AndriySvyryd
AndriySvyryd requested a review from cincuranet July 18, 2026 01:14
Comment thread src/EFCore/Storage/Json/JsonReaderData.cs Outdated
Comment thread src/EFCore/Storage/Json/JsonReaderData.cs Outdated
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 20, 2026 23:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread test/EFCore.Tests/Storage/Json/JsonReaderDataTest.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 20, 2026 23:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread test/EFCore.Tests/Storage/Json/JsonReaderDataTest.cs
Comment thread test/EFCore.Tests/Storage/Json/JsonReaderDataTest.cs
@AndriySvyryd
AndriySvyryd enabled auto-merge (squash) July 20, 2026 23:36
@AndriySvyryd
AndriySvyryd requested a review from cincuranet July 21, 2026 00:53
@AndriySvyryd
AndriySvyryd merged commit 7e244eb into main Jul 21, 2026
16 checks passed
@AndriySvyryd
AndriySvyryd deleted the copilot/fix-jsonreaderdata-leftover-calculation branch July 21, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possible issue in JsonReaderData commented by copilot on off-topic pr

4 participants