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

Extend detection for unsupported types #73241

Merged
merged 2 commits into from
Aug 3, 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
14 changes: 8 additions & 6 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private sealed class Parser

// Unsupported types
private readonly Type _delegateType;
private readonly Type _typeType;
private readonly Type _memberInfoType;
private readonly Type _serializationInfoType;
private readonly Type _intPtrType;
private readonly Type _uIntPtrType;
Expand Down Expand Up @@ -232,15 +232,15 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
_jsonObjectType = _metadataLoadContext.Resolve(JsonObjectFullName);
_jsonValueType = _metadataLoadContext.Resolve(JsonValueFullName);
_jsonDocumentType = _metadataLoadContext.Resolve(JsonDocumentFullName);
_dateOnlyType = _metadataLoadContext.Resolve(DateOnlyFullName);
_timeOnlyType = _metadataLoadContext.Resolve(TimeOnlyFullName);

// Unsupported types.
_delegateType = _metadataLoadContext.Resolve(SpecialType.System_Delegate);
_typeType = _metadataLoadContext.Resolve(typeof(Type));
_memberInfoType = _metadataLoadContext.Resolve(typeof(MemberInfo));
_serializationInfoType = _metadataLoadContext.Resolve(typeof(Runtime.Serialization.SerializationInfo));
_intPtrType = _metadataLoadContext.Resolve(typeof(IntPtr));
_uIntPtrType = _metadataLoadContext.Resolve(typeof(UIntPtr));
_dateOnlyType = _metadataLoadContext.Resolve(DateOnlyFullName);
_timeOnlyType = _metadataLoadContext.Resolve(TimeOnlyFullName);

_jsonConverterOfTType = _metadataLoadContext.Resolve(JsonConverterOfTFullName);

Expand Down Expand Up @@ -960,7 +960,10 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
}
}
}
else if (_knownUnsupportedTypes.Contains(type) || _delegateType.IsAssignableFrom(type))
else if (
_knownUnsupportedTypes.Contains(type) ||
_memberInfoType.IsAssignableFrom(type) ||
_delegateType.IsAssignableFrom(type))
{
classType = ClassType.KnownUnsupportedType;
}
Expand Down Expand Up @@ -1590,7 +1593,6 @@ private void PopulateKnownTypes()
AddTypeIfNotNull(_knownTypes, _jsonValueType);
AddTypeIfNotNull(_knownTypes, _jsonDocumentType);

_knownUnsupportedTypes.Add(_typeType);
_knownUnsupportedTypes.Add(_serializationInfoType);
_knownUnsupportedTypes.Add(_intPtrType);
_knownUnsupportedTypes.Add(_uIntPtrType);
Expand Down
16 changes: 5 additions & 11 deletions src/libraries/System.Text.Json/gen/Reflection/TypeWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -595,17 +595,11 @@ protected override bool IsPrimitiveImpl()

public override bool IsAssignableFrom(Type c)
{
if (c is TypeWrapper tr)
{
return tr._typeSymbol.AllInterfaces.Contains(_typeSymbol, SymbolEqualityComparer.Default) ||
(tr._namedTypeSymbol != null && tr._namedTypeSymbol.BaseTypes().Contains(_typeSymbol, SymbolEqualityComparer.Default));
}
else if (_metadataLoadContext.Resolve(c) is TypeWrapper trr)
{
return trr._typeSymbol.AllInterfaces.Contains(_typeSymbol, SymbolEqualityComparer.Default) ||
(trr._namedTypeSymbol != null && trr._namedTypeSymbol.BaseTypes().Contains(_typeSymbol, SymbolEqualityComparer.Default));
}
return false;
TypeWrapper? tr = c as TypeWrapper ?? _metadataLoadContext.Resolve(c) as TypeWrapper;

return tr is not null &&
(tr._typeSymbol.AllInterfaces.Contains(_typeSymbol, SymbolEqualityComparer.Default) ||
(tr._namedTypeSymbol != null && tr._namedTypeSymbol.BaseTypes().Contains(_typeSymbol, SymbolEqualityComparer.Default)));
}

