Skip to content

Deserialize escaped ULID JSON strings and dictionary keys - #119

Open
dajiaohuang wants to merge 1 commit into
Cysharp:masterfrom
dajiaohuang:fix/json-escaped-ulid-20261015
Open

dajiaohuang wants to merge 1 commit into
Cysharp:masterfrom
dajiaohuang:fix/json-escaped-ulid-20261015

Conversation

@dajiaohuang

Copy link
Copy Markdown

Problem

The advertised System.Text.Json converter rejects a valid ULID when its JSON string contains an escape. For example, JsonSerializer.Deserialize<Ulid>("\"\\u00301ARZ3NDEKTSV4RRFFQ69G5FAV\"") throws JsonException: Ulid invalid: length must be 26, although the decoded string is the ordinary 26-character ULID 01ARZ3NDEKTSV4RRFFQ69G5FAV.

The converter measures and parses the raw UTF-8 token, not its decoded string. Dictionary keys and multi-segment input have the same problem.

Change

Decode escaped JSON tokens with Utf8JsonReader.GetString() before checking the decoded length and parsing. Keep the existing byte-span/sequence path for unescaped tokens. The escape detection uses APIs available on the supported .NET 6 target; no new dependency or parsing-policy change is introduced.

Four regression cases exercise JsonSerializer.Deserialize for ordinary values and dictionary keys, using contiguous and segmented input. All use a synthetic ULID.

Validation

  • All four new cases fail on the unchanged base with the length JsonException, and pass after the change.
  • Release solution build and symbol pack pass, including all existing library TFMs.
  • All 37 solution tests pass normally, with hardware intrinsics/SSE2 disabled, and with SSE2 disabled alone.
  • All eight JSON converter tests also pass when targeted to native .NET 6.0.11.

Windows x64, SDK 10.0.100. The default net7.0 test assemblies ran on .NET 8.0.22 with DOTNET_ROLL_FORWARD=Major because native .NET 7 is not installed. Existing EOL/dependency/assembly warnings remain. Unity editor, browser execution and big-endian hardware were not tested; this does not change the existing handling of invalid Base32 text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant