Skip to content

Commit

Permalink
Ensure reflection-based metadata instantiations unwrap TargetInvocati…
Browse files Browse the repository at this point in the history
…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 <jkotas@microsoft.com>
  • Loading branch information
eiriktsarpalis and jkotas authored Jul 19, 2022
1 parent c6f538e commit 0b90ae4
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 26 deletions.
23 changes: 23 additions & 0 deletions src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)]
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down

0 comments on commit 0b90ae4

Please sign in to comment.