#pragma warning disable RS1024 // Compare symbols correctly
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,9 @@
<data name="CannotPopulateCollection" xml:space="preserve">
<value>The collection type '{0}' is abstract, an interface, or is read only, and could not be instantiated and populated.</value>
</data>
<data name="ConstructorContainsNullParameterNames" xml:space="preserve">
<value>The deserialization constructor for type '{0}' contains parameters with null names. This might happen because the parameter names have been trimmed by the linker. Consider using the source generated serializer instead.</value>
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="DefaultIgnoreConditionAlreadySpecified" xml:space="preserve">
<value>'IgnoreNullValues' and 'DefaultIgnoreCondition' cannot both be set to non-default values.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ public override bool CanConvert(Type type)
// If a type is added, also add to the SourceGeneration project.

return
// There's no safe way to construct a Type from untrusted user input.
typeof(Type).IsAssignableFrom(type) ||
// There's no safe way to construct a Type/MemberInfo from untrusted user input.
typeof(MemberInfo).IsAssignableFrom(type) ||
// (De)serialization of SerializationInfo is already disallowed due to Type being disallowed
// (the two ctors on SerializationInfo take a Type, and a Type member is present when serializing).
// Explicitly disallowing this type provides a clear exception when ctors with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,22 +230,25 @@ private static bool PropertyIsOverriddenAndIgnored(

internal override JsonParameterInfoValues[] GetParameterInfoValues()
{
ParameterInfo[] parameters = Converter.ConstructorInfo!.GetParameters();
return GetParameterInfoArray(parameters);
}

private static JsonParameterInfoValues[] GetParameterInfoArray(ParameterInfo[] parameters)
{
Debug.Assert(Converter.ConstructorInfo != null);
ParameterInfo[] parameters = Converter.ConstructorInfo.GetParameters();
int parameterCount = parameters.Length;
JsonParameterInfoValues[] jsonParameters = new JsonParameterInfoValues[parameterCount];

for (int i = 0; i < parameterCount; i++)
{
ParameterInfo reflectionInfo = parameters[i];

// Trimmed parameter names are reported as null in CoreCLR or "" in Mono.
if (string.IsNullOrEmpty(reflectionInfo.Name))
{
Debug.Assert(Converter.ConstructorInfo.DeclaringType != null);
ThrowHelper.ThrowNotSupportedException_BaseConverterDoesNotSupportMetadata(Converter.ConstructorInfo.DeclaringType);
}

JsonParameterInfoValues jsonInfo = new()
{
Name = reflectionInfo.Name!,
Name = reflectionInfo.Name,
ParameterType = reflectionInfo.ParameterType,
Position = reflectionInfo.Position,
HasDefaultValue = reflectionInfo.HasDefaultValue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,13 @@ public static void ThrowNotSupportedException_NoMetadataForType(Type type, IJson
throw new NotSupportedException(SR.Format(SR.NoMetadataForType, type, resolver?.GetType().FullName ?? "<null>"));
}


eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
[DoesNotReturn]
public static void ThrowNotSupportedException_ConstructorContainsNullParameterNames(Type declaringType)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
throw new NotSupportedException(SR.Format(SR.ConstructorContainsNullParameterNames, declaringType));
}

[DoesNotReturn]
public static void ThrowInvalidOperationException_NoMetadataForType(Type type, IJsonTypeInfoResolver? resolver)
{
Expand Down
195 changes: 109 additions & 86 deletions src/libraries/System.Text.Json/tests/Common/UnsupportedTypesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Reflection;
using System.Runtime.Serialization;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -20,110 +21,117 @@ public UnsupportedTypesTests(
SupportsJsonPathOnSerialize = supportsJsonPathOnSerialize;
}

[Fact]
public async Task DeserializeUnsupportedType()
[Theory]
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
[MemberData(nameof(GetUnsupportedValues))]
public async Task DeserializeUnsupportedType<T>(ValueWrapper<T> wrapper)
{
// Any test payload is fine.
string json = @"""Some string""";
_ = wrapper; // only used to instantiate T

await RunTest<Type>(json);
await RunTest<SerializationInfo>(json);
await RunTest<IntPtr>(json);
await RunTest<IntPtr?>(json); // One nullable variation.
await RunTest<UIntPtr>(json);
string json = @"""Some string"""; // Any test payload is fine.

async Task RunTest<T>(string json)
{
Type type = GetNullableOfTUnderlyingType(typeof(T), out bool isNullableOfT);
string fullName = type.FullName;
Type type = GetNullableOfTUnderlyingType(typeof(T), out bool isNullableOfT);
string fullName = type.FullName;

NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.DeserializeWrapper<T>(json));
string exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$", exAsStr);
NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.DeserializeWrapper<T>(json));
string exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$", exAsStr);

json = $@"{{""Prop"":{json}}}";
json = $@"{{""Prop"":{json}}}";

ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.DeserializeWrapper<ClassWithType<T>>(json));
exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$.Prop", exAsStr);
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.DeserializeWrapper<ClassWithType<T>>(json));
exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$.Prop", exAsStr);

// Verify Nullable<> semantics. NSE is not thrown because the serializer handles null.
if (isNullableOfT)
{
Assert.Null(JsonSerializer.Deserialize<T>("null"));
// Verify Nullable<> semantics. NSE is not thrown because the serializer handles null.
if (isNullableOfT)
{
Assert.Null(JsonSerializer.Deserialize<T>("null"));

json = $@"{{""Prop"":null}}";
ClassWithType<T> obj = await Serializer.DeserializeWrapper<ClassWithType<T>>(json);
Assert.Null(obj.Prop);
}
json = $@"{{""Prop"":null}}";
ClassWithType<T> obj = await Serializer.DeserializeWrapper<ClassWithType<T>>(json);
Assert.Null(obj.Prop);
}
}

