[release/3.1] Port fix to revert EncoderNLS and DecoderNLS Convert changes (#27996)

Author: GrabYourPitchforks

src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs

@@ -207,12 +207,10 @@ public unsafe override void Convert(byte* bytes, int byteCount,
             charsUsed = _encoding.GetChars(bytes, byteCount, chars, charCount, this);
             bytesUsed = _bytesUsed;
 
-            // Per MSDN, "The completed output parameter indicates whether all the data in the input
-            // buffer was converted and stored in the output buffer." That means we've successfully
-            // consumed all the input _and_ there's no pending state or fallback data remaining to be output.
+            // See comment in EncoderNLS.Convert for the details of the logic below.
 
             completed = (bytesUsed == byteCount)
-                && !this.HasState
+                && (!flush || !this.HasState)
                 && (_fallbackBuffer is null || _fallbackBuffer.Remaining == 0);
 
             // Our data thingy are now full, we can return

src/System.Private.CoreLib/shared/System/Text/EncoderNLS.cs

@@ -194,12 +194,39 @@ public override unsafe void Convert(char* chars, int charCount,
             bytesUsed = _encoding.GetBytes(chars, charCount, bytes, byteCount, this);
             charsUsed = _charsUsed;
 
-            // Per MSDN, "The completed output parameter indicates whether all the data in the input
-            // buffer was converted and stored in the output buffer." That means we've successfully
-            // consumed all the input _and_ there's no pending state or fallback data remaining to be output.
+            // If the 'completed' out parameter is set to false, it means one of two things:
+            // a) this call to Convert did not consume the entire source buffer; or
+            // b) this call to Convert did consume the entire source buffer, but there's
+            //    still pending data that needs to be written to the destination buffer.
+            //
+            // In either case, the caller should slice the input buffer, provide a fresh
+            // destination buffer, and call Convert again in a loop until 'completed' is true.
+            //
+            // The caller *must* specify flush = true on the final iteration(s) of the loop
+            // and iterate until 'completed' is set to true. Otherwise data loss may occur.
+            //
+            // Technically, the expected logic is detailed below.
+            //
+            // If 'flush' = false, the 'completed' parameter MUST be set to false if not all
+            // elements of the source buffer have been consumed. The 'completed' parameter MUST
+            // be set to true once the entire source buffer has been consumed and there is no
+            // pending data for the destination buffer. (In other words, the 'completed' parameter
+            // MUST be set to true if passing a zero-length source buffer and an infinite-length
+            // destination buffer will make no forward progress.) The 'completed' parameter value
+            // is undefined for the case where all source data has been consumed but there remains
+            // pending data for the destination buffer.
+            //
+            // If 'flush' = true, the 'completed' parameter is set to true IF AND ONLY IF:
+            // a) all elements of the source buffer have been transcoded into the destination buffer; AND
+            // b) there remains no internal partial read state within this instance; AND
+            // c) there remains no pending data for the destination buffer.
+            //
+            // In other words, if 'flush' = true, then when 'completed' is set to true it should mean
+            // that all data has been converted and that this instance is indistinguishable from a
+            // freshly-reset instance.
 
             completed = (charsUsed == charCount)
-                && !this.HasState
+                && (!flush || !this.HasState)
                 && (_fallbackBuffer is null || _fallbackBuffer.Remaining == 0);
 
             // Our data thingys are now full, we can return

tests/CoreFX/CoreFX.issues.rsp

@@ -70,9 +70,12 @@
 # https://github.com/dotnet/coreclr/issues/22414
 -nomethod System.Numerics.Tests.ToStringTest.RunRegionSpecificStandardFormatToStringTests
 
-# Failure in System.Text.Encoding.Tests due to bug fix in DecoderNLS.Convert
-# https://github.com/dotnet/coreclr/issues/27191
+# Failure in System.Text.Encoding.Tests due to bug fix in EncoderNLS.Convert and DecoderNLS.Convert
+# https://github.com/dotnet/coreclr/pull/27996
 -nomethod System.Text.Tests.DecoderConvert2.PosTest6
+-nomethod System.Text.Tests.DecoderConvert2.PosTest11
+-nomethod System.Text.Tests.EncoderConvert2.EncoderUTF8ConvertMixedASCIIUnicodeCharArrayPartial
+-nomethod System.Text.Tests.EncoderConvert2.EncoderUTF8ConvertUnicodeCharArrayPartial
 
 # Timeout in System.Text.RegularExpressions.Tests.RegexMatchTests.Match_ExcessPrefix
 # https://github.com/dotnet/coreclr/issues/18912