System.Text.Json Polymorphic Type Resolving Issue

[akurone]

Description

hi there,
as discussed in #dotnet/aspnetcore/issues/41399 SystemTextJsonOutputFormatter calls serializer with derived type of the object and this seems reasonable (as per the comment in the code). when called like this, even the attributes introduced in #63747 are present, the serializer does not output type discriminators. in my use case this causes a deserialization problem: without the discriminators, receiving side cannot use the json produced (by the endpoint). i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

Reproduction Steps

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

try
{
	Console.WriteLine($"{nameof(DerivedA)}, showing workaround:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedA(), typeof(DerivedA)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, like STJOutputFormatter:");
	//STJOutputFormatter calls serializer this way https://github.com/dotnet/aspnetcore/issues/41399
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(DerivedB)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type generic works as expected:");
	Console.WriteLine(JsonSerializer.Serialize<SomeBaseType>(new DerivedB()));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type info works as expected:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(SomeBaseType)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedC)}, workaround is crippled:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedC(), typeof(DerivedC)));
}
catch (Exception ex)
{
	Console.WriteLine(ex.Message);
}
Console.ReadLine();


[JsonPolymorphic/*properties of this attribute are not altering the result in this case*/,
 JsonDerivedType(typeof(DerivedA), nameof(DerivedA)),
 JsonDerivedType(typeof(DerivedB), nameof(DerivedB)),
 JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
public abstract class SomeBaseType { public int Prop1 { get; set; } = 1; }

[JsonDerivedType(typeof(DerivedA), nameof(DerivedA))] // this extra attribute is **the** workaround
public class DerivedA : SomeBaseType { public int Prop2 { get; set; } = 2; }

//no extra attribute
public class DerivedB : SomeBaseType { public int Prop3 { get; set; } = 3; }

[JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
//sealing this type will throw: https://github.com/dotnet/runtime/blob/v7.0.0-rc.2.22472.3/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs#L174
//but this is a valid use case here
public sealed class DerivedC : SomeBaseType { public int Prop4 { get; set; } = 4; }

Expected behavior

when the base type is decorated with required attributes (or other tools mentioned in #63747 are utilized to provide polymorphic information) the serializer should respect those even the call is made with direct type and add correct discriminator to output.
2nd case of the repro code should print:

DerivedB, like STJOutputFormatter:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

Actual behavior

when called with the derived type serializer is not adding discriminators to output (result of the repro code):

DerivedA, showing workaround:
{"$type":"DerivedA","Prop2":2,"Prop1":1}

DerivedB, like STJOutputFormatter:
{"Prop3":3,"Prop1":1}

DerivedB, with base type generic works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedB, with base type info works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedC, workaround is crippled:
Specified type 'DerivedC' does not support polymorphism. Polymorphic types cannot be structs, sealed types, generic types or System.Object.

Regression?

no

Known Workarounds

see the repro code.

Configuration

SDK: 7.0.100-rc.2.22477.23

Other information

No response

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

Issue Details

---

Description

hi there,
as discussed in #dotnet/aspnetcore/issues/41399 SystemTextJsonOutputFormatter calls serializer with derived type of the object and this seems reasonable (as per the comment in the code). when called like this, even the attributes introduced in #63747 are present, the serializer does not output type discriminators. in my use case this causes a deserialization problem: without the discriminators, receiving side cannot use the json produced (by the endpoint). i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

Reproduction Steps

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

try
{
	Console.WriteLine($"{nameof(DerivedA)}, showing workaround:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedA(), typeof(DerivedA)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, like STJOutputFormatter:");
	//STJOutputFormatter calls serializer this way https://github.com/dotnet/aspnetcore/issues/41399
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(DerivedB)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type generic works as expected:");
	Console.WriteLine(JsonSerializer.Serialize<SomeBaseType>(new DerivedB()));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type info works as expected:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(SomeBaseType)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedC)}, workaround is crippled:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedC(), typeof(DerivedC)));
}
catch (Exception ex)
{
	Console.WriteLine(ex.Message);
}
Console.ReadLine();