[Fact]
public async Task SerializeUnsupportedType()
[Theory]
[MemberData(nameof(GetUnsupportedValues))]
public async Task SerializeUnsupportedType<T>(ValueWrapper<T> wrapper)
{
// TODO refactor to Xunit theory
await RunTest(typeof(int));
await RunTest(new SerializationInfo(typeof(Type), new FormatterConverter()));
await RunTest((IntPtr)123);
await RunTest<IntPtr?>(new IntPtr(123)); // One nullable variation.
await RunTest((UIntPtr)123);

async Task RunTest<T>(T value)
T value = wrapper.value;

Type type = GetNullableOfTUnderlyingType(typeof(T), out bool isNullableOfT);
string fullName = type.FullName;

NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(value));
string exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$", exAsStr);

ClassWithType<T> obj = new ClassWithType<T> { Prop = value };
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(obj));
exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);

if (SupportsJsonPathOnSerialize)
{
Assert.Contains("$.Prop", exAsStr);
}
else
{
Type type = GetNullableOfTUnderlyingType(typeof(T), out bool isNullableOfT);
string fullName = type.FullName;

NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(value));
string exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);
Assert.Contains("$", exAsStr);

ClassWithType<T> obj = new ClassWithType<T> { Prop = value };
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(obj));
exAsStr = ex.ToString();
Assert.Contains(fullName, exAsStr);

if (SupportsJsonPathOnSerialize)
{
Assert.Contains("$.Prop", exAsStr);
}
else
{
Assert.Contains("$.", exAsStr);
Assert.DoesNotContain("$.Prop", exAsStr);
}

// Verify null semantics. NSE is not thrown because the serializer handles null.
if (!type.IsValueType || isNullableOfT)
{
string serialized = await Serializer.SerializeWrapper<T>((T)(object)null);
Assert.Equal("null", serialized);

obj.Prop = (T)(object)null;
serialized = await Serializer.SerializeWrapper(obj);
Assert.Equal(@"{""Prop"":null}", serialized);

serialized = await Serializer.SerializeWrapper(obj, new JsonSerializerOptions { IgnoreNullValues = true });
Assert.Equal(@"{}", serialized);
}
Assert.Contains("$.", exAsStr);
Assert.DoesNotContain("$.Prop", exAsStr);
}

