Fix InvalidCastException when casting a polymorphic converter (#73259)

* Fix InvalidCastException when casting a polymorphic converter.

* Extend test coverage and fixes for the case of deserialization.

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs

@@ -1,6 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
+using System.Text.Json.Reflection;
+
 namespace System.Text.Json.Serialization.Converters
 {
     /// <summary>
@@ -18,6 +21,9 @@ internal sealed class CastingConverter<T, TSource> : JsonConverter<T>
 
         internal CastingConverter(JsonConverter<TSource> sourceConverter) : base(initialize: false)
         {
+            Debug.Assert(typeof(T).IsInSubtypeRelationshipWith(typeof(TSource)));
+            Debug.Assert(sourceConverter.SourceConverterForCastingConverter is null, "casting converters should not be layered.");
+
             _sourceConverter = sourceConverter;
             Initialize();
 
@@ -28,6 +34,8 @@ internal CastingConverter(JsonConverter<TSource> sourceConverter) : base(initial
             CanBePolymorphic = sourceConverter.CanBePolymorphic;
         }
 
+        internal override JsonConverter? SourceConverterForCastingConverter => _sourceConverter;
+
         public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
             => CastOnRead(_sourceConverter.Read(ref reader, typeToConvert, options));
 

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs

@@ -49,18 +49,13 @@ protected virtual void ConvertCollection(ref ReadStack state, JsonSerializerOpti
 
         protected static JsonConverter<TElement> GetElementConverter(JsonTypeInfo elementTypeInfo)
         {
-            JsonConverter<TElement> converter = (JsonConverter<TElement>)elementTypeInfo.Converter;
-            Debug.Assert(converter != null); // It should not be possible to have a null converter at this point.
-
-            return converter;
+            return ((JsonTypeInfo<TElement>)elementTypeInfo).EffectiveConverter;
         }
 
         protected static JsonConverter<TElement> GetElementConverter(ref WriteStack state)
         {
-            JsonConverter<TElement> converter = (JsonConverter<TElement>)state.Current.JsonPropertyInfo!.EffectiveConverter;
-            Debug.Assert(converter != null); // It should not be possible to have a null converter at this point.
-
-            return converter;
+            Debug.Assert(state.Current.JsonPropertyInfo != null);
+            return (JsonConverter<TElement>)state.Current.JsonPropertyInfo.EffectiveConverter;
         }
 
         internal override bool OnTryRead(

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs

@@ -70,10 +70,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
 
         protected static JsonConverter<T> GetConverter<T>(JsonTypeInfo typeInfo)
         {
-            JsonConverter<T> converter = (JsonConverter<T>)typeInfo.Converter;
-            Debug.Assert(converter != null); // It should not be possible to have a null converter at this point.
-
-            return converter;
+            return ((JsonTypeInfo<T>)typeInfo).EffectiveConverter;
         }
 
         internal sealed override bool OnTryRead(

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

@@ -16,26 +16,24 @@ public abstract partial class JsonConverter<T> : JsonConverter
         /// <summary>
         /// When overridden, constructs a new <see cref="JsonConverter{T}"/> instance.
         /// </summary>
-        protected internal JsonConverter()
-        {
-            Initialize();
-        }
+        protected internal JsonConverter() : this(initialize: true)
+        { }
 
         internal JsonConverter(bool initialize)
         {
+            IsValueType = typeof(T).IsValueType;
+            IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly;
+
             // Initialize uses abstract members, in order for them to be initialized correctly
-            // without throwing we need to delay call to Initialize
+            // without throwing we might need to delay call to Initialize
             if (initialize)
             {
                 Initialize();
             }
         }
 
-        internal void Initialize()
+        private protected void Initialize()
         {
-            IsValueType = typeof(T).IsValueType;
-            IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly;
-
             if (HandleNull)
             {
                 HandleNullOnRead = true;
@@ -76,10 +74,24 @@ internal sealed override JsonParameterInfo CreateJsonParameterInfo()
 
         internal sealed override JsonConverter<TTarget> CreateCastingConverter<TTarget>()
         {
+            if (this is JsonConverter<TTarget> conv)
+            {
+                return conv;
+            }
+
             JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(this, typeof(TTarget));
-            return new CastingConverter<TTarget, T>(this);
+
+            // Avoid layering casting converters by consulting any source converters directly.
+            return
+                SourceConverterForCastingConverter?.CreateCastingConverter<TTarget>()
+                ?? new CastingConverter<TTarget, T>(this);
         }
 
+        /// <summary>
+        /// Set if this converter is itself a casting converter.
+        /// </summary>
+        internal virtual JsonConverter? SourceConverterForCastingConverter => null;
+
         internal override Type? KeyType => null;
 
         internal override Type? ElementType => null;

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

@@ -25,7 +25,16 @@ public abstract class JsonPropertyInfo
         /// <summary>
         /// Converter after applying CustomConverter (i.e. JsonConverterAttribute)
         /// </summary>
-        internal abstract JsonConverter EffectiveConverter { get; }
+        internal JsonConverter EffectiveConverter
+        {
+            get
+            {
+                Debug.Assert(_effectiveConverter != null);
+                return _effectiveConverter;
+            }
+        }
+
+        private protected JsonConverter? _effectiveConverter;
 
         /// <summary>
         /// Gets or sets a custom converter override for the current property.
@@ -705,16 +714,15 @@ internal bool ReadJsonAndAddExtensionProperty(
                 }
                 else
                 {
-                    JsonConverter<object> converter = (JsonConverter<object>)GetDictionaryValueConverter(JsonTypeInfo.ObjectType);
+                    JsonConverter<object> converter = GetDictionaryValueConverter<object>();
                     object value = converter.Read(ref reader, JsonTypeInfo.ObjectType, Options)!;
                     dictionaryObjectValue[state.Current.JsonPropertyNameAsString!] = value;
                 }
             }
             else if (propValue is IDictionary<string, JsonElement> dictionaryElementValue)
             {
-                Type elementType = typeof(JsonElement);
-                JsonConverter<JsonElement> converter = (JsonConverter<JsonElement>)GetDictionaryValueConverter(elementType);
-                JsonElement value = converter.Read(ref reader, elementType, Options);
+                JsonConverter<JsonElement> converter = GetDictionaryValueConverter<JsonElement>();
+                JsonElement value = converter.Read(ref reader, typeof(JsonElement), Options);
                 dictionaryElementValue[state.Current.JsonPropertyNameAsString!] = value;
             }
             else
@@ -726,25 +734,17 @@ internal bool ReadJsonAndAddExtensionProperty(
 
             return true;
 
-            JsonConverter GetDictionaryValueConverter(Type dictionaryValueType)
+            JsonConverter<TValue> GetDictionaryValueConverter<TValue>()
             {
-                JsonConverter converter;
-                JsonTypeInfo? dictionaryValueInfo = JsonTypeInfo.ElementTypeInfo;
-                if (dictionaryValueInfo != null)
-                {
-                    // Fast path when there is a generic type such as Dictionary<,>.
-                    converter = dictionaryValueInfo.Converter;
-                }
-                else
-                {
+                JsonTypeInfo dictionaryValueInfo =
+                    JsonTypeInfo.ElementTypeInfo
                     // Slower path for non-generic types that implement IDictionary<,>.
                     // It is possible to cache this converter on JsonTypeInfo if we assume the property value
                     // will always be the same type for all instances.
-                    converter = Options.GetConverterInternal(dictionaryValueType);
-                }
+                    ?? Options.GetTypeInfoInternal(typeof(TValue));
 
-                Debug.Assert(converter != null);
-                return converter;
+                Debug.Assert(dictionaryValueInfo is JsonTypeInfo<TValue>);
+                return ((JsonTypeInfo<TValue>)dictionaryValueInfo).EffectiveConverter;
             }
         }
 

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs

@@ -114,7 +114,16 @@ private protected override void SetShouldSerialize(Delegate? predicate)
         internal override object? DefaultValue => default(T);
         internal override bool PropertyTypeCanBeNull => default(T) is null;
 
-        internal JsonConverter<T> TypedEffectiveConverter { get; private set; } = null!;
+        internal new JsonConverter<T> EffectiveConverter
+        {
+            get
+            {
+                Debug.Assert(_typedEffectiveConverter != null);
+                return _typedEffectiveConverter;
+            }
+        }
+
+        private JsonConverter<T>? _typedEffectiveConverter;
 
         private protected override void DetermineMemberAccessors(MemberInfo memberInfo)
         {
@@ -203,15 +212,17 @@ internal JsonPropertyInfo(JsonPropertyInfoValues<T> propertyInfo, JsonSerializer
 
         private protected override void DetermineEffectiveConverter(JsonTypeInfo jsonTypeInfo)
         {
-            JsonConverter converter =
-                Options.ExpandConverterFactory(CustomConverter, PropertyType)
-                ?? jsonTypeInfo.Converter;
+            Debug.Assert(jsonTypeInfo is JsonTypeInfo<T>);
 
-            TypedEffectiveConverter = converter is JsonConverter<T> typedConv ? typedConv : converter.CreateCastingConverter<T>();
-            ConverterStrategy = TypedEffectiveConverter.ConverterStrategy;
-        }
+            JsonConverter<T> converter =
+                Options.ExpandConverterFactory(CustomConverter, PropertyType) // Expand any property-level custom converters.
+                ?.CreateCastingConverter<T>()                                 // Cast to JsonConverter<T>, potentially with wrapping.
+                ?? ((JsonTypeInfo<T>)jsonTypeInfo).EffectiveConverter;        // Fall back to the effective converter for the type.
 
-        internal override JsonConverter EffectiveConverter => TypedEffectiveConverter;
+            _effectiveConverter = converter;
+            _typedEffectiveConverter = converter;
+            ConverterStrategy = converter.ConverterStrategy;
+        }
 
         internal override object? GetValueAsObject(object obj)
         {
@@ -232,7 +243,7 @@ internal override bool GetMemberAndWriteJson(object obj, ref WriteStack state, U
 #if NETCOREAPP
                 !typeof(T).IsValueType && // treated as a constant by recent versions of the JIT.
 #else
-                !TypedEffectiveConverter.IsValueType &&
+                !EffectiveConverter.IsValueType &&
 #endif
                 Options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles &&
                 value is not null &&
@@ -265,7 +276,7 @@ value is not null &&
             {
                 Debug.Assert(PropertyTypeCanBeNull);
 
-                if (TypedEffectiveConverter.HandleNullOnWrite)
+                if (EffectiveConverter.HandleNullOnWrite)
                 {
                     if (state.Current.PropertyState < StackFramePropertyState.Name)
                     {
@@ -274,10 +285,10 @@ value is not null &&
                     }
 
                     int originalDepth = writer.CurrentDepth;
-                    TypedEffectiveConverter.Write(writer, value, Options);
+                    EffectiveConverter.Write(writer, value, Options);
                     if (originalDepth != writer.CurrentDepth)
                     {
-                        ThrowHelper.ThrowJsonException_SerializationConverterWrite(TypedEffectiveConverter);
+                        ThrowHelper.ThrowJsonException_SerializationConverterWrite(EffectiveConverter);
                     }
                 }
                 else
@@ -295,7 +306,7 @@ value is not null &&
                     writer.WritePropertyNameSection(EscapedNameSection);
                 }
 
-                return TypedEffectiveConverter.TryWrite(writer, value, Options, ref state);
+                return EffectiveConverter.TryWrite(writer, value, Options, ref state);
             }
         }
 
@@ -317,7 +328,7 @@ internal override bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteS
             }
             else
             {
-                success = TypedEffectiveConverter.TryWriteDataExtensionProperty(writer, value, Options, ref state);
+                success = EffectiveConverter.TryWriteDataExtensionProperty(writer, value, Options, ref state);
             }
 
             return success;
@@ -328,11 +339,11 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
             bool success;
 
             bool isNullToken = reader.TokenType == JsonTokenType.Null;
-            if (isNullToken && !TypedEffectiveConverter.HandleNullOnRead && !state.IsContinuation)
+            if (isNullToken && !EffectiveConverter.HandleNullOnRead && !state.IsContinuation)
             {
                 if (default(T) is not null)
                 {
-                    ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypedEffectiveConverter.TypeToConvert);
+                    ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(EffectiveConverter.TypeToConvert);
                 }
 
                 if (!IgnoreNullTokensOnRead)
@@ -344,15 +355,15 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
                 success = true;
                 state.Current.MarkRequiredPropertyAsRead(this);
             }
-            else if (TypedEffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
+            else if (EffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
             {
                 // CanUseDirectReadOrWrite == false when using streams
                 Debug.Assert(!state.IsContinuation);
 
                 if (!isNullToken || !IgnoreNullTokensOnRead || default(T) is not null)
                 {
                     // Optimize for internal converters by avoiding the extra call to TryRead.
-                    T? fastValue = TypedEffectiveConverter.Read(ref reader, PropertyType, Options);
+                    T? fastValue = EffectiveConverter.Read(ref reader, PropertyType, Options);
                     Set!(obj, fastValue!);
                 }
 
@@ -364,7 +375,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
                 success = true;
                 if (!isNullToken || !IgnoreNullTokensOnRead || default(T) is not null || state.IsContinuation)
                 {
-                    success = TypedEffectiveConverter.TryRead(ref reader, PropertyType, Options, ref state, out T? value);
+                    success = EffectiveConverter.TryRead(ref reader, PropertyType, Options, ref state, out T? value);
                     if (success)
                     {
                         Set!(obj, value!);
@@ -380,11 +391,11 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
         {
             bool success;
             bool isNullToken = reader.TokenType == JsonTokenType.Null;
-            if (isNullToken && !TypedEffectiveConverter.HandleNullOnRead && !state.IsContinuation)
+            if (isNullToken && !EffectiveConverter.HandleNullOnRead && !state.IsContinuation)
             {
                 if (default(T) is not null)
                 {
-                    ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypedEffectiveConverter.TypeToConvert);
+                    ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(EffectiveConverter.TypeToConvert);
                 }
 
                 value = default(T);
@@ -393,17 +404,17 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
             else
             {
                 // Optimize for internal converters by avoiding the extra call to TryRead.
-                if (TypedEffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
+                if (EffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
                 {
                     // CanUseDirectReadOrWrite == false when using streams
                     Debug.Assert(!state.IsContinuation);
 
-                    value = TypedEffectiveConverter.Read(ref reader, PropertyType, Options);
+                    value = EffectiveConverter.Read(ref reader, PropertyType, Options);
                     success = true;
                 }
                 else
                 {
-                    success = TypedEffectiveConverter.TryRead(ref reader, PropertyType, Options, ref state, out T? typedValue);
+                    success = EffectiveConverter.TryRead(ref reader, PropertyType, Options, ref state, out T? typedValue);
                     value = typedValue;
                 }
             }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

@@ -16,6 +16,11 @@ public abstract class JsonTypeInfo<T> : JsonTypeInfo
 
         private Func<T>? _typedCreateObject;
 
+        /// <summary>
+        /// A Converter whose declared type always matches that of the current JsonTypeInfo.
+        /// It might be the same instance as JsonTypeInfo.Converter or it could be wrapped
+        /// in a CastingConverter in cases where a polymorphic converter is being used.
+        /// </summary>
         internal JsonConverter<T> EffectiveConverter { get; }
 
         /// <summary>
@@ -84,7 +89,7 @@ private protected override void SetCreateObject(Delegate? createObject)
         internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
             : base(typeof(T), converter, options)
         {
-            EffectiveConverter = converter is JsonConverter<T> jsonConverter ? jsonConverter : converter.CreateCastingConverter<T>();
+            EffectiveConverter = converter.CreateCastingConverter<T>();
         }
 
         /// <summary>