Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix BinaryReader.ReadChars for fragmented Streams #26324

Merged
merged 2 commits into from
Aug 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/System.Private.CoreLib/shared/System/IO/BinaryReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,28 @@ private int InternalReadChars(Span<char> buffer)
{
numBytes <<= 1;
}

// We do not want to read even a single byte more than necessary.
//
// Subtract pending bytes that the decoder may be holding onto. This assumes that each
// decoded char corresponds to one or more bytes. Note that custom encodings or encodings with
// a custom replacement sequence may violate this assumption.
if (numBytes > 1)
{
DecoderNLS? decoder = _decoder as DecoderNLS;
// For internal decoders, we can check whether the decoder has any pending state.
// For custom decoders, assume that the decoder has pending state.
if (decoder == null || decoder.HasState)
{
numBytes -= 1;

// The worst case is charsRemaining = 2 and UTF32Decoder holding onto 3 pending bytes. We need to read just
// one byte in this case.
if (_2BytesPerChar && numBytes > 2)
numBytes -= 2;
}
}

if (numBytes > MaxCharBytesSize)
{
numBytes = MaxCharBytesSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public override unsafe void Convert(byte* bytes, int byteCount,
public bool MustFlush => _mustFlush;

// Anything left in our decoder?
internal virtual bool HasState => false;
internal virtual bool HasState => _leftoverByteCount != 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

@GrabYourPitchforks Could you please review this part of the fix?


// Allow encoding to clear our must flush instead of throwing (in ThrowCharsOverflow)
internal void ClearMustFlush()
Expand Down