// Verify null semantics. NSE is not thrown because the serializer handles null.
if (!type.IsValueType || isNullableOfT)
{
string serialized = await Serializer.SerializeWrapper<T>((T)(object)null);
Assert.Equal("null", serialized);

obj.Prop = (T)(object)null;
serialized = await Serializer.SerializeWrapper(obj);
Assert.Equal(@"{""Prop"":null}", serialized);

serialized = await Serializer.SerializeWrapper(obj, new JsonSerializerOptions { IgnoreNullValues = true });
Assert.Equal(@"{}", serialized);
}

#if !BUILDING_SOURCE_GENERATOR_TESTS
Type runtimeType = GetNullableOfTUnderlyingType(value.GetType(), out bool _);
Type runtimeType = GetNullableOfTUnderlyingType(value.GetType(), out bool _);

ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper<object>(value));
exAsStr = ex.ToString();
Assert.Contains(runtimeType.FullName, exAsStr);
Assert.Contains("$", exAsStr);
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper<object>(value));
exAsStr = ex.ToString();
Assert.Contains(runtimeType.FullName, exAsStr);
Assert.Contains("$", exAsStr);

ClassWithType<object> polyObj = new ClassWithType<object> { Prop = value };
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(polyObj));
exAsStr = ex.ToString();
Assert.Contains(runtimeType.FullName, exAsStr);
ClassWithType<object> polyObj = new ClassWithType<object> { Prop = value };
ex = await Assert.ThrowsAsync<NotSupportedException>(async () => await Serializer.SerializeWrapper(polyObj));
exAsStr = ex.ToString();
Assert.Contains(runtimeType.FullName, exAsStr);
#endif
}
}

public static IEnumerable<object[]> GetUnsupportedValues()
{
yield return WrapArgs(typeof(int));
yield return WrapArgs(typeof(ClassWithExtensionProperty).GetConstructor(Array.Empty<Type>()));
yield return WrapArgs(typeof(ClassWithExtensionProperty).GetProperty(nameof(ClassWithExtensionProperty.MyInt)));
yield return WrapArgs(new SerializationInfo(typeof(Type), new FormatterConverter()));
yield return WrapArgs((IntPtr)123);
yield return WrapArgs<IntPtr?>(new IntPtr(123)); // One nullable variation.
yield return WrapArgs((UIntPtr)123);

static object[] WrapArgs<T>(T value) => new object[] { new ValueWrapper<T>(value) };
}

// Helper record used to path both value & type information to generic theories.
// This is needed e.g. when passing System.Type instances whose runtime type
// actually is System.Reflection.RuntimeType.
public record ValueWrapper<T>(T value)
{
public override string ToString() => value.ToString();
}

public class ClassWithIntPtr
Expand Down Expand Up @@ -151,6 +159,21 @@ public override void Write(Utf8JsonWriter writer, IntPtr value, JsonSerializerOp
}
}

#if !BUILDING_SOURCE_GENERATOR_TESTS
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
[Fact]
public async Task TypeWithNullConstructorParameterName_ThrowsNotSupportedException()
{
// Regression test for https://github.com/dotnet/runtime/issues/58690
Type type = Assembly.GetExecutingAssembly().GetType("System.Runtime.CompilerServices.NullableContextAttribute")!;
ConstructorInfo ctorInfo = type.GetConstructor(new Type[] { typeof(byte) });
Assert.True(string.IsNullOrEmpty(ctorInfo.GetParameters()[0].Name));
object value = ctorInfo.Invoke(new object[] { (byte)0 });

await Assert.ThrowsAnyAsync<NotSupportedException>(() => Serializer.SerializeWrapper(value));
await Assert.ThrowsAnyAsync<NotSupportedException>(() => Serializer.DeserializeWrapper("{}", type));
}
#endif

[Fact]
public async Task RuntimeConverterIsSupported_IntPtr()
{
Expand Down
Loading