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

Abierto
  • ¿Algo no está bien?
#137,241 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
68/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
csharp
Área
backend, testing

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
C#
Estrellas
18.4k
Forks
5.8k
Merge medio
2 d 19 h
PR fusionados (30 d)
590

Preparar el entorno

Abrir en Codespaces

Inicia el contenedor de desarrollo del proyecto en tu navegador, con tu propia cuenta de GitHub.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de dotnet/runtime

Todos los issues de dotnet/runtime

Issues similares

Más issues de C#

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.