Skip to content

Commit

Permalink
Fix NullReference on JsonExtensionData (dotnet#34569)
Browse files Browse the repository at this point in the history 
* Fix NullReference on JsonExtensionData

Fix NullReferenceException on serialization when JsonExtensionData dictionary has a custom JsonConverter.

Fix dotnet#32903

* Add test for converters declared through attribute

* Ensure custom serialization is not used for reading JsonExtensionData

* Cover all the combinations in tests
  • Loading branch information
@RezaJooyandeh
RezaJooyandeh authored Apr 14, 2020
1 parent 3d0ef96 commit cfb109b
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 5 deletions.
9 changes: 9 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,17 @@ internal bool TryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions opt

internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, JsonSerializerOptions options, ref WriteStack state)
{
Debug.Assert(value != null);

if (!IsInternalConverter)
{
return TryWrite(writer, value, options, ref state);
}

Debug.Assert(this is JsonDictionaryConverter<T>);

state.Current.PolymorphicJsonPropertyInfo = state.Current.DeclaredJsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PropertyInfoForClassInfo;

if (writer.CurrentDepth >= options.EffectiveMaxDepth)
{
ThrowHelper.ThrowJsonException_SerializerCycleDetected(options.EffectiveMaxDepth);
Expand Down
6 changes: 2 additions & 4 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ public bool ReadJsonAndAddExtensionProperty(object obj, ref ReadStack state, ref
}
else
{
JsonConverter<object> converter = (JsonConverter<object>)
state.Current.JsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PropertyInfoForClassInfo.ConverterBase;
JsonConverter<object> converter = (JsonConverter<object>)Options.GetConverter(typeof(object));

if (!converter.TryRead(ref reader, typeof(JsonElement), Options, ref state, out object? value))
{
Expand All @@ -240,8 +239,7 @@ public bool ReadJsonAndAddExtensionProperty(object obj, ref ReadStack state, ref
Debug.Assert(propValue is IDictionary<string, JsonElement>);
IDictionary<string, JsonElement> dictionaryJsonElement = (IDictionary<string, JsonElement>)propValue;

JsonConverter<JsonElement> converter = (JsonConverter<JsonElement>)
state.Current.JsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PropertyInfoForClassInfo.ConverterBase;
JsonConverter<JsonElement> converter = (JsonConverter<JsonElement>)Options.GetConverter(typeof(JsonElement));

if (!converter.TryRead(ref reader, typeof(JsonElement), Options, ref state, out JsonElement value))
{
Expand Down
1 change: 0 additions & 1 deletion ...s/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ public override bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteSta
}
else
{
state.Current.PolymorphicJsonPropertyInfo = state.Current.DeclaredJsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PropertyInfoForClassInfo;
success = Converter.TryWriteDataExtensionProperty(writer, value, Options, ref state);
}

Expand Down
202 changes: 202 additions & 0 deletions src/libraries/System.Text.Json/tests/Serialization/ExtensionDataTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using Xunit;

namespace System.Text.Json.Serialization.Tests
Expand Down Expand Up @@ -250,6 +253,205 @@ public static void IgnoredDataShouldNotBeExtensionData()
Assert.Null(obj.MyOverflow);
}

private class ClassWithExtensionData<T>
{
[JsonExtensionData]
public T Overflow { get; set; }
}

public class CustomOverflowDictionary<T> : Dictionary<string, T>
{
}

public class DictionaryOverflowConverter : JsonConverter<Dictionary<string, object>>
{
public override Dictionary<string, object> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotImplementedException();
}

public override void Write(Utf8JsonWriter writer, Dictionary<string, object> value, JsonSerializerOptions options)
{
writer.WriteString("MyCustomOverflowWrite", "OverflowValueWrite");
}
}

public class JsonElementOverflowConverter : JsonConverter<Dictionary<string, JsonElement>>
{
public override Dictionary<string, JsonElement> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotImplementedException();
}

public override void Write(Utf8JsonWriter writer, Dictionary<string, JsonElement> value, JsonSerializerOptions options)
{
writer.WriteString("MyCustomOverflowWrite", "OverflowValueWrite");
}
}

public class CustomObjectDictionaryOverflowConverter : JsonConverter<CustomOverflowDictionary<object>>
{
public override CustomOverflowDictionary<object> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotImplementedException();
}

public override void Write(Utf8JsonWriter writer, CustomOverflowDictionary<object> value, JsonSerializerOptions options)
{
writer.WriteString("MyCustomOverflowWrite", "OverflowValueWrite");
}
}

public class CustomJsonElementDictionaryOverflowConverter : JsonConverter<CustomOverflowDictionary<JsonElement>>
{
public override CustomOverflowDictionary<JsonElement> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotImplementedException();
}

public override void Write(Utf8JsonWriter writer, CustomOverflowDictionary<JsonElement> value, JsonSerializerOptions options)
{
writer.WriteString("MyCustomOverflowWrite", "OverflowValueWrite");
}
}

