Skip to content

Fix dictionary deserialization crash on $-prefixed keys#9983

Merged
haiyuazhang merged 3 commits intomicrosoft:mainfrom
haiyuazhang:fix/dollar-prefix-dictionary-deserialization
Mar 17, 2026
Merged

Fix dictionary deserialization crash on $-prefixed keys#9983
haiyuazhang merged 3 commits intomicrosoft:mainfrom
haiyuazhang:fix/dollar-prefix-dictionary-deserialization

Conversation

@haiyuazhang
Copy link
Member

Fixes #9982

haiyuazhang and others added 2 commits March 11, 2026 02:04
Add TryReadComplexType overload for IReadOnlyDictionary<string, T> that
manually reads dictionary entries via Utf8JsonReader, bypassing the
default System.Text.Json dictionary converter which rejects property
names starting with '$' when ReferenceHandler is enabled.

This fixes a crash when generating SDKs from specs with OData parameters
(e.g. \) that appear as dictionary keys in example values.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a \ key to the existing PutStorageTask example test data and
assert it deserializes correctly. Without the fix, this reproduces the
JsonException: 'Properties that start with \$ are not allowed in types
that support metadata.'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Mar 10, 2026
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 10, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@9983

commit: 4794b7f

@github-actions
Copy link
Contributor

No changes needing a change description found.

@haiyuazhang
Copy link
Member Author

haiyuazhang commented Mar 17, 2026

@jorgerangel-msft I ran RegenPreview locally against azure-sdk-for-net with -Azure. After fixing a TCGC version mismatch in the SDK repo (0.66.0 -> 0.66.2), 24 of 34 libraries regenerated successfully with no regressions.

The 10 failures are due to a ModelReaderWriterContextDefinition static initializer crash -- the same crash reproduces when switching to main to regen. This file was not modified in this PR.

@haiyuazhang haiyuazhang added this pull request to the merge queue Mar 17, 2026
Merged via the queue into microsoft:main with commit a2e1312 Mar 17, 2026
25 checks passed
@haiyuazhang haiyuazhang deleted the fix/dollar-prefix-dictionary-deserialization branch March 17, 2026 19:31
JoshLove-msft added a commit to JoshLove-msft/typespec that referenced this pull request Mar 20, 2026
The dictionary TryReadComplexType overload (added in microsoft#9983) manually
iterated dictionary entries and called ReadWithConverter for each value.
This corrupted the Utf8JsonReader position for nested InputExampleValue
objects, causing a TypeInitializationException during code model loading.

Fix: delegate to System.Text.Json's built-in dictionary converter via
ReadWithConverter for the whole dictionary, which correctly handles
reader positioning for nested objects with \/\ references.
For specs with \$-prefixed dictionary keys (like \), fall back
to reading without the ReferenceHandler since that is what rejects
non-metadata \$-prefixed property names.

Fixes regression from microsoft#9983.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JoshLove-msft added a commit to JoshLove-msft/typespec that referenced this pull request Mar 20, 2026
The dictionary TryReadComplexType overload (added in microsoft#9983) manually
iterated dictionary entries but treated \ metadata as regular
dictionary keys. When a dictionary value object contains \ (for
reference tracking), the manual iteration read it as a dictionary
entry key and tried to parse the reference ID string as an
InputExampleValue, corrupting the reader position.

Fix: skip \ metadata entries (via TryReadReferenceId) when iterating
dictionary keys, consistent with how other converters handle reference
tracking metadata. This preserves the manual iteration needed to handle
\$-prefixed user keys (like \) while correctly ignoring
serialization metadata.

Fixes regression from microsoft#9983.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Mar 20, 2026
…es (#10109)

The dictionary TryReadComplexType overload (added in #9983) manually
iterated dictionary entries but treated \ metadata as regular dictionary
keys. When a dictionary value object contains \ (for reference
tracking), the manual iteration read it as a dictionary entry key and
tried to parse the reference ID string as an InputExampleValue,
corrupting the reader position.

Fix: skip \ metadata entries (via TryReadReferenceId) when iterating
dictionary keys, consistent with how other converters handle reference
tracking metadata. This preserves the manual iteration needed to handle
\$-prefixed user keys (like \) while correctly ignoring serialization
metadata.

Fixes regression from #9983.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Generator crashes on specs with $-prefixed OData parameter names in example values

2 participants