[JsonPolymorphic/*properties of this attribute are not altering the result in this case*/,
 JsonDerivedType(typeof(DerivedA), nameof(DerivedA)),
 JsonDerivedType(typeof(DerivedB), nameof(DerivedB)),
 JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
public abstract class SomeBaseType { public int Prop1 { get; set; } = 1; }

[JsonDerivedType(typeof(DerivedA), nameof(DerivedA))] // this extra attribute is **the** workaround
public class DerivedA : SomeBaseType { public int Prop2 { get; set; } = 2; }

//no extra attribute
public class DerivedB : SomeBaseType { public int Prop3 { get; set; } = 3; }

[JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
//sealing this type will throw: https://github.com/dotnet/runtime/blob/v7.0.0-rc.2.22472.3/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs#L174
//but this is a valid use case here
public sealed class DerivedC : SomeBaseType { public int Prop4 { get; set; } = 4; }

Expected behavior

when the base type is decorated with required attributes (or other tools mentioned in #63747 are utilized to provide polymorphic information) the serializer should respect those even the call is made with direct type and add correct discriminator to output.
2nd case of the repro code should print:

DerivedB, like STJOutputFormatter:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

Actual behavior

when called with the derived type serializer is not adding discriminators to output (result of the repro code):

DerivedA, showing workaround:
{"$type":"DerivedA","Prop2":2,"Prop1":1}

DerivedB, like STJOutputFormatter:
{"Prop3":3,"Prop1":1}

DerivedB, with base type generic works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedB, with base type info works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedC, workaround is crippled:
Specified type 'DerivedC' does not support polymorphism. Polymorphic types cannot be structs, sealed types, generic types or System.Object.

Regression?

no

Known Workarounds

see the repro code.

Configuration

SDK: 7.0.100-rc.2.22477.23

Other information

No response

Author:	akurone
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

This is by design -- polymorphic serialization is type-directed and as such only works if the declared type has been configured as polymorphic and this configuration is not inherited by derived types. This matches the semantics of object polymorphism: it only kicks in if the declared type is of type object but nothing happens if you specify a derived type.

Arguably, the issue lies with the implementation of SystemTextJsonOutputFormatter. Using the contract for the runtime type rather than that of the declared type can result in unintended serialization/disclosure of derived type data. I think it should be changed, if possible. cc @captainsafia.

i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

You might consider using a custom contract resolver that implements inherited polymorphism configuration.

[akurone]

thanks for the quick response Eirik.
i (over-pragmatically) thought changing a relatively new feature could be a bit more easy than something old enough to break a lot (and also has many friends like json object response formatter/producer/what ever that can break a lot on their own) and opened the issue here; also having all the properties of derived type in the json output is a more valid requirement than mine (which might not be possible before STJ supported polymorphic serialization) - so the justification seemed reasonable.

You might consider using a custom contract resolver that implements inherited polymorphism configuration.

i already implemented a custom resolver to solve my problem (repro code is the simplest form i could summarize it), it appends needed info to the derived type:

//if the type is a derived type of a base which defines its own polymorphic type info, 
//containing this type as a derived type then:
jsonTypeInfo.PolymorphismOptions = new() { TypeDiscriminatorPropertyName = TypeProperty };
jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(new JsonDerivedType(type, type.Name));
return jsonTypeInfo;

this tricks the serializer to think that the type (which is already a derived type of some base) is going to be derived (but in my case this is never going to happen; hence originally was sealed). so using this resolver effectively changes the sample in #63747 below:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

public class Derived : Base
{
    public int Y { get; set; }
}

to:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

//with no actual derived types and can not be sealed now
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Derived : Base
{
    public int Y { get; set; }
}

i agree with your point; both from design perspective and being practical: not having to declare same info twice is "more" ok for me 😊. but if the change (of STJOutputFormatter & friends) you mention is not going happen; having something (attribute is just to demo the idea, i will use resolver if this is implemented) like this will be ok:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

[IAmADerivedTypeAndINeedToOutputTypeDiscriminator(typeof(Derived), typeDiscriminatorId: "derived")]
public sealed class Derived : Base
{
    public int Y { get; set; }
}

[eiriktsarpalis]

Closing as answered.

[akurone]

hello Eirik,
my last comment (which i think you take as answer) just sums up the workaround and its limitations and states my agreement with your proposed solution(: resolver or attribute (i'd rather not -hence the need for something like IAmADerivedTypeAndINeedToOutputTypeDiscriminator- but) i have to change (unseal the derived type) my current domain class to be able to serialize this kind of polymorphic object with STJOutputFormatter; and (as you mentioned) a change in the formatter to properly call STJ serializer would be a better solution).

so i think closing the issue here means this will not be implemented:

...
[IAmADerivedTypeAndINeedToOutputTypeDiscriminator(typeof(Derived), typeDiscriminatorId: "derived")]
public sealed class Derived : Base
...

this issue is closed as answered (and the "answer" is just a summary of current situation) but the problem still exists and no comments from @captainsafia, i am a little confused how to proceed: shall i open an issue in aspnetcore repo or shall i stick with my half way workaround as the solution?

[eiriktsarpalis]

Hi, I closed the issue because, as mentioned, this is by-design behaviour, and we won't be changing it in future releases (it would constitute a breaking change).

-hence the need for something like IAmADerivedTypeAndINeedToOutputTypeDiscriminator-

I don't think a special attribute is necessary, it should be possible to write a custom resolver that enforces the semantics you desire. Obviously unsealing the derived type is necessary for this to work:

var options = new JsonSerializerOptions { TypeInfoResolver = new InheritedPolymorphismResolver() };
JsonSerializer.Serialize(new Derived(), options); // {"$type":"derived","Y":0,"X":0}

[JsonDerivedType(typeof(Base), typeDiscriminator: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminator: "derived")]
public class Base
{
    public int X { get; set; }
}

public class Derived : Base
{
    public int Y { get; set; }
}

public class InheritedPolymorphismResolver : DefaultJsonTypeInfoResolver
{
    public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
    {
        JsonTypeInfo typeInfo = base.GetTypeInfo(type, options);

        // Only handles class hierarchies -- interface hierarchies left out intentionally here
        if (!type.IsSealed && typeInfo.PolymorphismOptions is null && type.BaseType != null)
        {
            // recursively resolve metadata for the base type and extract any derived type declarations that overlap with the current type
            JsonPolymorphismOptions? basePolymorphismOptions = GetTypeInfo(type.BaseType, options).PolymorphismOptions;
            if (basePolymorphismOptions != null)
            {
                foreach (JsonDerivedType derivedType in basePolymorphismOptions.DerivedTypes)
                {
                    if (type.IsAssignableFrom(derivedType.DerivedType))
                    {
                        typeInfo.PolymorphismOptions ??= new()
                        {
                            IgnoreUnrecognizedTypeDiscriminators = basePolymorphismOptions.IgnoreUnrecognizedTypeDiscriminators,
                            TypeDiscriminatorPropertyName = basePolymorphismOptions.TypeDiscriminatorPropertyName,
                            UnknownDerivedTypeHandling = basePolymorphismOptions.UnknownDerivedTypeHandling,
                        };

                        typeInfo.PolymorphismOptions.DerivedTypes.Add(derivedType);
                    }
                }
            }
        }

        return typeInfo;
    }
}

[akurone]

thanks a lot Eirik, i submitted the issue to aspnetcore team. for reference to people having the same issue: along with the workaround described here -if you must seal the types and accept to take a little performance hit and can negotiate the return value with the receiving party- you can wrap the polymorphic type as a second workaround, see: #dotnet/aspnetcore/issues/44852