镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Validate custom union type classifier results before deserialization - #136226

Merged
eiriktsarpalis merged 1 commit into
dotnet:mainfrom
eiriktsarpalis:eiriktsarpalis-json-classifier-validation
Nov 6, 2026
Merged

eiriktsarpalis merged 1 commit into
dotnet:mainfrom
eiriktsarpalis:eiriktsarpalis-json-classifier-validation

Conversation

@eiriktsarpalis

Copy link
Copy Markdown
Member

JsonUnionConverter<TUnion> currently resolves metadata and invokes the converter for any non-null type returned by a custom classifier. Unlike polymorphic deserialization, it does not check the result against the configured case types before dispatch.

Cache the final UnionCases in a HashSet<Type> for classifier-backed unions and throw JsonException for undeclared results before resolving their metadata. Membership is exact: an undeclared subtype is rejected even when assignable to a declared case. Default classification and JSON null handling are unchanged.

The shared regression tests cover invalid and undeclared results, undeclared subtypes, cases added or removed through contract customization, and declared nullable cases. The rejection regressions were reproduced before the fix. All 154 new cases pass across reflection and source-generated serializers targeting net11.0 and net481. The affected union, structural-classifier, and polymorphism test groups also pass: 7,676 reflection cases and 3,284 source-generation cases per target, with 8 existing skips per source-generation run. Release builds pass for all four library target frameworks.

HashSet<Type> avoids adding a System.Collections.Immutable product dependency. Local lookup benchmarks found FrozenSet<Type> faster for 2-4 entries, but not consistently faster for 8-16 entries. End-to-end BenchmarkDotNet measurements on a Windows x64 Hyper-V VM were:

Operation Before After Allocated, unchanged
Scalar union deserialization 105.5 ns 108.3 ns 48 B
Object union deserialization 429.8 ns 425.9 ns 568 B

The scalar case adds approximately 3% overhead; the object case is approximately unchanged in this local run.

Note

This pull request was generated with GitHub Copilot.

Reject classifier results that are not declared union cases before resolving metadata or invoking their converters. Cache the allowed types after contract customization and cover invalid results and modified contracts with shared regression tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 60a19e82-e4d1-4d77-a6ed-cd039cd02cab
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis
eiriktsarpalis merged commit 9d98be7 into dotnet:main Nov 6, 2026
90 checks passed
@eiriktsarpalis

Copy link
Copy Markdown
Member Author

/backport to release/11.0

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0 (link to workflow run)

@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Nov 6, 2026
ViveliDuCh pushed a commit that referenced this pull request Nov 10, 2026
…eserialization (#137079)

Backport of #136226 to release/11.0

/cc @eiriktsarpalis

## Customer Impact

- [x] Customer reported
- [ ] Found internally

The newly introduced type classifier infrastructure intended to support
union type deserialization did not include any validation for
user-defined classifier delegates returning invalid types.

## Regression

- [ ] Yes
- [x] No

## Testing

Added testing covering the missed scenario.

## Risk

Low. Adds a straightforward validation step after invoking the
user-defined delegate.

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Copilot-Session: 60a19e82-e4d1-4d77-a6ed-cd039cd02cab
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.

2 participants