Skip to content

Make custom JSON converters safe for multi-segment readers - #8923

Merged
flobernd merged 1 commit into
mainfrom
fix-converter-sequence-safety
Jun 24, 2026
Merged

flobernd merged 1 commit into
mainfrom
fix-converter-sequence-safety

Conversation

@flobernd

Copy link
Copy Markdown
Member

Summary

The hand-written DateTime, TimeSpan, Stringified, and fractional numeric converters read reader.ValueSpan directly to parse stringified numbers with Utf8Parser. ValueSpan is unsafe when Utf8JsonReader is constructed over a multi-segment ReadOnlySequence<byte> (it throws, or returns a non-contiguous span), so these converters only worked on the single-segment reader that JsonSerializer uses for stream deserialization. Any sequence-based deserializer, such as a streaming reader over a PipeReader, failed on the first stringified epoch date or duration. Surfaced while prototyping streaming deserialization for #8919.

Changes

  • Add JsonReaderExtensions.GetValueBytes(ref reader, scoped Span<byte>): copies the current token's value into a caller-provided contiguous buffer (single-segment via ValueSpan, multi-segment by iterating ValueSequence segments), returns the length, and throws JsonException when the value exceeds the buffer.
  • Update the 18 numeric/date converters to parse from buffer[..length], with the scratch buffer sized per type (bool/int 16, long/float 32, double and ISO date 64).
  • IndexSettingsTimeSeriesConverter extended-year first-byte peek now reads reader.ValueSequence.First.Span / reader.ValueSpan directly, avoiding a GetString allocation.
  • The scoped modifier on the buffer parameter is required so a stackalloc can be passed (otherwise the by-value Span would be escape-able through the unscoped ref Utf8JsonReader, CS8350).

Generated converters were already sequence-safe via the HasValueSequence-guarded ValueTextEquals, so no generator changes are needed. Builds clean across the target frameworks (verified net10.0 and netstandard2.0); the existing single-segment typed deserialize path is unchanged.

The hand-written DateTime, TimeSpan, Stringified, and fractional numeric
converters read reader.ValueSpan directly to parse stringified numbers with
Utf8Parser. ValueSpan is unsafe when Utf8JsonReader is constructed over a
multi-segment ReadOnlySequence (it throws, or returns a non-contiguous
span), so these converters only worked on the single-segment reader that
JsonSerializer uses for stream deserialization. Any sequence-based
deserializer, such as a streaming reader over a PipeReader, failed on the
first stringified epoch date or duration.

Add JsonReaderExtensions.GetValueBytes(ref reader, scoped Span<byte>) which
copies the current token's value into a caller-provided contiguous buffer
(single-segment via ValueSpan, multi-segment by iterating ValueSequence
segments), returns its length, and throws JsonException when the value
exceeds the buffer. The 18 numeric/date converters now parse from
buffer[..length], with the scratch buffer sized per type (bool/int 16,
long/float 32, double and ISO date 64). The IndexSettingsTimeSeries
extended-year peek only needs the first byte and now reads
reader.ValueSequence.First.Span / reader.ValueSpan directly, avoiding a
GetString allocation.

The scoped modifier on the buffer parameter is required: without it the
by-value Span would be escape-able through the unscoped ref Utf8JsonReader,
so passing a stackalloc would be rejected (CS8350). Generated converters
were already sequence-safe via the HasValueSequence-guarded ValueTextEquals,
so no generator changes are needed.
@flobernd
flobernd merged commit 8d5b50b into main Jun 24, 2026
27 of 29 checks passed
@flobernd
flobernd deleted the fix-converter-sequence-safety branch June 24, 2026 15:02
@github-actions

Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.19
9.4

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

flobernd added a commit that referenced this pull request Jun 24, 2026
) (#8924)

Co-authored-by: Florian Bernd <git@flobernd.de>
flobernd added a commit that referenced this pull request Jun 24, 2026
…) (#8925)

Co-authored-by: Florian Bernd <git@flobernd.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant