Skip to content

System.IO: Fix StringStream reads with empty encoder fallback - #131904

Open
jozkee wants to merge 2 commits into
dotnet:mainfrom
jozkee:agents/fix-stringstream-read-eof-behavior
Open

System.IO: Fix StringStream reads with empty encoder fallback#131904
jozkee wants to merge 2 commits into
dotnet:mainfrom
jozkee:agents/fix-stringstream-read-eof-behavior

Conversation

@jozkee

@jozkee jozkee commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

Fix StringStream.Read so it does not return 0 while encoded input remains after an encoder consumes characters without producing bytes.

With UTF-8 and an empty encoder replacement fallback, invalid UTF-16 characters can be consumed while producing no output. The small-buffer streaming path previously interpreted that zero-byte conversion as EOF even when later characters could still produce bytes.

The streaming path now continues conversion while:

  • no bytes have been produced,
  • source characters remain, and
  • the encoder consumed characters.

It stops after producing output or if the encoder makes no progress. The existing single-shot fast path and async delegation remain unchanged. This follows the retry-until-output-or-EOF precedent used by TranscodingStream.

Tests

Added coverage for:

  • synchronous and asynchronous reads,
  • one-byte streaming and large-buffer fast paths,
  • two and four invalid surrogate characters before valid output,
  • ReadByte, and
  • input whose characters all encode to zero bytes in both paths.

Validation

  • .\build.cmd clr+libs -rc checked — succeeded, 0 warnings, 0 errors
  • .\build.cmd clr.corelib+clr.nativecorelib+libs.pretest -rc checked — succeeded, 0 warnings, 0 errors
  • dotnet build src\libraries\System.Runtime\tests\System.IO.Tests\System.IO.Tests.csproj /t:Test '/p:XUnitOptions=-class System.IO.Tests.StringStreamTests_String_Misc' — 16 passed, 0 failed
  • dotnet build src\libraries\System.Runtime\tests\System.IO.Tests\System.IO.Tests.csproj /t:Test /p:testnobuild=true — 2,424 passed, 0 failed

Note

This pull request description was generated with GitHub Copilot.

Continue streaming conversion when the encoder consumes characters without producing bytes so Read does not signal EOF prematurely. Add synchronous, asynchronous, ReadByte, fast-path, and all-skipped-input coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 21:29
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes System.IO.StringStream.Read to avoid returning 0 (EOF) when the encoder consumed input characters but produced zero bytes (e.g., UTF-8 with an empty EncoderReplacementFallback), ensuring reads continue until bytes are produced or the encoder makes no progress. This aligns Read / ReadAsync / ReadByte behavior with expected stream semantics where 0 indicates end-of-stream.

Changes:

  • Update StringStream.Read(Span<byte>) to loop when no bytes have been produced yet but the encoder consumed characters, preventing premature EOF.
  • Add targeted tests covering sync/async reads, ReadByte, fast-path vs 1-byte streaming path, and “all input skipped” cases under empty encoder replacement fallback.

Reviewed changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs Adjusts the streaming conversion loop to retry when characters were consumed but no bytes were produced, avoiding false EOF.
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs Adds regression tests for empty encoder replacement fallback scenarios across sync/async paths and ReadByte.

@jozkee
jozkee requested a review from ViveliDuCh August 5, 2026 21:43
Copilot AI review requested due to automatic review settings August 6, 2026 15:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 no new comments.

@ViveliDuCh ViveliDuCh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for catching this!

@jozkee
jozkee enabled auto-merge (squash) August 6, 2026 15:29
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.

3 participants