Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure reflection-based metadata instantiations unwrap TargetInvocationException. #72397

Merged
merged 2 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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