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

Aperta
  • Qualcosa non va?
#137,241 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
csharp
Ambito
backend, testing

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.

Lingua principale
C#
Stelle
18.4k
Fork
5.8k
Merge medio
2g 19h
PR unite (30g)
590

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di dotnet/runtime

Tutte le issue di dotnet/runtime

Issue simili

Altre issue su C#

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.