[System.Text.Json] PipeReadBufferState multi-segment BOM scanner rebases on mismatch and can slice 3 non-BOM bytes off the payload

Đang mở
  • Có gì không ổn?
#137,241 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
68/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
csharp
Lĩnh vực
backend, testing

Hướng nghiên cứu

Start with src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PipeReadBufferState.cs and trace ProcessReadBytes' multi-segment BOM scan. Add a test using a hand-built ReadOnlySequence<byte> with a non-BOM first segment and a later segment beginning with the BOM bytes; check the existing System.Text.Json tests for multi-segment sequence construction. Done when that input retains its first three payload bytes and split BOM handling still works.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

area-System.IO untriaged
Description

PipeReadBufferState.ProcessReadBytes has a multi-segment UTF-8 BOM scanner with two compounding faults. On a mismatch it rebases the scan at the next segment instead of aborting, and the slice that follows assumes the match began at offset 0 of the sequence. Together these can cause the first three bytes of the payload to be discarded when a segment other than the first begins with the BOM bytes.

File: src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PipeReadBufferState.cs

else
{
    // BOM spans multiple segments                                                      // :114
    SequencePosition pos = _sequence.Start;
    int matched = 0;                                                                    // :116
    while (matched < JsonConstants.Utf8Bom.Length && _sequence.TryGet(ref pos, out ReadOnlyMemory<byte> mem, advance: true))   // :117
    {
        ReadOnlySpan<byte> span = mem.Span;
        for (int i = 0; i < span.Length && matched < JsonConstants.Utf8Bom.Length; i++, matched++)
        {
            if (span[i] != JsonConstants.Utf8Bom[matched])
            {
                matched = 0;                                                            // :124
                break;                                                                  // inner `for` only
            }
        }
    }

    if (matched == JsonConstants.Utf8Bom.Length)                                        // :130
    {
        _sequence = _sequence.Slice(JsonConstants.Utf8Bom.Length);                      // :132
    }
}

Fault 1 — a mismatch does not abort the scan. matched = 0; break; leaves the inner for only. The outer while condition is matched < 3, which is still true, so it advances to the next segment and restarts matching from Utf8Bom[0]. A mismatch therefore rebases the scan rather than ending it.

Fault 2 — the slice assumes the match started at the beginning. _sequence.Slice(3) removes the first three bytes of the sequence. That is only correct if the match began at sequence offset 0. After fault 1 has rebased the scan to a later segment, it is not.

This branch is reached only when _sequence.First.Length < JsonConstants.Utf8Bom.Length — i.e. the first segment is shorter than 3 bytes. The fast path at :105-111 handles the common case correctly, and the genuinely-split BOM case (for example [EF] then [BB BF ...]) also works, so any fix needs to preserve both.

Worked example

First segment 41 42 ("AB"), second segment beginning EF BB BF:

  1. while: matched = 0, segment 1 → span[0] = 0x41 vs Utf8Bom[0] = 0xEF → mismatch → matched = 0, break out of the for.
  2. while: matched = 0 < 3, so it advances to segment 2 → EF, BB, BF all match → matched = 3.
  3. matched == 3 → _sequence.Slice(3) removes sequence bytes 0–2, which are 41 42 EF.

The payload is left starting at BB BF .... Three bytes that were not a BOM have been discarded and the remainder is corrupt, so deserialization then fails on input that was well-formed.

The bytes EF BB BF are also the UTF-8 encoding of U+FEFF, so they can legitimately appear inside a JSON string value, not only as a leading BOM.

Reproduction, and an honest statement of what I did not do

I have not produced a runtime repro, and I do not want to overstate this. The trigger depends on the segment layout the PipeReader happens to produce, and a first segment shorter than three bytes is unusual in practice. What I am reporting is that the scanner's logic is incorrect on its own terms: a mismatch should end the scan, and the slice should be tied to where the match actually began.

A unit test over a hand-built ReadOnlySequence<byte> with the segmentation above would exercise it directly without depending on pipe behaviour — System.Text.Json's existing tests already construct multi-segment sequences this way.

Scope

ProcessReadBytes does not exist on release/9.0 (the method is absent from that version of the file), so this is new code in the .NET 10 PipeReader deserialization path. The same logic is present on release/11.0 and on main, both checked through the GitHub contents API.

I could not find an existing issue. PipeReadBufferState → 4 results, all unrelated (#136173 boxing, #129471 and #121564 test failures, #118408 sequence/stream); ProcessReadBytes BOM → 0; Utf8Bom segment → 1, unrelated. Control: System.Text.Json BOM → 25 results, so the search is live.

Suggested fix

Abort the scan on a mismatch rather than rebasing, so that matched == 3 can only mean "the sequence starts with a BOM". For example, track the mismatch and break the outer loop as well:

SequencePosition pos = _sequence.Start;
int matched = 0;
bool mismatch = false;
while (!mismatch && matched < JsonConstants.Utf8Bom.Length &&
       _sequence.TryGet(ref pos, out ReadOnlyMemory<byte> mem, advance: true))
{
    ReadOnlySpan<byte> span = mem.Span;
    for (int i = 0; i < span.Length && matched < JsonConstants.Utf8Bom.Length; i++, matched++)
    {
        if (span[i] != JsonConstants.Utf8Bom[matched])
        {
            mismatch = true;
            break;
        }
    }
}

if (!mismatch && matched == JsonConstants.Utf8Bom.Length)
{
    _sequence = _sequence.Slice(JsonConstants.Utf8Bom.Length);
}

That keeps the fast path and the legitimately-split-BOM case working while making a mismatch terminal. I have not opened a PR; happy to if a maintainer prefers a particular shape.

Configuration

.NET 10 (10.0 servicing). Also present on release/11.0 and main. Not applicable to .NET 9 — the method does not exist there.

Other information

Found while reading the .NET 10 PipeReader-based JSON deserialization path. Analysis was done with the help of Claude Code; every line reference was verified by hand against the source and the branch checks were made through the GitHub contents API.

Ngôn ngữ chính
C#
Star
18.4k
Fork
5.8k
Merge trung bình
2 ngày 19 giờ
Pull request đã merge (30 ngày)
590

Chuẩn bị môi trường

Mở trong Codespaces

Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của dotnet/runtime

Tất cả issue của dotnet/runtime

Issue tương tự

Thêm issue về C#

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.