Throw when reference cannot be assigned to converter.TypeToConvert (#39015)

* Throw when reference cannot be assigned to converter.TypeToConvert

* Throw InvalidOperationException instead of JsonException since the cause for this error would be an error in the user code rather than in the JSON payload

* Use try-catch instead of IsAssignableFrom

* Alternatively validate reference can be assigned as T at the time is fetch

* Add suggested comment

diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
index d6ae6ad..f6b8261 100644 (file)
--- a/src/libraries/System.Text.Json/src/Resources/Strings.resx
+++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -545,4 +545,7 @@
   <data name="ConverterCanConvertNullableRedundant" xml:space="preserve">
     <value>The converter '{0}' handles type '{1}' but is being asked to convert type '{2}'. Either create a separate converter for type '{2}' or change the converter's 'CanConvert' method to only return 'true' for a single type.</value>
   </data>
-</root>
\ No newline at end of file
+  <data name="MetadataReferenceOfTypeCannotBeAssignedToType" xml:space="preserve">
+    <value>The object with reference id '{0}' of type '{1}' cannot be assigned to the type '{2}'.</value>
+  </data>
+</root>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
index 2093cd6..d077ac1 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
@@ -141,10 +141,11 @@ namespace System.Text.Json.Serialization.Converters
                 bool preserveReferences = options.ReferenceHandler != null;
                 if (preserveReferences && state.Current.ObjectState < StackFrameObjectState.PropertyValue)
                 {
-                    if (JsonSerializer.ResolveMetadataForJsonObject(ref reader, ref state, options))
+                    if (JsonSerializer.ResolveMetadataForJsonObject<TCollection>(ref reader, ref state, options))
                     {
                         if (state.Current.ObjectState == StackFrameObjectState.ReadRefEndObject)
                         {
+                            // This will never throw since it was previously validated in ResolveMetadataForJsonObject.
                             value = (TCollection)state.Current.ReturnValue!;
                             return true;
                         }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
index 3f05795..1848f5b 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
@@ -115,10 +115,11 @@ namespace System.Text.Json.Serialization.Converters
                 // Handle the metadata properties.
                 if (preserveReferences && state.Current.ObjectState < StackFrameObjectState.PropertyValue)
                 {
-                    if (JsonSerializer.ResolveMetadataForJsonArray(ref reader, ref state, options))
+                    if (JsonSerializer.ResolveMetadataForJsonArray<TCollection>(ref reader, ref state, options))
                     {
                         if (state.Current.ObjectState == StackFrameObjectState.ReadRefEndObject)
                         {
+                            // This will never throw since it was previously validated in ResolveMetadataForJsonArray.
                             value = (TCollection)state.Current.ReturnValue!;
                             return true;
                         }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
index 237bb83..9dde207 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
@@ -77,10 +77,11 @@ namespace System.Text.Json.Serialization.Converters
                 {
                     if (options.ReferenceHandler != null)
                     {
-                        if (JsonSerializer.ResolveMetadataForJsonObject(ref reader, ref state, options))
+                        if (JsonSerializer.ResolveMetadataForJsonObject<T>(ref reader, ref state, options))
                         {
                             if (state.Current.ObjectState == StackFrameObjectState.ReadRefEndObject)
                             {
+                                // This will never throw since it was previously validated in ResolveMetadataForJsonObject.
                                 value = (T)state.Current.ReturnValue!;
                                 return true;
                             }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs
index 2fd7942..29ce3e3 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs
@@ -23,7 +23,7 @@ namespace System.Text.Json
         /// Sets state.Current.ReturnValue to the reference target for $ref cases;
         /// Sets state.Current.ReturnValue to a new instance for $id cases.
         /// </summary>
-        internal static bool ResolveMetadataForJsonObject(
+        internal static bool ResolveMetadataForJsonObject<T>(
             ref Utf8JsonReader reader,
             ref ReadStack state,
             JsonSerializerOptions options)
@@ -112,9 +112,9 @@ namespace System.Text.Json
                 }
 
                 string referenceId = reader.GetString()!;
-
-                // todo: https://github.com/dotnet/runtime/issues/32354
-                state.Current.ReturnValue = state.ReferenceResolver.ResolveReference(referenceId);
+                object value = state.ReferenceResolver.ResolveReference(referenceId);
+                ValidateValueIsCorrectType<T>(value, referenceId);
+                state.Current.ReturnValue = value;
 
                 state.Current.ObjectState = StackFrameObjectState.ReadAheadRefEndObject;
             }
@@ -164,7 +164,7 @@ namespace System.Text.Json
         /// Sets state.Current.ReturnValue to the reference target for $ref cases;
         /// Sets state.Current.ReturnValue to a new instance for $id cases.
         /// </summary>
-        internal static bool ResolveMetadataForJsonArray(
+        internal static bool ResolveMetadataForJsonArray<T>(
             ref Utf8JsonReader reader,
             ref ReadStack state,
             JsonSerializerOptions options)
@@ -191,7 +191,6 @@ namespace System.Text.Json
                     ThrowHelper.ThrowJsonException_MetadataPreservedArrayValuesNotFound(ref state, converter.TypeToConvert);
                 }
 
-
                 ReadOnlySpan<byte> propertyName = reader.GetSpan();
                 MetadataPropertyName metadata = GetMetadataPropertyName(propertyName);
                 if (metadata == MetadataPropertyName.Id)
@@ -251,10 +250,11 @@ namespace System.Text.Json
                     ThrowHelper.ThrowJsonException_MetadataValueWasNotString(reader.TokenType);
                 }
 
-                string key = reader.GetString()!;
+                string referenceId = reader.GetString()!;
+                object value = state.ReferenceResolver.ResolveReference(referenceId);
+                ValidateValueIsCorrectType<T>(value, referenceId);
+                state.Current.ReturnValue = value;
 
-                // todo: https://github.com/dotnet/runtime/issues/32354
-                state.Current.ReturnValue = state.ReferenceResolver.ResolveReference(key);
                 state.Current.ObjectState = StackFrameObjectState.ReadAheadRefEndObject;
             }
             else if (state.Current.ObjectState == StackFrameObjectState.ReadIdValue)
@@ -435,5 +435,20 @@ namespace System.Text.Json
 
             return refMetadataFound;
         }
+
+        private static void ValidateValueIsCorrectType<T>(object value, string referenceId)
+        {
+            try
+            {
+                // No need to worry about unboxing here since T will always be a reference type at this point.
+                T _ = (T)value;
+            }
+            catch (InvalidCastException)
+            {
+                ThrowHelper.ThrowInvalidOperationException_MetadataReferenceOfTypeCannotBeAssignedToType(
+                    referenceId, value.GetType(), typeof(T));
+                throw;
+            }
+        }
     }
 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
index e82f44b..bb0e156 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
@@ -605,6 +605,13 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public static void ThrowInvalidOperationException_MetadataReferenceOfTypeCannotBeAssignedToType(string referenceId, Type currentType, Type typeToConvert)
+        {
+            throw new InvalidOperationException(SR.Format(SR.MetadataReferenceOfTypeCannotBeAssignedToType, referenceId, currentType, typeToConvert));
+        }
+
+        [DoesNotReturn]
         internal static void ThrowUnexpectedMetadataException(
             ReadOnlySpan<byte> propertyName,
             ref Utf8JsonReader reader,

diff --git a/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.Deserialize.cs b/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.Deserialize.cs
index ddf2931..18221ff 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.Deserialize.cs
+++ b/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.Deserialize.cs
@@ -1543,5 +1543,34 @@ namespace System.Text.Json.Serialization.Tests
         }
         #endregion
         #endregion
+
+        [Fact]
+        public static void ReferenceIsAssignableFrom()
+        {
+            const string json = @"{""Derived"":{""$id"":""my_id_1""},""Base"":{""$ref"":""my_id_1""}}";
+            BaseAndDerivedWrapper root = JsonSerializer.Deserialize<BaseAndDerivedWrapper>(json, s_serializerOptionsPreserve);
+
+            Assert.Same(root.Base, root.Derived);
+        }
+
+        [Fact]
+        public static void ReferenceIsNotAssignableFrom()
+        {
+            const string json = @"{""Base"":{""$id"":""my_id_1""},""Derived"":{""$ref"":""my_id_1""}}";
+            InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<BaseAndDerivedWrapper>(json, s_serializerOptionsPreserve));
+
+            Assert.Contains("my_id_1", ex.Message);
+            Assert.Contains(typeof(Derived).ToString(), ex.Message);
+            Assert.Contains(typeof(Base).ToString(), ex.Message);
+        }
+
+        private class BaseAndDerivedWrapper
+        {
+            public Base Base { get; set; }
+            public Derived Derived { get; set; }
+        }
+
+        private class Derived : Base { }
+        private class Base { }
     }
 }