Skip to content

Commit

Permalink
Fail JsonTypeInfo.CreateObject setting when not supported by converte…
Browse files Browse the repository at this point in the history
…r. (#73395)

* Fail JsonTypeInfo.CreateObject setting when not supported by converter.

* add clarifying comment.

* Remove dead code.
  • Loading branch information
eiriktsarpalis authored Aug 4, 2022
1 parent 247bcdc commit ec3e934
Show file tree
Hide file tree
Showing 22 changed files with 112 additions and 21 deletions.
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 @@ -650,6 +650,9 @@
<data name="InvalidJsonTypeInfoOperationForKind" xml:space="preserve">
<value>Invalid JsonTypeInfo operation for JsonTypeInfoKind '{0}'.</value>
</data>
<data name="CreateObjectConverterNotCompatible" xml:space="preserve">
<value>The converter for type '{0}' does not support setting 'CreateObject' delegates.</value>
</data>
<data name="CombineOneOfResolversIsNull" xml:space="preserve">
<value>One of the provided resolvers is null.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ internal sealed class CastingConverter<T, TSource> : JsonConverter<T>

public override bool HandleNull => _sourceConverter.HandleNull;
internal override ConverterStrategy ConverterStrategy => _sourceConverter.ConverterStrategy;
internal override bool SupportsCreateObjectDelegate => _sourceConverter.SupportsCreateObjectDelegate;

internal CastingConverter(JsonConverter<TSource> sourceConverter) : base(initialize: false)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ protected override void Add(in TElement value, ref ReadStack state)
((List<TElement>)state.Current.ReturnValue!).Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
state.Current.ReturnValue = new List<TElement>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ protected override void Add(in TElement value, ref ReadStack state)
((BufferedAsyncEnumerable)state.Current.ReturnValue!)._buffer.Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
state.Current.ReturnValue = new BufferedAsyncEnumerable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ protected override void Add(in object? value, ref ReadStack state)
((List<object?>)state.Current.ReturnValue!).Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
if (!_isDeserializable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ protected override void Add(in TElement value, ref ReadStack state)
((List<TElement>)state.Current.ReturnValue!).Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
if (!_isDeserializable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ protected override void Add(TKey key, in TValue value, JsonSerializerOptions opt
((Dictionary<TKey, TValue>)state.Current.ReturnValue!)[key] = value;
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state)
{
if (!_isDeserializable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ protected sealed override void Add(TKey key, in TValue value, JsonSerializerOpti

internal sealed override bool CanHaveMetadata => false;

internal override bool SupportsCreateObjectDelegate => false;
protected sealed override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state)
{
state.Current.ReturnValue = new Dictionary<TKey, TValue>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ protected sealed override void Add(in TElement value, ref ReadStack state)

internal sealed override bool CanHaveMetadata => false;

internal override bool SupportsCreateObjectDelegate => false;
protected sealed override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
state.Current.ReturnValue = new List<TElement>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace System.Text.Json.Serialization
/// </summary>
internal abstract class JsonCollectionConverter<TCollection, TElement> : JsonResumableConverter<TCollection>
{
internal override bool SupportsCreateObjectDelegate => true;
internal sealed override ConverterStrategy ConverterStrategy => ConverterStrategy.Enumerable;
internal override Type ElementType => typeof(TElement);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace System.Text.Json.Serialization
/// </summary>
internal abstract class JsonDictionaryConverter<TDictionary> : JsonResumableConverter<TDictionary>
{
internal override bool SupportsCreateObjectDelegate => true;
internal sealed override ConverterStrategy ConverterStrategy => ConverterStrategy.Dictionary;

protected internal abstract bool OnWriteResume(Utf8JsonWriter writer, TDictionary dictionary, JsonSerializerOptions options, ref WriteStack state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ protected override void Add(in TElement value, ref ReadStack state)
((List<TElement>)state.Current.ReturnValue!).Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
state.Current.ReturnValue = new List<TElement>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ protected override void Add(TKey key, in TValue value, JsonSerializerOptions opt

internal override bool CanHaveMetadata => false;

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state)
{
state.Current.ReturnValue = new List<Tuple<TKey, TValue>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ protected override void Add(in TElement value, ref ReadStack state)
((List<TElement>)state.Current.ReturnValue!).Add(value);
}

internal override bool SupportsCreateObjectDelegate => false;
protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
state.Current.ReturnValue = new List<TElement>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal JsonConverter<T> Converter
internal override Type? ElementType => Converter.ElementType;

internal override bool ConstructorIsParameterized => Converter.ConstructorIsParameterized;

internal override bool SupportsCreateObjectDelegate => Converter.SupportsCreateObjectDelegate;
internal override bool CanHaveMetadata => Converter.CanHaveMetadata;

public JsonMetadataServicesConverter(Func<JsonConverter<T>> converterCreator, ConverterStrategy converterStrategy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace System.Text.Json.Serialization.Converters
internal class ObjectDefaultConverter<T> : JsonObjectConverter<T> where T : notnull
{
internal override bool CanHaveMetadata => true;
internal override bool SupportsCreateObjectDelegate => true;

internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, [MaybeNullWhen(false)] out T value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ internal JsonConverter() { }

internal abstract ConverterStrategy ConverterStrategy { get; }

/// <summary>
/// Indicates that the converter can consume the <see cref="JsonTypeInfo.CreateObject"/> delegate.
/// Needed because certain collection converters cannot support arbitrary delegates.
/// TODO remove once https://github.com/dotnet/runtime/pull/73395/ and
/// https://github.com/dotnet/runtime/issues/71944 have been addressed.
/// </summary>
internal virtual bool SupportsCreateObjectDelegate => false;

/// <summary>
/// Can direct Read or Write methods be called (for performance).
/// </summary>
Expand Down Expand Up @@ -117,19 +125,6 @@ internal static bool ShouldFlush(Utf8JsonWriter writer, ref WriteStack state)
// Whether a type (ConverterStrategy.Object) is deserialized using a parameterized constructor.
internal virtual bool ConstructorIsParameterized { get; }

/// <summary>
/// For reflection-based metadata generation, indicates whether the
/// converter avails of default constructors when deserializing types.
/// </summary>
internal bool UsesDefaultConstructor =>
ConverterStrategy switch
{
ConverterStrategy.Object => !ConstructorIsParameterized && this is not ObjectConverter,
ConverterStrategy.Enumerable or
ConverterStrategy.Dictionary => true,
_ => false
};

internal ConstructorInfo? ConstructorInfo { get; set; }

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ private protected override void SetCreateObject(Delegate? createObject)
ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
}

if (!Converter.SupportsCreateObjectDelegate)
{
Debug.Assert(_createObject is null);
Debug.Assert(_typedCreateObject == null);
ThrowHelper.ThrowInvalidOperationException_CreateObjectConverterNotCompatible(Type);
}

Func<object>? untypedCreateObject;
Func<T>? typedCreateObject;

Expand All @@ -86,6 +93,18 @@ private protected override void SetCreateObject(Delegate? createObject)
_typedCreateObject = typedCreateObject;
}

private protected void SetCreateObjectIfCompatible(Delegate? createObject)
{
Debug.Assert(!IsConfigured);

// Guard against the reflection resolver/source generator attempting to pass
// a CreateObject delegate to converters/metadata that do not support it.
if (Converter.SupportsCreateObjectDelegate && !Converter.ConstructorIsParameterized)
{
SetCreateObject(createObject);
}
}

internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
: base(typeof(T), converter, options)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ internal ReflectionJsonTypeInfo(JsonConverter converter, JsonSerializerOptions o
}

Func<object>? createObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
if (converter.UsesDefaultConstructor)
{
SetCreateObject(createObject);
}

SetCreateObjectIfCompatible(createObject);
CreateObjectForExtensionDataProperty = createObject;

// Plug in any converter configuration -- should be run last.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues
}
else
{
SetCreateObject(objectInfo.ObjectCreator);
SetCreateObjectIfCompatible(objectInfo.ObjectCreator);
CreateObjectForExtensionDataProperty = ((JsonTypeInfo)this).CreateObject;
}

Expand Down Expand Up @@ -76,7 +76,7 @@ public SourceGenJsonTypeInfo(
SerializeHandler = collectionInfo.SerializeHandler;
CreateObjectWithArgs = createObjectWithArgs;
AddMethodDelegate = addFunc;
CreateObject = collectionInfo.ObjectCreator;
SetCreateObjectIfCompatible(collectionInfo.ObjectCreator);
PopulatePolymorphismMetadata();
MapInterfaceTypesToCallbacks();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,12 @@ public static void ThrowInvalidOperationException_JsonTypeInfoOperationNotPossib
throw new InvalidOperationException(SR.Format(SR.InvalidJsonTypeInfoOperationForKind, kind));
}

[DoesNotReturn]
public static void ThrowInvalidOperationException_CreateObjectConverterNotCompatible(Type type)
{
throw new InvalidOperationException(SR.Format(SR.CreateObjectConverterNotCompatible, type));
}

[DoesNotReturn]
public static void ReThrowWithPath(ref ReadStack state, JsonReaderException ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Reflection;
using System.Text.Json.Serialization.Metadata;
Expand Down Expand Up @@ -445,6 +447,62 @@ private static void TestTypeInfoImmutability<T>(JsonTypeInfo<T> typeInfo)
}
}

[Theory]
[InlineData(typeof(ICollection<int>), typeof(List<int>), false)]
[InlineData(typeof(IList), typeof(List<int>), false)]
[InlineData(typeof(IList<int>), typeof(List<int>), false)]
[InlineData(typeof(List<int>), typeof(List<int>), false)]
[InlineData(typeof(ISet<int>), typeof(HashSet<int>), false)]
[InlineData(typeof(Queue<int>), typeof(Queue<int>), false)]
[InlineData(typeof(Stack<int>), typeof(Stack<int>), false)]
[InlineData(typeof(IDictionary), typeof(Hashtable), true)]
[InlineData(typeof(IDictionary<string, int>), typeof(ConcurrentDictionary<string, int>), true)]
public static void JsonTypeInfo_CreateObject_SupportedCollection(Type collectionType, Type runtimeType, bool isDictionary)
{
bool isDelegateInvoked = false;

var options = new JsonSerializerOptions
{
TypeInfoResolver = new DefaultJsonTypeInfoResolver
{
Modifiers =
{
jsonTypeInfo =>
{
if (jsonTypeInfo.Type == collectionType)
{
jsonTypeInfo.CreateObject = () =>
{
isDelegateInvoked = true;
return Activator.CreateInstance(runtimeType);
};
}
}
}
}
};

string json = isDictionary ? "{}" : "[]";
object result = JsonSerializer.Deserialize(json, collectionType, options);
Assert.IsType(runtimeType, result);
Assert.True(isDelegateInvoked);
}

[Theory]
[InlineData(typeof(int[]))]
[InlineData(typeof(ImmutableArray<int>))]
[InlineData(typeof(IEnumerable))]
[InlineData(typeof(IEnumerable<int>))]
[InlineData(typeof(IAsyncEnumerable<int>))]
[InlineData(typeof(IReadOnlyDictionary<string, int>))]
[InlineData(typeof(ImmutableDictionary<string, int>))]
public static void JsonTypeInfo_CreateObject_UnsupportedCollection_ThrowsInvalidOperationException(Type collectionType)
{
var options = new JsonSerializerOptions();
JsonTypeInfo jsonTypeInfo = JsonTypeInfo.CreateJsonTypeInfo(collectionType, options);
Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.CreateObject = () => new object());
}

[Theory]
[InlineData(typeof(object), JsonTypeInfoKind.None)]
[InlineData(typeof(string), JsonTypeInfoKind.None)]
Expand Down

0 comments on commit ec3e934

Please sign in to comment.