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

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

Merged
merged 4 commits into from Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
Expand Up @@ -2148,12 +2148,61 @@ public static void VerifyIDictionaryWithNonStringKey()
public class ClassWithoutParameterlessCtor
{
public ClassWithoutParameterlessCtor(int num) { }
public string Name { get; set; }
}

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

public 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);
}
}
}
154 changes: 147 additions & 7 deletions src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs
Expand Up @@ -2,6 +2,7 @@
// 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.IO;
using Xunit;
Expand Down Expand Up @@ -213,22 +214,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]"));

ahsonkhan marked this conversation as resolved.
Show resolved Hide resolved
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<GenericClassWithProtectedInternalCtor<string>>("{\"Result\":null}"));
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<ConcreteDerivedClassWithNoPublicDefaultCtor>("{\"ErrorString\":\"oops\"}"));
}

class PublicParameterizedConstructorTestClass
public class PublicParameterizedConstructorTestClass
ahsonkhan marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly string _name;
public PublicParameterizedConstructorTestClass(string name)
{
_name = name;
throw new InvalidOperationException();
ahsonkhan marked this conversation as resolved.
Show resolved Hide resolved
}

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

public string Name { get; }

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

public class ClassWithInternalParameterlessCtor
{
internal ClassWithInternalParameterlessCtor()
{
throw new InvalidOperationException();
ahsonkhan marked this conversation as resolved.
Show resolved Hide resolved
}
public string Name

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

public string Name { get; set; }

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

public class ClassWithPrivateParameterlessCtor
{
private ClassWithPrivateParameterlessCtor()
{
get { return _name; }
throw new InvalidOperationException();
}

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

public string Name { get; set; }

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

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

internal CollectionWithoutPublicParameterlessCtor()
{
throw new InvalidOperationException();
}

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();
}
}

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

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

public 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 +548,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
Expand Up @@ -76,6 +76,31 @@ private class Indexer
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