From 4e29494f5610ecd739b1b762b36d3b93c61a4a4b Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 15 Jan 2020 13:04:09 -0800 Subject: [release/3.1] Port fix to revert EncoderNLS and DecoderNLS Convert changes (#27996) --- .../shared/System/Text/DecoderNLS.cs | 6 ++-- .../shared/System/Text/EncoderNLS.cs | 35 +++++++++++++++++++--- tests/CoreFX/CoreFX.issues.rsp | 7 +++-- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs b/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs index a8153ffd90..205890257e 100644 --- a/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs +++ b/src/System.Private.CoreLib/shared/System/Text/DecoderNLS.cs @@ -207,12 +207,10 @@ namespace System.Text 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 diff --git a/src/System.Private.CoreLib/shared/System/Text/EncoderNLS.cs b/src/System.Private.CoreLib/shared/System/Text/EncoderNLS.cs index e71ad05aa5..b2d1d6e42d 100644 --- a/src/System.Private.CoreLib/shared/System/Text/EncoderNLS.cs +++ b/src/System.Private.CoreLib/shared/System/Text/EncoderNLS.cs @@ -194,12 +194,39 @@ namespace System.Text 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 diff --git a/tests/CoreFX/CoreFX.issues.rsp b/tests/CoreFX/CoreFX.issues.rsp index fb0cfcf242..c8ef9409a8 100644 --- a/tests/CoreFX/CoreFX.issues.rsp +++ b/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 -- cgit v1.2.3