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

オープン
#137,241 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
68/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
csharp
領域
backend, testing

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
C#
スター
18.4k
フォーク
5.8k
平均マージ
2日 19時間
マージ済み PR(30日)
590

環境構築

Codespaces で開く

このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

dotnet/runtime のほかの issue

dotnet/runtime の issue をすべて見る

似ている issue

C# の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。