-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix StackOverflowException serializing dictionary with string key and object-converter that handles string #122835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix StackOverflowException serializing dictionary with string key and object-converter that handles string #122835
Conversation
…y and object-converter that handles string When a CastingConverter<T> wraps a source converter whose Type doesn't match T (e.g., CastingConverter<string> wrapping JsonConverter<object>), and the source converter doesn't override WriteAsPropertyName, the source's fallback would look up a converter for its type (object), which delegates back via runtime type lookup causing infinite recursion. Fix by checking if the source converter's Type matches T. If not, use the base class logic which correctly uses the fallback for type T. Fixes #TBD Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a StackOverflowException that occurs when serializing dictionaries with string keys while using a custom JsonConverter<object> that handles strings via CanConvert. The root cause was infinite recursion in CastingConverter when the source converter's type doesn't match the target type - the converter would delegate to the source, which would look up a converter for its own type (e.g., object), which would resolve back to the same CastingConverter<string>, creating an infinite loop.
Key changes:
- Added type equality check in
CastingConverterbefore delegating property name serialization/deserialization methods - When types don't match, delegates to base class logic which correctly resolves the fallback converter for type
T - Added comprehensive regression test covering both serialization and deserialization scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs |
Added type equality checks to all four property name methods (ReadAsPropertyName, ReadAsPropertyNameCore, WriteAsPropertyName, WriteAsPropertyNameCore) to prevent infinite recursion by falling back to base class logic when source converter type doesn't match target type T |
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.KeyConverters.cs |
Added regression test ObjectConverterHandlingStrings_DictionaryWithStringKey_DoesNotCauseStackOverflow with helper class GenericObjectConverterHandlingStrings to verify the fix prevents StackOverflowException when using JsonConverter for string dictionary keys |
...tem.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.KeyConverters.cs
Show resolved
Hide resolved
The converter now writes empty strings for WriteAsPropertyName and returns "unexpected_key" for ReadAsPropertyName. This lets us verify the assertions exercise the correct (fallback) converter since the key "key" is preserved. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
main PR
Description
When a
CastingConverter<T>wraps a source converter whoseTypedoesn't matchT(e.g.,CastingConverter<string>wrappingJsonConverter<object>), property name serialization causes infinite recursion:CastingConverter<string>.WriteAsPropertyNameCore→ source converterWriteAsPropertyNamegets fallback for typeobject→ObjectConverterObjectConverterlooks up converter for runtime typestring→ returns sameCastingConverter<string>Fix: In
CastingConverter, check if_sourceConverter.Type == typeof(T)before delegating property name methods. If they differ, use base class logic which correctly resolves fallback for typeT.Customer Impact
Users with custom
JsonConverter<object>converters that handle specific types viaCanConvertwill hit StackOverflowException when serializing dictionaries with keys of those types.Regression
No - this is a pre-existing issue.
Testing
ObjectConverterHandlingStrings_DictionaryWithStringKey_DoesNotCauseStackOverflowwith a test converter that writes distinctive values (empty string forWriteAsPropertyName, constant forReadAsPropertyName) to verify the fallback converter is correctly used instead of the custom converterRisk
Low. The fix adds a type equality check before delegation. If types match, behavior is unchanged. If they differ, we use the base class fallback path which is already well-tested.
Package authoring no longer needed in .NET 9
IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.
Original prompt