Ensure reflection-based metadata instantiations unwrap TargetInvocati…

…onException. (#72397) * Ensure reflection-based metadata instantiations unwrap TargetInvocationException. * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs Co-authored-by: Jan Kotas <[email protected]>
dotnet · Jul 19, 2022 · 0b90ae4 · 0b90ae4
1 parent c6f538e
commit 0b90ae4
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 26 deletions.
diff --git a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs
@@ -3,6 +3,7 @@
 
 using System.Reflection;
 using System.Runtime.CompilerServices;
+using System.Runtime.ExceptionServices;
 using System.Text.Json.Serialization;
 
 namespace System.Text.Json.Reflection
@@ -59,5 +60,27 @@ private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
             ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(typeof(TAttribute), memberInfo);
             return null;
         }
+
+        /// <summary>
+        /// Polyfill for BindingFlags.DoNotWrapExceptions
+        /// </summary>
+        public static object? InvokeNoWrapExceptions(this MethodInfo methodInfo, object? obj, object?[] parameters)
+        {
+#if NETCOREAPP
+            return methodInfo.Invoke(obj, BindingFlags.DoNotWrapExceptions, null, parameters, null);
+#else
+            object? result = null;
+            try
+            {
+                result = methodInfo.Invoke(obj, parameters);
+            }
+            catch (TargetInvocationException ex)
+            {
+                ExceptionDispatchInfo.Capture(ex.InnerException).Throw();
+            }
+
+            return result;
+#endif
+        }
     }
 }
diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs
@@ -4,7 +4,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
-using System.Runtime.ExceptionServices;
+using System.Text.Json.Reflection;
 using System.Threading;
 
 namespace System.Text.Json.Serialization.Metadata
@@ -36,6 +36,10 @@ private DefaultJsonTypeInfoResolver(bool mutable)
         }
 
         /// <inheritdoc/>
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
+            Justification = "The ctor is marked RequiresUnreferencedCode.")]
+        [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode",
+            Justification = "The ctor is marked RequiresDynamicCode.")]
         public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
         {
             if (type == null)
@@ -64,28 +68,13 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options
             return typeInfo;
         }
 
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
-            Justification = "The ctor is marked RequiresUnreferencedCode.")]
-        [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode",
-            Justification = "The ctor is marked RequiresDynamicCode.")]
-        private JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options)
+        [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
+        [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
+        private static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options)
         {
-            MethodInfo methodInfo = typeof(DefaultJsonTypeInfoResolver).GetMethod(nameof(CreateReflectionJsonTypeInfo), BindingFlags.NonPublic | BindingFlags.Static)!;
-#if NETCOREAPP
-            return (JsonTypeInfo)methodInfo.MakeGenericMethod(type).Invoke(null, BindingFlags.NonPublic | BindingFlags.DoNotWrapExceptions, null, new[] { options }, null)!;
-#else
-            try
-            {
-                return (JsonTypeInfo)methodInfo.MakeGenericMethod(type).Invoke(null, new[] { options })!;
-            }
-            catch (TargetInvocationException ex)
-            {
-                // Some of the validation is done during construction (i.e. validity of JsonConverter, inner types etc.)
-                // therefore we need to unwrap TargetInvocationException for better user experience
-                ExceptionDispatchInfo.Capture(ex.InnerException).Throw();
-                throw null!;
-            }
-#endif
+            s_createReflectionJsonTypeInfoMethodInfo ??= typeof(DefaultJsonTypeInfoResolver).GetMethod(nameof(CreateReflectionJsonTypeInfo), BindingFlags.NonPublic | BindingFlags.Static)!;
+            return (JsonTypeInfo)s_createReflectionJsonTypeInfoMethodInfo.MakeGenericMethod(type)
+                .InvokeNoWrapExceptions(null, new object[] { options })!;
         }
 
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
@@ -96,6 +85,8 @@ private static JsonTypeInfo<T> CreateReflectionJsonTypeInfo<T>(JsonSerializerOpt
             return new ReflectionJsonTypeInfo<T>(converter, options);
         }
 
+        private static MethodInfo? s_createReflectionJsonTypeInfoMethodInfo;
+
         /// <summary>
         /// List of JsonTypeInfo modifiers. Modifying callbacks are called consecutively after initial resolution
         /// and cannot be changed after GetTypeInfo is called.

diff --git a/...raries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/...raries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
@@ -63,15 +63,15 @@ internal JsonPropertyInfo CreatePropertyUsingReflection(Type propertyType)
                 // If a JsonTypeInfo has already been cached for the property type,
                 // avoid reflection-based initialization by delegating construction
                 // of JsonPropertyInfo<T> construction to the property type metadata.
-                return jsonTypeInfo.CreateJsonPropertyInfo(declaringTypeInfo: this, Options);
+                jsonPropertyInfo = jsonTypeInfo.CreateJsonPropertyInfo(declaringTypeInfo: this, Options);
             }
             else
             {
                 // Metadata for `propertyType` has not been registered yet.
                 // Use reflection to instantiate the correct JsonPropertyInfo<T>
                 s_createJsonPropertyInfo ??= typeof(JsonTypeInfo).GetMethod(nameof(CreateJsonPropertyInfo), BindingFlags.NonPublic | BindingFlags.Static)!;
-                MethodInfo factoryInfo = s_createJsonPropertyInfo.MakeGenericMethod(propertyType);
-                jsonPropertyInfo = (JsonPropertyInfo)factoryInfo.Invoke(null, new object[] { this, Options })!;
+                jsonPropertyInfo = (JsonPropertyInfo)s_createJsonPropertyInfo.MakeGenericMethod(propertyType)
+                    .InvokeNoWrapExceptions(null, new object[] { this, Options })!;
             }
 
             Debug.Assert(jsonPropertyInfo.PropertyType == propertyType);

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
@@ -7,6 +7,7 @@
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.ExceptionServices;
+using System.Text.Json.Reflection;
 using System.Threading;
 
 namespace System.Text.Json.Serialization.Metadata
@@ -578,7 +579,7 @@ public static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions o
 
             s_createJsonTypeInfo ??= typeof(JsonTypeInfo).GetMethod(nameof(CreateJsonTypeInfo), new Type[] { typeof(JsonSerializerOptions) })!;
             return (JsonTypeInfo)s_createJsonTypeInfo.MakeGenericMethod(type)
-                .Invoke(null, new object[] { options })!;
+                .InvokeNoWrapExceptions(null, new object[] { options })!;
         }
 
         /// <summary>

diff --git a/...t.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs b/...t.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs
@@ -673,6 +673,23 @@ ref struct RefStruct
             public int Foo { get; set; }
         }
 
+        [Fact]
+        public static void CreateJsonTypeInfo_ThrowingConverterFactory_DoesNotWrapException()
+        {
+            var options = new JsonSerializerOptions { Converters = { new ClassWithThrowingConverterFactory.Converter() } };
+            // Should not be wrapped in TargetInvocationException.
+            Assert.Throws<NotFiniteNumberException>(() => JsonTypeInfo.CreateJsonTypeInfo(typeof(ClassWithThrowingConverterFactory), options));
+        }
+
+        public class ClassWithThrowingConverterFactory
+        {
+            public class Converter : JsonConverterFactory
+            {
+                public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(ClassWithThrowingConverterFactory);
+                public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) => throw new NotFiniteNumberException();
+            }
+        }
+
         [Fact]
         public static void CreateJsonPropertyInfoWithNullArgumentsThrows()
         {