Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 3, 2026

main PR

Description

When a CastingConverter<T> wraps a source converter whose Type doesn't match T (e.g., CastingConverter<string> wrapping JsonConverter<object>), property name serialization causes infinite recursion:

  1. CastingConverter<string>.WriteAsPropertyNameCore → source converter
  2. Source's base WriteAsPropertyName gets fallback for type objectObjectConverter
  3. ObjectConverter looks up converter for runtime type string → returns same CastingConverter<string>
  4. Loop → StackOverflow
// Reproducer
public class GenericJsonConverter : JsonConverter<object>
{
    public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string);
    public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString();
    public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString());
}

var options = new JsonSerializerOptions { Converters = { new GenericJsonConverter() } };
var value = new Dictionary<string, int> { { "key", 123 } };
JsonSerializer.Serialize(value, options); // StackOverflowException

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 type T.

Customer Impact

Users with custom JsonConverter<object> converters that handle specific types via CanConvert will hit StackOverflowException when serializing dictionaries with keys of those types.

Regression

No - this is a pre-existing issue.

Testing

  • All 49,801 System.Text.Json tests pass
  • Added regression test ObjectConverterHandlingStrings_DictionaryWithStringKey_DoesNotCauseStackOverflow

Risk

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

This section details on the original issue you should resolve

<issue_title>JsonSerializer.Serialize throws StackOverflowException serializing dictionary with string key and object-converter that handles string</issue_title>
<issue_description>This issue has been moved from a ticket on Developer Community.


[severity:It's more difficult to complete my work]
Having a custom "generic" serializer that handles multiple types (including string) and then serializing a dictionary with string key makes the Serialize call to get stuck in a forever loop (with stack overflow exception as result). Perhaps a bit odd use case but I stumbled on it after converting from Newtonsoft to .NET so I just let you know in any case. My real use case is a bit more complicated as usual but this is what I could boil it down to:

using System.Text.Json;
using System.Text.Json.Serialization;

namespace JsonSerializationTest
{
    // example minimal "generic" converter
    public class GenericJsonConverter : JsonConverter<object> // using string here works but that is not an option for my real usecase
    {
        public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string);
        public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString();
        public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString());
    }

    public class Tests
    {
        [Test]
        public void DoesNotWork()
        {
            var options = new JsonSerializerOptions();
            options.Converters.Add(new GenericJsonConverter());
            var value = new Dictionary<string, int> { { "key", 123 } };
            Assert.That(() => JsonSerializer.Serialize(value, options), Throws.InstanceOf<StackOverflowException>());
        }

        [Test]
        public void Works()
        {
            var options = new JsonSerializerOptions();
            var value = new Dictionary<string, int> { { "key", 123 } };
            Assert.DoesNotThrow(() => JsonSerializer.Serialize(value, options));
        }
    }
}

Original Comments

Feedback Bot on 14/07/2025, 09:07 AM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

Comments on the Issue (you are @copilot in this section)

@eiriktsarpalis I can reproduce, it appears to be an issue with how dictionary key serialization interacts with converters whose generic type doesn't match that of the serialized type. You could work around the issue either by making the converter be of type string, a[dopt the `JsonConverterFactory` pattern](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to#sample-factory-pattern-converter) for generic converters, or manually override the `WriteAsPropertyName` method: ```C# public class GenericJsonConverter : JsonConverter // using string here works but that is not an option for my real usecase { public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string); public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString(); public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString()); public override void WriteAsPropertyName(Utf8JsonWriter writer, [DisallowNull] object value, JsonSerializerOptions options) => writer.WritePropertyName(value.ToString()); } ```

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…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>
Copilot AI changed the title [WIP] Fix StackOverflowException in JsonSerializer for dictionary serialization Fix StackOverflowException serializing dictionary with string key and object-converter that handles string Jan 3, 2026
Copilot AI requested a review from stephentoub January 3, 2026 19:20
@stephentoub stephentoub marked this pull request as ready for review January 4, 2026 00:22
Copilot AI review requested due to automatic review settings January 4, 2026 00:22
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a 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 CastingConverter before 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

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.

JsonSerializer.Serialize throws StackOverflowException serializing dictionary with string key and object-converter that handles string

2 participants