[Theory]
[InlineData(typeof(Dictionary<string, object>), typeof(DictionaryOverflowConverter))]
[InlineData(typeof(Dictionary<string, JsonElement>), typeof(JsonElementOverflowConverter))]
[InlineData(typeof(CustomOverflowDictionary<object>), typeof(CustomObjectDictionaryOverflowConverter))]
[InlineData(typeof(CustomOverflowDictionary<JsonElement>), typeof(CustomJsonElementDictionaryOverflowConverter))]
public static void ExtensionProperty_SupportsWritingToCustomSerializerWithOptions(Type overflowType, Type converterType)
{
typeof(ExtensionDataTests)
.GetMethod(nameof(ExtensionProperty_SupportsWritingToCustomSerializerWithOptionsInternal), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(overflowType, converterType)
.Invoke(null, null);
}

private static void ExtensionProperty_SupportsWritingToCustomSerializerWithOptionsInternal<TDictionary, TConverter>()
where TDictionary : new()
where TConverter : JsonConverter, new()
{
var root = new ClassWithExtensionData<TDictionary>()
{
Overflow = new TDictionary()
};

var options = new JsonSerializerOptions();
options.Converters.Add(new TConverter());

string json = JsonSerializer.Serialize(root, options);
Assert.Equal(@"{""MyCustomOverflowWrite"":""OverflowValueWrite""}", json);
}

private interface IClassWithOverflow<T>
{
public T Overflow { get; set; }
}

private class ClassWithExtensionDataWithAttributedConverter : IClassWithOverflow<Dictionary<string, object>>
{
[JsonExtensionData]
[JsonConverter(typeof(DictionaryOverflowConverter))]
public Dictionary<string, object> Overflow { get; set; }
}

private class ClassWithJsonElementExtensionDataWithAttributedConverter : IClassWithOverflow<Dictionary<string, JsonElement>>
{
[JsonExtensionData]
[JsonConverter(typeof(JsonElementOverflowConverter))]
public Dictionary<string, JsonElement> Overflow { get; set; }
}

private class ClassWithCustomElementExtensionDataWithAttributedConverter : IClassWithOverflow<CustomOverflowDictionary<object>>
{
[JsonExtensionData]
[JsonConverter(typeof(CustomObjectDictionaryOverflowConverter))]
public CustomOverflowDictionary<object> Overflow { get; set; }
}

private class ClassWithCustomJsonElementExtensionDataWithAttributedConverter : IClassWithOverflow<CustomOverflowDictionary<JsonElement>>
{
[JsonExtensionData]
[JsonConverter(typeof(CustomJsonElementDictionaryOverflowConverter))]
public CustomOverflowDictionary<JsonElement> Overflow { get; set; }
}

[Theory]
[InlineData(typeof(ClassWithExtensionDataWithAttributedConverter), typeof(Dictionary<string, object>))]
[InlineData(typeof(ClassWithJsonElementExtensionDataWithAttributedConverter), typeof(Dictionary<string, JsonElement>))]
[InlineData(typeof(ClassWithCustomElementExtensionDataWithAttributedConverter), typeof(CustomOverflowDictionary<object>))]
[InlineData(typeof(ClassWithCustomJsonElementExtensionDataWithAttributedConverter), typeof(CustomOverflowDictionary<JsonElement>))]
public static void ExtensionProperty_SupportsWritingToCustomSerializerWithExplicitConverter(Type attributedType, Type dictionaryType)
{
typeof(ExtensionDataTests)
.GetMethod(nameof(ExtensionProperty_SupportsWritingToCustomSerializerWithExplicitConverterInternal), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(attributedType, dictionaryType)
.Invoke(null, null);
}

private static void ExtensionProperty_SupportsWritingToCustomSerializerWithExplicitConverterInternal<TRoot, TDictionary>()
where TRoot : IClassWithOverflow<TDictionary>, new()
where TDictionary : new()
{
var root = new TRoot()
{
Overflow = new TDictionary()
};

string json = JsonSerializer.Serialize(root);
Assert.Equal(@"{""MyCustomOverflowWrite"":""OverflowValueWrite""}", json);
}

[Theory]
[InlineData(typeof(Dictionary<string, object>), typeof(DictionaryOverflowConverter), typeof(object))]
[InlineData(typeof(Dictionary<string, JsonElement>), typeof(JsonElementOverflowConverter), typeof(JsonElement))]
[InlineData(typeof(CustomOverflowDictionary<object>), typeof(CustomObjectDictionaryOverflowConverter), typeof(object))]
[InlineData(typeof(CustomOverflowDictionary<JsonElement>), typeof(CustomJsonElementDictionaryOverflowConverter), typeof(JsonElement))]
public static void ExtensionProperty_IgnoresCustomSerializerWithOptions(Type overflowType, Type converterType, Type elementType)
{
typeof(ExtensionDataTests)
.GetMethod(nameof(ExtensionProperty_IgnoresCustomSerializerWithOptionsInternal), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(overflowType, elementType, converterType)
.Invoke(null, null);
}

private static void ExtensionProperty_IgnoresCustomSerializerWithOptionsInternal<TDictionary, TOverflowItem, TConverter>()
where TConverter : JsonConverter, new()
where TDictionary : IDictionary<string, TOverflowItem>
{
var options = new JsonSerializerOptions();
options.Converters.Add(new TConverter());

ClassWithExtensionData<TDictionary> obj
= JsonSerializer.Deserialize<ClassWithExtensionData<TDictionary>>(@"{""TestKey"":""TestValue""}", options);

Assert.Equal("TestValue", ((JsonElement)(object)obj.Overflow["TestKey"]).GetString());
}

[Theory]
[InlineData(typeof(ClassWithExtensionDataWithAttributedConverter), typeof(Dictionary<string, object>), typeof(object))]
[InlineData(typeof(ClassWithJsonElementExtensionDataWithAttributedConverter), typeof(Dictionary<string, JsonElement>), typeof(JsonElement))]
[InlineData(typeof(ClassWithCustomElementExtensionDataWithAttributedConverter), typeof(CustomOverflowDictionary<object>), typeof(object))]
[InlineData(typeof(ClassWithCustomJsonElementExtensionDataWithAttributedConverter), typeof(CustomOverflowDictionary<JsonElement>), typeof(JsonElement))]
public static void ExtensionProperty_IgnoresCustomSerializerWithExplicitConverter(Type attributedType, Type dictionaryType, Type elementType)
{
typeof(ExtensionDataTests)
.GetMethod(nameof(ExtensionProperty_IgnoresCustomSerializerWithExplicitConverterInternal), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(attributedType, dictionaryType, elementType)
.Invoke(null, null);
}

private static void ExtensionProperty_IgnoresCustomSerializerWithExplicitConverterInternal<TRoot, TDictionary, TOverflowItem>()
where TRoot : IClassWithOverflow<TDictionary>, new()
where TDictionary : IDictionary<string, TOverflowItem>
{
ClassWithExtensionData<TDictionary> obj
= JsonSerializer.Deserialize<ClassWithExtensionData<TDictionary>>(@"{""TestKey"":""TestValue""}");

Assert.Equal("TestValue", ((JsonElement)(object)obj.Overflow["TestKey"]).GetString());
}

[Fact]
public static void ExtensionPropertyObjectValue_Empty()
{
Expand Down

0 comments on commit cfb109b

Please sign in to comment.