Fix BinaryReader.ReadChars for fragmented Streams

BinaryReader.ReadChars incorrectly read more than necessary from the underlying Stream when multi-byte characters straddled the read chunks. Fixes https://github.com/dotnet/corefx/issues/40455
jkotas · Aug 22, 2019 · 43a04b0 · 43a04b0
1 parent 42e189b
commit 43a04b0
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 1 deletion.
diff --git a/src/System.Private.CoreLib/shared/System/IO/BinaryReader.cs b/src/System.Private.CoreLib/shared/System/IO/BinaryReader.cs
@@ -384,6 +384,25 @@ 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;
+                        if (_2BytesPerChar && numBytes > 2)
+                            numBytes -= 2;
+                    }
+                }
+
                 if (numBytes > MaxCharBytesSize)
                 {
                     numBytes = MaxCharBytesSize;

diff --git a/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs b/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs
@@ -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;
 
         // Allow encoding to clear our must flush instead of throwing (in ThrowCharsOverflow)
         internal void ClearMustFlush()