From 8ff2443711f678c78a8ec93ac4cf663f5e33e55d Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 13 Jul 2026 09:50:06 -0700 Subject: [PATCH 1/5] DCS: Add more formal BOM-detection to Json encoding stream wrappers. --- .../Json/JsonEncodingStreamWrapper.cs | 83 ++++++++++++------- .../tests/DataContractJsonSerializer.cs | 65 +++++++++++++++ 2 files changed, 117 insertions(+), 31 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs index 2e07b82314dfdf..fbd5d5f992b2d0 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs @@ -122,15 +122,12 @@ public static ArraySegment ProcessBuffer(byte[] buffer, int offset, int co try { SupportedEncoding expectedEnc = GetSupportedEncoding(encoding); - SupportedEncoding dataEnc; - if (count < 2) - { - dataEnc = SupportedEncoding.UTF8; - } - else - { - dataEnc = ReadEncoding(buffer[offset], buffer[offset + 1]); - } + SupportedEncoding dataEnc = DetectEncoding(buffer.AsSpan(offset, count), out int bomLength); + + // Skip past any byte order mark; it is not part of the document. + offset += bomLength; + count -= bomLength; + if ((expectedEnc != SupportedEncoding.None) && (expectedEnc != dataEnc)) { ThrowExpectedEncodingMismatch(expectedEnc, dataEnc); @@ -320,6 +317,42 @@ private static SupportedEncoding GetSupportedEncoding(Encoding? encoding) } } + // Determines the encoding of a JSON document from its leading bytes. A leading byte order + // mark, when present, authoritatively selects the encoding and its length is reported via + // bomLength so callers can skip past it. When no BOM is present, the encoding is inferred + // from the position of the zero byte in the leading (always ASCII) JSON character. Both the + // stream and the buffer code paths funnel through this single method so the detection logic + // lives in one place. + private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int bomLength) + { + bomLength = 0; + + if (data.Length < 2) + { + // A single-byte (or empty) JSON document is necessarily UTF-8. + return SupportedEncoding.UTF8; + } + + if (data[0] == 0xFF && data[1] == 0xFE) + { + bomLength = 2; + return SupportedEncoding.UTF16LE; + } + if (data[0] == 0xFE && data[1] == 0xFF) + { + bomLength = 2; + return SupportedEncoding.UTF16BE; + } + if (data.Length >= 3 && data[0] == 0xEF && data[1] == 0xBB && data[2] == 0xBF) + { + bomLength = 3; + return SupportedEncoding.UTF8; + } + + // No byte order mark: infer the encoding from the leading ASCII character. + return ReadEncoding(data[0], data[1]); + } + private static SupportedEncoding ReadEncoding(byte b1, byte b2) { if (b1 == 0x00 && b2 != 0x00) @@ -473,31 +506,19 @@ private void InitForWriting(Stream outputStream, Encoding writeEncoding) private SupportedEncoding ReadEncoding() { - int b1 = _stream.ReadByte(); - int b2 = _stream.ReadByte(); - EnsureByteBuffer(); - SupportedEncoding e; + // Read up to the first three bytes so a byte order mark (at most three bytes long) can + // be detected. Fewer bytes are read only when the stream ends early. + Span leading = stackalloc byte[3]; + int read = _stream.ReadAtLeast(leading, leading.Length, throwOnEndOfStream: false); - if (b1 == -1) - { - e = SupportedEncoding.UTF8; - _byteCount = 0; - } - else if (b2 == -1) - { - e = SupportedEncoding.UTF8; - _bytes[0] = (byte)b1; - _byteCount = 1; - } - else - { - e = ReadEncoding((byte)b1, (byte)b2); - _bytes[0] = (byte)b1; - _bytes[1] = (byte)b2; - _byteCount = 2; - } + SupportedEncoding e = DetectEncoding(leading.Slice(0, read), out int bomLength); + + // Preserve any bytes that follow the byte order mark; they belong to the document. + int preserve = read - bomLength; + leading.Slice(bomLength, preserve).CopyTo(_bytes); + _byteCount = preserve; return e; } diff --git a/src/libraries/System.Runtime.Serialization.Json/tests/DataContractJsonSerializer.cs b/src/libraries/System.Runtime.Serialization.Json/tests/DataContractJsonSerializer.cs index 0df7cb5f484b63..91060d57221ae4 100644 --- a/src/libraries/System.Runtime.Serialization.Json/tests/DataContractJsonSerializer.cs +++ b/src/libraries/System.Runtime.Serialization.Json/tests/DataContractJsonSerializer.cs @@ -2319,6 +2319,71 @@ public static void DCJS_CreateJsonWriterTest() } } + public static IEnumerable ByteOrderMarkEncodings() + { + yield return new object[] { new UnicodeEncoding(bigEndian: false, byteOrderMark: true) }; // UTF-16LE with BOM + yield return new object[] { new UnicodeEncoding(bigEndian: true, byteOrderMark: true) }; // UTF-16BE with BOM + yield return new object[] { new UTF8Encoding(encoderShouldEmitUTF8Identifier: true) }; // UTF-8 with BOM + } + + [Theory] + [MemberData(nameof(ByteOrderMarkEncodings))] + public static void DCJS_DeserializeStreamWithByteOrderMark(Encoding encoding) + { + var serializer = new DataContractJsonSerializer(typeof(Person1)); + var value = new Person1 { Name = "John", Age = 42 }; + byte[] bytes = GetJsonBytesWithByteOrderMark(serializer, value, encoding); + + // Auto-detected encoding (no encoding specified). This is the scenario from the bug report. + using (var stream = new MemoryStream(bytes)) + { + var result = (Person1)serializer.ReadObject(stream); + Assert.Equal(value.Name, result.Name); + Assert.Equal(value.Age, result.Age); + } + + // Encoding explicitly specified and matching the byte order mark. + using (var stream = new MemoryStream(bytes)) + { + XmlDictionaryReader reader = JsonReaderWriterFactory.CreateJsonReader(stream, encoding, XmlDictionaryReaderQuotas.Max, null); + var result = (Person1)serializer.ReadObject(reader); + Assert.Equal(value.Name, result.Name); + Assert.Equal(value.Age, result.Age); + } + } + + [Theory] + [MemberData(nameof(ByteOrderMarkEncodings))] + public static void DCJS_DeserializeBufferWithByteOrderMark(Encoding encoding) + { + var serializer = new DataContractJsonSerializer(typeof(Person1)); + var value = new Person1 { Name = "John", Age = 42 }; + byte[] bytes = GetJsonBytesWithByteOrderMark(serializer, value, encoding); + + XmlDictionaryReader reader = JsonReaderWriterFactory.CreateJsonReader(bytes, 0, bytes.Length, XmlDictionaryReaderQuotas.Max); + var result = (Person1)serializer.ReadObject(reader); + Assert.Equal(value.Name, result.Name); + Assert.Equal(value.Age, result.Age); + } + + private static byte[] GetJsonBytesWithByteOrderMark(DataContractJsonSerializer serializer, object value, Encoding encoding) + { + string json; + using (var utf8Stream = new MemoryStream()) + { + serializer.WriteObject(utf8Stream, value); + json = Encoding.UTF8.GetString(utf8Stream.ToArray()); + } + + byte[] preamble = encoding.GetPreamble(); + Assert.NotEmpty(preamble); + byte[] body = encoding.GetBytes(json); + byte[] bytes = new byte[preamble.Length + body.Length]; + preamble.CopyTo(bytes, 0); + body.CopyTo(bytes, preamble.Length); + return bytes; + } + [Fact] public static void DCJS_MyISerializableType() { From dd5c04d00460c2d72740618e9569fe118732d64f Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 13 Jul 2026 12:00:11 -0700 Subject: [PATCH 2/5] DCS: Use a single Read for JSON BOM detection to avoid blocking on short input ReadEncoding used ReadAtLeast, which loops until the requested minimum number of bytes is accumulated and therefore blocks a consumer waiting for bytes an untrusted producer may never send. Replace it with a single Stream.Read that performs one underlying read and decides the encoding from however many bytes were available, defaulting to UTF-8 when there are too few. This recognizes every byte order mark whenever the leading bytes are present while allowing valid short documents (for example a single-byte JSON value) to proceed without waiting for a fixed minimum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Serialization/Json/JsonEncodingStreamWrapper.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs index fbd5d5f992b2d0..75e8907f92055b 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs @@ -508,10 +508,16 @@ private SupportedEncoding ReadEncoding() { EnsureByteBuffer(); - // Read up to the first three bytes so a byte order mark (at most three bytes long) can - // be detected. Fewer bytes are read only when the stream ends early. + // Read whatever bytes are immediately available, up to the three occupied by the longest + // byte order mark. A single Read is used deliberately instead of ReadAtLeast: Read performs + // one underlying read and returns however many bytes were available, whereas ReadAtLeast + // loops until it has accumulated the requested count. That loop would block a consumer + // waiting for bytes an untrusted producer may never send (for example, one that transmits a + // single byte and then stalls without closing the stream). Detecting from however many bytes + // arrive - and defaulting to UTF-8 when there are too few - avoids that hang while still + // recognizing every mark whenever the leading bytes are present. Span leading = stackalloc byte[3]; - int read = _stream.ReadAtLeast(leading, leading.Length, throwOnEndOfStream: false); + int read = _stream.Read(leading); SupportedEncoding e = DetectEncoding(leading.Slice(0, read), out int bomLength); From 63558dc95c60efac71e1f7174b2c3182f16167a3 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 13 Jul 2026 22:42:52 -0700 Subject: [PATCH 3/5] Fold two stages of BOM-detection into one. --- .../Json/JsonEncodingStreamWrapper.cs | 70 ++++++++----------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs index 75e8907f92055b..0a69238b1aeb5f 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs @@ -327,52 +327,42 @@ private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int { bomLength = 0; + // Not enough characters for a BON if (data.Length < 2) { // A single-byte (or empty) JSON document is necessarily UTF-8. return SupportedEncoding.UTF8; } - if (data[0] == 0xFF && data[1] == 0xFE) + switch ((data[0], data[1])) { - bomLength = 2; - return SupportedEncoding.UTF16LE; - } - if (data[0] == 0xFE && data[1] == 0xFF) - { - bomLength = 2; - return SupportedEncoding.UTF16BE; - } - if (data.Length >= 3 && data[0] == 0xEF && data[1] == 0xBB && data[2] == 0xBF) - { - bomLength = 3; - return SupportedEncoding.UTF8; - } + // Detect known BOM's + case(0xFF, 0xFE): + bomLength = 2; + return SupportedEncoding.UTF16LE; + case(0xFE, 0xFF): + bomLength = 2; + return SupportedEncoding.UTF16BE; + case(0xEF, 0xBB): + if (data.Length >= 3 && data[2] == 0xBF) { + bomLength = 3; + return SupportedEncoding.UTF16LE; + } + break; - // No byte order mark: infer the encoding from the leading ASCII character. - return ReadEncoding(data[0], data[1]); - } + // No byte order mark or inference from the leading ASCII character. + case (0x00, not 0x00): + return SupportedEncoding.UTF16BE; + case (not 0x00, 0x00): + return SupportedEncoding.UTF16LE; - private static SupportedEncoding ReadEncoding(byte b1, byte b2) - { - if (b1 == 0x00 && b2 != 0x00) - { - return SupportedEncoding.UTF16BE; - } - else if (b1 != 0x00 && b2 == 0x00) - { - // 857 It's possible to misdetect UTF-32LE as UTF-16LE, but that's OK. - return SupportedEncoding.UTF16LE; - } - else if (b1 == 0x00 && b2 == 0x00) - { // UTF-32BE not supported - throw new XmlException(SR.JsonInvalidBytes); - } - else - { - return SupportedEncoding.UTF8; + case (0x00, 0x00): + throw new XmlException(SR.JsonInvalidBytes); } + + // No BOM detected or inferred. Assume UTF8 + return SupportedEncoding.UTF8; } private static void ThrowExpectedEncodingMismatch(SupportedEncoding expEnc, SupportedEncoding actualEnc) @@ -510,12 +500,10 @@ private SupportedEncoding ReadEncoding() // Read whatever bytes are immediately available, up to the three occupied by the longest // byte order mark. A single Read is used deliberately instead of ReadAtLeast: Read performs - // one underlying read and returns however many bytes were available, whereas ReadAtLeast - // loops until it has accumulated the requested count. That loop would block a consumer - // waiting for bytes an untrusted producer may never send (for example, one that transmits a - // single byte and then stalls without closing the stream). Detecting from however many bytes - // arrive - and defaulting to UTF-8 when there are too few - avoids that hang while still - // recognizing every mark whenever the leading bytes are present. + // one underlying read and returns however many bytes were available. If it's enough for + // BOM detection, we will try to determine encoding. If not, we continue BOM-less. + // `_stream` here is buffered, so `Read()` should be able to return a full BOM if it's there. + // We need 3 bytes for full ASCII/UTF-8/16 detection. Span leading = stackalloc byte[3]; int read = _stream.Read(leading); From d8c2e9a9d7da16891efedecd5aed1b2fab1bef32 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 14 Jul 2026 00:08:34 -0700 Subject: [PATCH 4/5] Fix syntax --- .../Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs index 0a69238b1aeb5f..81208ebb86607e 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs @@ -337,13 +337,13 @@ private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int switch ((data[0], data[1])) { // Detect known BOM's - case(0xFF, 0xFE): + case (0xFF, 0xFE): bomLength = 2; return SupportedEncoding.UTF16LE; - case(0xFE, 0xFF): + case (0xFE, 0xFF): bomLength = 2; return SupportedEncoding.UTF16BE; - case(0xEF, 0xBB): + case (0xEF, 0xBB): if (data.Length >= 3 && data[2] == 0xBF) { bomLength = 3; return SupportedEncoding.UTF16LE; From c292b1502296b8c32a61394acdb83442f68c72ff Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 14 Jul 2026 00:32:37 -0700 Subject: [PATCH 5/5] PR nits... and one super-important copy-pasta fix. --- .../Serialization/Json/JsonEncodingStreamWrapper.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs index 81208ebb86607e..a96f756e0a1ae8 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonEncodingStreamWrapper.cs @@ -327,7 +327,7 @@ private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int { bomLength = 0; - // Not enough characters for a BON + // Not enough characters for a BOM if (data.Length < 2) { // A single-byte (or empty) JSON document is necessarily UTF-8. @@ -336,7 +336,7 @@ private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int switch ((data[0], data[1])) { - // Detect known BOM's + // Detect known BOMs case (0xFF, 0xFE): bomLength = 2; return SupportedEncoding.UTF16LE; @@ -344,9 +344,10 @@ private static SupportedEncoding DetectEncoding(ReadOnlySpan data, out int bomLength = 2; return SupportedEncoding.UTF16BE; case (0xEF, 0xBB): - if (data.Length >= 3 && data[2] == 0xBF) { + if (data.Length >= 3 && data[2] == 0xBF) + { bomLength = 3; - return SupportedEncoding.UTF16LE; + return SupportedEncoding.UTF8; } break;