Skip to content

Commit

Permalink
Do not deserialize using internal or private default ctors for all su…
Browse files Browse the repository at this point in the history
…pported TFMs. (dotnet#32213)

* Do not deserialize using internal or private default ctors for all
supported TFMs.

* Add a test with generic class with protected internal ctor and clean up.

* Make test classes private and add Debug.Fail instead of throwing.
  • Loading branch information
ahsonkhan authored Feb 14, 2020
1 parent 7c56d13 commit 0174b98
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
public override JsonClassInfo.ConstructorDelegate? CreateConstructor(Type type)
{
Debug.Assert(type != null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);

if (type.IsAbstract)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal sealed class ReflectionMemberAccessor : MemberAccessor
public override JsonClassInfo.ConstructorDelegate? CreateConstructor(Type type)
{
Debug.Assert(type != null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);

if (type.IsAbstract)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2145,15 +2145,64 @@ public static void VerifyIDictionaryWithNonStringKey()
Assert.Throws<JsonException>(() => JsonSerializer.Serialize(dictionary));
}

public class ClassWithoutParameterlessCtor
private class ClassWithoutParameterlessCtor
{
public ClassWithoutParameterlessCtor(int num) { }
public string Name { get; set; }
}

private class ClassWithInternalParameterlessConstructor
{
internal ClassWithInternalParameterlessConstructor() { }
public string Name { get; set; }
}

private class ClassWithPrivateParameterlessConstructor
{
private ClassWithPrivateParameterlessConstructor() { }
public string Name { get; set; }
}

[Fact]
public static void DictionaryWith_ObjectWithNoParameterlessCtor_AsValue_Throws()
{
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<Dictionary<string, ClassWithoutParameterlessCtor>>(@"{""key"":{}}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<Dictionary<string, ClassWithInternalParameterlessConstructor>>(@"{""key"":{}}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<Dictionary<string, ClassWithPrivateParameterlessConstructor>>(@"{""key"":{}}"));
}

[Fact]
public static void DictionaryWith_ObjectWithNoParameterlessCtor_Serialize_Works()
{
var noParameterless = new Dictionary<string, ClassWithoutParameterlessCtor>()
{
["key"] = new ClassWithoutParameterlessCtor(5)
{
Name = "parameterless"
}
};

string json = JsonSerializer.Serialize(noParameterless);
Assert.Equal("{\"key\":{\"Name\":\"parameterless\"}}", json);

var onlyInternal = new Dictionary<string, ClassWithInternalParameterlessConstructor>()
{
["key"] = new ClassWithInternalParameterlessConstructor()
{
Name = "internal"
}
};

json = JsonSerializer.Serialize(onlyInternal);
Assert.Equal("{\"key\":{\"Name\":\"internal\"}}", json);

var onlyPrivate = new Dictionary<string, ClassWithPrivateParameterlessConstructor>()
{
["key"] = null
};

json = JsonSerializer.Serialize(onlyPrivate);
Assert.Equal("{\"key\":null}", json);
}
}
}
155 changes: 148 additions & 7 deletions src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs
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.
// See the LICENSE file in the project root for more information.

using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using Xunit;

Expand Down Expand Up @@ -213,22 +215,161 @@ public static void ReadObjectFail(string json, bool failsOnObject)
}

[Fact]
public static void ReadObjectFail_ReferenceTypeMissingParameterlessConstructor()
public static void ReadObjectFail_ReferenceTypeMissingPublicParameterlessConstructor()
{
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<PublicParameterizedConstructorTestClass>(@"{""Name"":""Name!""}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<ClassWithInternalParameterlessCtor>(@"{""Name"":""Name!""}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<ClassWithPrivateParameterlessCtor>(@"{""Name"":""Name!""}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<CollectionWithoutPublicParameterlessCtor>(@"[""foo"", 1, false]"));

Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<GenericClassWithProtectedInternalCtor<string>>("{\"Result\":null}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<ConcreteDerivedClassWithNoPublicDefaultCtor>("{\"ErrorString\":\"oops\"}"));
}

class PublicParameterizedConstructorTestClass
private class PublicParameterizedConstructorTestClass
{
private readonly string _name;
public PublicParameterizedConstructorTestClass(string name)
{
_name = name;
Debug.Fail("The JsonSerializer should not be callin non-public ctors, by default.");
}

private PublicParameterizedConstructorTestClass(int internalId)
{
Name = internalId.ToString();
}

public string Name { get; }

public static PublicParameterizedConstructorTestClass Instance { get; } = new PublicParameterizedConstructorTestClass(42);
}

private class ClassWithInternalParameterlessCtor
{
internal ClassWithInternalParameterlessCtor()
{
Debug.Fail("The JsonSerializer should not be callin non-public ctors, by default.");
}
public string Name

private ClassWithInternalParameterlessCtor(string name)
{
Name = name;
}

public string Name { get; set; }

public static ClassWithInternalParameterlessCtor Instance { get; } = new ClassWithInternalParameterlessCtor("InstancePropertyInternal");
}

private class ClassWithPrivateParameterlessCtor
{
private ClassWithPrivateParameterlessCtor()
{
get { return _name; }
Debug.Fail("The JsonSerializer should not be callin non-public ctors, by default.");
}

private ClassWithPrivateParameterlessCtor(string name)
{
Name = name;
}

public string Name { get; set; }

public static ClassWithPrivateParameterlessCtor Instance { get; } = new ClassWithPrivateParameterlessCtor("InstancePropertyPrivate");
}

private class CollectionWithoutPublicParameterlessCtor : IList
{
private readonly List<object> _list;

internal CollectionWithoutPublicParameterlessCtor()
{
Debug.Fail("The JsonSerializer should not be callin non-public ctors, by default.");
}

public CollectionWithoutPublicParameterlessCtor(List<object> list)
{
_list = list;
}

public object this[int index] { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }

public bool IsFixedSize => throw new NotImplementedException();

public bool IsReadOnly => false;

public int Count => _list.Count;

public bool IsSynchronized => throw new NotImplementedException();

public object SyncRoot => throw new NotImplementedException();

public int Add(object value)
{
int pos = _list.Count;
_list.Add(value);
return pos;
}

public void Clear()
{
throw new NotImplementedException();
}

public bool Contains(object value)
{
throw new NotImplementedException();
}

public void CopyTo(Array array, int index)
{
throw new NotImplementedException();
}

public IEnumerator GetEnumerator() => _list.GetEnumerator();

public int IndexOf(object value)
{
throw new NotImplementedException();
}

public void Insert(int index, object value)
{
throw new NotImplementedException();
}

public void Remove(object value)
{
throw new NotImplementedException();
}

public void RemoveAt(int index)
{
throw new NotImplementedException();
}
}

private class GenericClassWithProtectedInternalCtor<T>
{
public T Result { get; set; }

protected internal GenericClassWithProtectedInternalCtor()
{
Result = default;
}
}

private sealed class ConcreteDerivedClassWithNoPublicDefaultCtor : GenericClassWithProtectedInternalCtor<string>
{
public string ErrorString { get; set; }

private ConcreteDerivedClassWithNoPublicDefaultCtor(string error)
{
ErrorString = error;
}

public static ConcreteDerivedClassWithNoPublicDefaultCtor Ok() => new ConcreteDerivedClassWithNoPublicDefaultCtor("ok");
public static GenericClassWithProtectedInternalCtor<T> Ok<T>() => new GenericClassWithProtectedInternalCtor<T>();
public static ConcreteDerivedClassWithNoPublicDefaultCtor Error(string error) => new ConcreteDerivedClassWithNoPublicDefaultCtor(error);
}

[Fact]
Expand Down Expand Up @@ -408,7 +549,7 @@ public class ClassMixingSkippedTypes
public Dictionary<string, int> ParsedDictionary { get; set; }

public IList<int> AnotherSkippedList { get; }
public IDictionary<string, string> AnotherSkippedDictionary2 { get; }
public IDictionary<string, string> AnotherSkippedDictionary2 { get; }
public IDictionary<string, string> SkippedDictionaryNotInJson { get; }
public SimpleTestClass AnotherSkippedClass { get; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,31 @@ public int this[int index]
public string NonIndexerProp { get; set; }
}

[Fact]
public static void WriteObjectWorks_ReferenceTypeMissingPublicParameterlessConstructor()
{
PublicParameterizedConstructorTestClass paramterless = PublicParameterizedConstructorTestClass.Instance;
Assert.Equal("{\"Name\":\"42\"}", JsonSerializer.Serialize(paramterless));

ClassWithInternalParameterlessCtor internalObj = ClassWithInternalParameterlessCtor.Instance;
Assert.Equal("{\"Name\":\"InstancePropertyInternal\"}", JsonSerializer.Serialize(internalObj));

ClassWithPrivateParameterlessCtor privateObj = ClassWithPrivateParameterlessCtor.Instance;
Assert.Equal("{\"Name\":\"InstancePropertyPrivate\"}", JsonSerializer.Serialize(privateObj));

var list = new CollectionWithoutPublicParameterlessCtor(new List<object> { 1, "foo", false });
Assert.Equal("[1,\"foo\",false]", JsonSerializer.Serialize(list));

var envelopeList = new List<object>()
{
ConcreteDerivedClassWithNoPublicDefaultCtor.Error("oops"),
ConcreteDerivedClassWithNoPublicDefaultCtor.Ok<string>(),
ConcreteDerivedClassWithNoPublicDefaultCtor.Ok<int>(),
ConcreteDerivedClassWithNoPublicDefaultCtor.Ok()
};
Assert.Equal("[{\"ErrorString\":\"oops\",\"Result\":null},{\"Result\":null},{\"Result\":0},{\"ErrorString\":\"ok\",\"Result\":null}]", JsonSerializer.Serialize(envelopeList));
}

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

0 comments on commit 0174b98

Please sign in to comment.