From ae623164f66cc3f5c2a4a7cf9e29c4b6adaca0e3 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 31 Mar 2020 22:44:56 -0700 Subject: [PATCH 1/3] Add JsonIncludeAttribute & support for non-public property accessors --- .../System.Text.Json/ref/System.Text.Json.cs | 5 + .../src/Resources/Strings.resx | 5 +- .../src/System.Text.Json.csproj | 1 + .../Json/Serialization/JsonClassInfo.Cache.cs | 4 +- .../Text/Json/Serialization/JsonClassInfo.cs | 239 +++++++------ .../Serialization/JsonIncludeAttribute.cs | 21 ++ .../JsonPropertyInfoOfTTypeToConvert.cs | 8 +- .../JsonSerializer.Read.HandlePropertyName.cs | 2 +- .../ReflectionEmitMemberAccessor.cs | 4 +- .../Serialization/ReflectionMemberAccessor.cs | 4 +- .../Text/Json/ThrowHelper.Serialization.cs | 7 + ...pertyVisibilityTests.NonPublicAccessors.cs | 332 ++++++++++++++++++ .../Serialization/PropertyVisibilityTests.cs | 2 +- .../Serialization/TestClasses.Constructor.cs | 20 -- .../tests/Serialization/TestClasses.cs | 22 ++ .../tests/System.Text.Json.Tests.csproj | 1 + 16 files changed, 540 insertions(+), 137 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonIncludeAttribute.cs create mode 100644 src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.NonPublicAccessors.cs diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 5c3e9d81519b8..42363bb46cbbd 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -507,6 +507,11 @@ public sealed partial class JsonIgnoreAttribute : System.Text.Json.Serialization public JsonIgnoreAttribute() { } public System.Text.Json.Serialization.JsonIgnoreCondition Condition { get { throw null; } set { } } } + [System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple = false)] + public sealed partial class JsonIncludeAttribute : System.Text.Json.Serialization.JsonAttribute + { + public JsonIncludeAttribute() { } + } [System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple=false)] public sealed partial class JsonPropertyNameAttribute : System.Text.Json.Serialization.JsonAttribute { diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 4717b348cc5db..efd854c19d3cb 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -512,4 +512,7 @@ Serialization and deserialization of 'System.Type' instances are not supported and should be avoided since they can lead to security issues. - + + The non-public property '{0}' on type '{1}' is annotated with 'JsonIncludeAttribute' which is invalid. + + \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 94c7b47f1b0c6..841c64e9c81d8 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -140,6 +140,7 @@ + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs index 330e3052f19c0..0463a31565dfb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs @@ -77,7 +77,7 @@ public Dictionary CreateParameterCache(int capacity, } } - public static JsonPropertyInfo AddProperty(Type propertyType, PropertyInfo propertyInfo, Type parentClassType, JsonSerializerOptions options) + public static JsonPropertyInfo AddProperty(PropertyInfo propertyInfo, Type parentClassType, JsonSerializerOptions options) { JsonIgnoreAttribute? ignoreAttribute = JsonPropertyInfo.GetAttribute(propertyInfo); if (ignoreAttribute?.Condition == JsonIgnoreCondition.Always) @@ -85,6 +85,8 @@ public static JsonPropertyInfo AddProperty(Type propertyType, PropertyInfo prope return JsonPropertyInfo.CreateIgnoredPropertyPlaceholder(propertyInfo, options); } + Type propertyType = propertyInfo.PropertyType; + JsonConverter converter = GetConverter( propertyType, parentClassType, diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index 362614918cdcd..da21c2ae62c19 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -95,69 +95,10 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) { case ClassType.Object: { - // Create the policy property. - PropertyInfoForClassInfo = CreatePropertyInfoForClassInfo(type, runtimeType, converter!, options); - CreateObject = options.MemberAccessorStrategy.CreateConstructor(type); - PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public); - - Dictionary cache = CreatePropertyCache(properties.Length); - - foreach (PropertyInfo propertyInfo in properties) - { - // Ignore indexers - if (propertyInfo.GetIndexParameters().Length > 0) - { - continue; - } - - // For now we only support public getters\setters - if (propertyInfo.GetMethod?.IsPublic == true || - propertyInfo.SetMethod?.IsPublic == true) - { - JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo.PropertyType, propertyInfo, type, options); - Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); - - // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. - if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) - { - JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; - - if (other.ShouldDeserialize == false && other.ShouldSerialize == false) - { - // Overwrite the one just added since it has [JsonIgnore]. - cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; - } - else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true) - { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); - } - // else ignore jsonPropertyInfo since it has [JsonIgnore]. - } - } - } - - JsonPropertyInfo[] cacheArray; - if (DetermineExtensionDataProperty(cache)) - { - // Remove from cache since it is handled independently. - cache.Remove(DataExtensionProperty!.NameAsString!); - - cacheArray = new JsonPropertyInfo[cache.Count + 1]; - - // Set the last element to the extension property. - cacheArray[cache.Count] = DataExtensionProperty; - } - else - { - cacheArray = new JsonPropertyInfo[cache.Count]; - } - - // Set fields when finished to avoid concurrency issues. - PropertyCache = cache; - cache.Values.CopyTo(cacheArray, 0); - PropertyCacheArray = cacheArray; + HandlePublicProperties(); + HandleNonPublicProperties(); if (converter.ConstructorIsParameterized) { @@ -189,6 +130,90 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) } } + private void HandlePublicProperties() + { + PropertyInfo[] properties = Type.GetProperties(BindingFlags.Instance | BindingFlags.Public); + + Dictionary cache = CreatePropertyCache(properties.Length); + + foreach (PropertyInfo propertyInfo in properties) + { + // Ignore indexers + if (propertyInfo.GetIndexParameters().Length > 0) + { + continue; + } + + // For now we only support public getters\setters + if (propertyInfo.GetMethod?.IsPublic == true || + propertyInfo.SetMethod?.IsPublic == true) + { + JsonPropertyInfo jsonPropertyInfo = AddProperty( + propertyInfo, + Type, + Options); + Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); + + // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. + if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) + { + JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; + + if (other.ShouldDeserialize == false && other.ShouldSerialize == false) + { + // Overwrite the one just added since it has [JsonIgnore]. + cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; + } + else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true) + { + ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); + } + // else ignore jsonPropertyInfo since it has [JsonIgnore]. + } + } + } + + JsonPropertyInfo[] cacheArray; + if (DetermineExtensionDataProperty(cache)) + { + // Remove from cache since it is handled independently. + cache.Remove(DataExtensionProperty!.NameAsString!); + + cacheArray = new JsonPropertyInfo[cache.Count + 1]; + + // Set the last element to the extension property. + cacheArray[cache.Count] = DataExtensionProperty; + } + else + { + cacheArray = new JsonPropertyInfo[cache.Count]; + } + + // Set fields when finished to avoid concurrency issues. + PropertyCache = cache; + cache.Values.CopyTo(cacheArray, 0); + PropertyCacheArray = cacheArray; + } + + private void HandleNonPublicProperties() + { + PropertyInfo[] properties = Type.GetProperties(BindingFlags.Instance | BindingFlags.NonPublic); + + foreach (PropertyInfo propertyInfo in properties) + { + // Ignore indexers + if (propertyInfo.GetIndexParameters().Length > 0) + { + continue; + } + + if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) + { + ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); + } + } + } + private void InitializeConstructorParameters(ConstructorInfo constructorInfo) { ParameterInfo[] parameters = constructorInfo!.GetParameters(); @@ -254,52 +279,6 @@ private void InitializeConstructorParameters(ConstructorInfo constructorInfo) PropertyCache = propertyCache; } - public bool DetermineExtensionDataProperty(Dictionary cache) - { - JsonPropertyInfo? jsonPropertyInfo = GetPropertyWithUniqueAttribute(Type, typeof(JsonExtensionDataAttribute), cache); - if (jsonPropertyInfo != null) - { - Type declaredPropertyType = jsonPropertyInfo.DeclaredPropertyType; - if (typeof(IDictionary).IsAssignableFrom(declaredPropertyType) || - typeof(IDictionary).IsAssignableFrom(declaredPropertyType)) - { - JsonConverter converter = Options.GetConverter(declaredPropertyType); - Debug.Assert(converter != null); - } - else - { - ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type, jsonPropertyInfo); - } - - DataExtensionProperty = jsonPropertyInfo; - return true; - } - - return false; - } - - private static JsonPropertyInfo? GetPropertyWithUniqueAttribute(Type classType, Type attributeType, Dictionary cache) - { - JsonPropertyInfo? property = null; - - foreach (JsonPropertyInfo jsonPropertyInfo in cache.Values) - { - Debug.Assert(jsonPropertyInfo.PropertyInfo != null); - Attribute? attribute = jsonPropertyInfo.PropertyInfo.GetCustomAttribute(attributeType); - if (attribute != null) - { - if (property != null) - { - ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(classType, attributeType); - } - - property = jsonPropertyInfo; - } - } - - return property; - } - private static JsonParameterInfo AddConstructorParameter( ParameterInfo parameterInfo, JsonPropertyInfo jsonPropertyInfo, @@ -382,5 +361,51 @@ public static JsonConverter GetConverter( return converter; } + + public bool DetermineExtensionDataProperty(Dictionary cache) + { + JsonPropertyInfo? jsonPropertyInfo = GetPropertyWithUniqueAttribute(Type, typeof(JsonExtensionDataAttribute), cache); + if (jsonPropertyInfo != null) + { + Type declaredPropertyType = jsonPropertyInfo.DeclaredPropertyType; + if (typeof(IDictionary).IsAssignableFrom(declaredPropertyType) || + typeof(IDictionary).IsAssignableFrom(declaredPropertyType)) + { + JsonConverter converter = Options.GetConverter(declaredPropertyType); + Debug.Assert(converter != null); + } + else + { + ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type, jsonPropertyInfo); + } + + DataExtensionProperty = jsonPropertyInfo; + return true; + } + + return false; + } + + private static JsonPropertyInfo? GetPropertyWithUniqueAttribute(Type classType, Type attributeType, Dictionary cache) + { + JsonPropertyInfo? property = null; + + foreach (JsonPropertyInfo jsonPropertyInfo in cache.Values) + { + Debug.Assert(jsonPropertyInfo.PropertyInfo != null); + Attribute? attribute = jsonPropertyInfo.PropertyInfo.GetCustomAttribute(attributeType); + if (attribute != null) + { + if (property != null) + { + ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(classType, attributeType); + } + + property = jsonPropertyInfo; + } + } + + return property; + } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonIncludeAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonIncludeAttribute.cs new file mode 100644 index 0000000000000..0e1727d683a56 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonIncludeAttribute.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace System.Text.Json.Serialization +{ + /// + /// Indicates that the member should be included for serialization and deserialization. + /// + /// + /// When applied to a property, indicates that non-public getters and setters can be used for serialization and deserialization. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)] + public sealed class JsonIncludeAttribute : JsonAttribute + { + /// + /// Initializes a new instance of . + /// + public JsonIncludeAttribute() { } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs index 1ceb16dc7700f..8c6a3d313953e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs @@ -38,13 +38,17 @@ public override void Initialize( if (propertyInfo != null) { - if (propertyInfo.GetMethod?.IsPublic == true) + bool useNonPublicAccessors = GetAttribute(propertyInfo) != null; + + MethodInfo? getMethod = propertyInfo.GetMethod; + if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) { HasGetter = true; Get = options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); } - if (propertyInfo.SetMethod?.IsPublic == true) + MethodInfo? setMethod = propertyInfo.SetMethod; + if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) { HasSetter = true; Set = options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index 3def5052f8191..a59a14fd63489 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -38,7 +38,7 @@ internal static JsonPropertyInfo LookupProperty( if (jsonPropertyInfo == JsonPropertyInfo.s_missingProperty) { JsonPropertyInfo? dataExtProperty = state.Current.JsonClassInfo.DataExtensionProperty; - if (dataExtProperty != null) + if (dataExtProperty != null && dataExtProperty.HasGetter && dataExtProperty.HasSetter) { state.Current.JsonPropertyNameAsString = JsonHelpers.Utf8GetString(unescapedPropertyName); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs index 3e34ada077cda..b6fe68e3bb536 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs @@ -232,7 +232,7 @@ public override Func>, TCollection> C private static Delegate CreatePropertyGetter(PropertyInfo propertyInfo, Type classType, Type propertyType) { - MethodInfo? realMethod = propertyInfo.GetGetMethod(); + MethodInfo? realMethod = propertyInfo.GetMethod; Type objectType = typeof(object); Debug.Assert(realMethod != null); @@ -268,7 +268,7 @@ private static Delegate CreatePropertyGetter(PropertyInfo propertyInfo, Type cla private static Delegate CreatePropertySetter(PropertyInfo propertyInfo, Type classType, Type propertyType) { - MethodInfo? realMethod = propertyInfo.GetSetMethod(); + MethodInfo? realMethod = propertyInfo.SetMethod; Type objectType = typeof(object); Debug.Assert(realMethod != null); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs index db8e8573b3935..1943dc5632894 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs @@ -145,7 +145,7 @@ public override Func>, TCollection> C public override Func CreatePropertyGetter(PropertyInfo propertyInfo) { - MethodInfo getMethodInfo = propertyInfo.GetGetMethod()!; + MethodInfo getMethodInfo = propertyInfo.GetMethod!; return delegate (object obj) { @@ -155,7 +155,7 @@ public override Func CreatePropertyGetter(Property public override Action CreatePropertySetter(PropertyInfo propertyInfo) { - MethodInfo setMethodInfo = propertyInfo.GetSetMethod()!; + MethodInfo setMethodInfo = propertyInfo.SetMethod!; return delegate (object obj, TProperty value) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index f643ffbff8f63..f36130cba423d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -203,6 +203,13 @@ public static void ThrowInvalidOperationException_ExtensionDataCannotBindToCtorP throw new InvalidOperationException(SR.Format(SR.ExtensionDataCannotBindToCtorParam, propertyInfo, classType, constructorInfo)); } + [DoesNotReturn] + [MethodImpl(MethodImplOptions.NoInlining)] + public static void ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(PropertyInfo propertyInfo, Type parentType) + { + throw new InvalidOperationException(SR.Format(SR.JsonIncludeOnNonPublicInvalid, propertyInfo.Name, parentType)); + } + [DoesNotReturn] [MethodImpl(MethodImplOptions.NoInlining)] public static void ThrowNotSupportedException_ObjectWithParameterizedCtorRefMetadataNotHonored( diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.NonPublicAccessors.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.NonPublicAccessors.cs new file mode 100644 index 0000000000000..de7e206c5a953 --- /dev/null +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.NonPublicAccessors.cs @@ -0,0 +1,332 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Generic; +using Xunit; + +namespace System.Text.Json.Serialization.Tests +{ + public static partial class PropertyVisibilityTests + { + [Fact] + public static void NonPublic_AccessorsNotSupported_WithoutAttribute() + { + string json = @"{ + ""MyInt"":1, + ""MyString"":""Hello"", + ""MyFloat"":2, + ""MyUri"":""https://microsoft.com"" + }"; + + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(0, obj.MyInt); + Assert.Null(obj.MyString); + Assert.Equal(2f, obj.GetMyFloat); + Assert.Equal(new Uri("https://microsoft.com"), obj.MyUri); + + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""MyInt"":0", json); + Assert.Contains(@"""MyString"":null", json); + Assert.DoesNotContain(@"""MyFloat"":", json); + Assert.DoesNotContain(@"""MyUri"":", json); + } + + private class MyClass_WithNonPublicAccessors + { + public int MyInt { get; private set; } + public string MyString { get; internal set; } + public float MyFloat { private get; set; } + public Uri MyUri { internal get; set; } + + // For test validation. + internal float GetMyFloat => MyFloat; + } + + [Fact] + public static void Honor_JsonSerializablePropertyAttribute_OnProperties() + { + string json = @"{ + ""MyInt"":1, + ""MyString"":""Hello"", + ""MyFloat"":2, + ""MyUri"":""https://microsoft.com"" + }"; + + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(1, obj.MyInt); + Assert.Equal("Hello", obj.MyString); + Assert.Equal(2f, obj.GetMyFloat); + Assert.Equal(new Uri("https://microsoft.com"), obj.MyUri); + + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""MyInt"":1", json); + Assert.Contains(@"""MyString"":""Hello""", json); + Assert.Contains(@"""MyFloat"":2", json); + Assert.Contains(@"""MyUri"":""https://microsoft.com""", json); + } + + private class MyClass_WithNonPublicAccessors_WithPropertyAttributes + { + [JsonInclude] + public int MyInt { get; private set; } + [JsonInclude] + public string MyString { get; internal set; } + [JsonInclude] + public float MyFloat { private get; set; } + [JsonInclude] + public Uri MyUri { internal get; set; } + + // For test validation. + internal float GetMyFloat => MyFloat; + } + + private class MyClass_WithNonPublicAccessors_WithPropertyAttributes_And_PropertyIgnore + { + [JsonInclude] + [JsonIgnore] + public int MyInt { get; private set; } + + [JsonInclude] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenNull)] + public string MyString { get; internal set; } = "DefaultString"; + + [JsonInclude] + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + public float MyFloat { private get; set; } + + [JsonInclude] + [JsonIgnore(Condition = JsonIgnoreCondition.Never)] + public Uri MyUri { internal get; set; } + + // For test validation. + internal float GetMyFloat => MyFloat; + } + + [Fact] + public static void ExtensionDataCanHaveNonPublicSetter() + { + string json = @"{""Key"":""Value""}"; + + // Baseline + var obj1 = JsonSerializer.Deserialize(json); + Assert.Null(obj1.ExtensionData); + Assert.Equal("{}", JsonSerializer.Serialize(obj1)); + + // With attribute + var obj2 = JsonSerializer.Deserialize(json); + Assert.Equal("Value", obj2.ExtensionData["Key"].GetString()); + Assert.Equal(json, JsonSerializer.Serialize(obj2)); + } + + private class ClassWithExtensionData_NonPublicSetter + { + [JsonExtensionData] + public Dictionary ExtensionData { get; private set; } + } + + private class ClassWithExtensionData_NonPublicSetter_WithAttribute + { + [JsonExtensionData] + [JsonInclude] + public Dictionary ExtensionData { get; private set; } + } + + private class ClassWithExtensionData_NonPublicGetter + { + [JsonExtensionData] + public Dictionary ExtensionData { internal get; set; } + } + + [Fact] + public static void HonorCustomConverter() + { + var options = new JsonSerializerOptions(); + options.Converters.Add(new JsonStringEnumConverter()); + + string json = @"{""MyEnum"":""AnotherValue"",""MyInt"":2}"; + + // Deserialization baseline, without enum converter, we get JsonException. + Assert.Throws(() => JsonSerializer.Deserialize(json)); + + var obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(MySmallEnum.AnotherValue, obj.GetMyEnum); + Assert.Equal(25, obj.MyInt); + + // ConverterForInt32 throws this exception. + Assert.Throws(() => JsonSerializer.Serialize(obj, options)); + } + + private struct StructWithPropertiesWithConverter + { + [JsonInclude] + public MySmallEnum MyEnum { private get; set; } + + [JsonInclude] + [JsonConverter(typeof(ConverterForInt32))] + public int MyInt { get; private set; } + + // For test validation. + internal MySmallEnum GetMyEnum => MyEnum; + } + + private enum MySmallEnum + { + DefaultValue = 0, + AnotherValue = 1 + } + + [Fact] + public static void HonorCaseInsensitivity() + { + var options = new JsonSerializerOptions { PropertyNameCaseInsensitive = true }; + + string json = @"{""MYSTRING"":""Hello""}"; + Assert.Null(JsonSerializer.Deserialize(json).MyString); + Assert.Equal("Hello", JsonSerializer.Deserialize(json, options).MyString); + } + + private struct MyStruct_WithNonPublicAccessors_WithTypeAttribute + { + [JsonInclude] + public int MyInt { get; private set; } + [JsonInclude] + public string MyString { get; internal set; } + [JsonInclude] + public float MyFloat { private get; set; } + [JsonInclude] + public Uri MyUri { internal get; set; } + + // For test validation. + internal float GetMyFloat => MyFloat; + } + + [Fact] + public static void HonorNamingPolicy() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = new SimpleSnakeCasePolicy() }; + + string json = @"{""my_string"":""Hello""}"; + Assert.Null(JsonSerializer.Deserialize(json).MyString); + Assert.Equal("Hello", JsonSerializer.Deserialize(json, options).MyString); + } + + [Fact] + public static void HonorJsonPropertyName() + { + string json = @"{""prop1"":1,""prop2"":2}"; + + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(MySmallEnum.AnotherValue, obj.GetMyEnum); + Assert.Equal(2, obj.MyInt); + + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""prop1"":1", json); + Assert.Contains(@"""prop2"":2", json); + } + + private struct StructWithPropertiesWithJsonPropertyName + { + [JsonInclude] + [JsonPropertyName("prop1")] + public MySmallEnum MyEnum { private get; set; } + + [JsonInclude] + [JsonPropertyName("prop2")] + public int MyInt { get; private set; } + + // For test validation. + internal MySmallEnum GetMyEnum => MyEnum; + } + + [Fact] + public static void Map_JsonSerializableProperties_ToCtorArgs() + { + var obj = JsonSerializer.Deserialize(@"{""X"":1,""Y"":2}"); + Assert.Equal(1, obj.X); + Assert.Equal(2, obj.GetY); + } + + private struct PointWith_JsonSerializableProperties + { + [JsonInclude] + public int X { get; internal set; } + [JsonInclude] + public int Y { internal get; set; } + + internal int GetY => Y; + + [JsonConstructor] + public PointWith_JsonSerializableProperties(int x, int y) => (X, Y) = (x, y); + } + + [Fact] + public static void Public_And_NonPublicPropertyAccessors_PropertyAttributes() + { + string json = @"{""W"":1,""X"":2,""Y"":3,""Z"":4}"; + + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(1, obj.W); + Assert.Equal(2, obj.X); + Assert.Equal(3, obj.Y); + Assert.Equal(4, obj.GetZ); + + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""W"":1", json); + Assert.Contains(@"""X"":2", json); + Assert.Contains(@"""Y"":3", json); + Assert.Contains(@"""Z"":4", json); + } + + private class ClassWithMixedPropertyAccessors_PropertyAttributes + { + [JsonInclude] + public int W { get; set; } + [JsonInclude] + public int X { get; internal set; } + [JsonInclude] + public int Y { get; set; } + [JsonInclude] + public int Z { private get; set; } + + internal int GetZ => Z; + } + + [Theory] + [InlineData(typeof(ClassWithPrivateProperty_WithJsonIncludeProperty))] + [InlineData(typeof(ClassWithInternalProperty_WithJsonIncludeProperty))] + [InlineData(typeof(ClassWithProtectedProperty_WithJsonIncludeProperty))] + public static void NonPublicProperty_WithJsonInclude_Invalid(Type type) + { + InvalidOperationException ex = Assert.Throws(() => JsonSerializer.Deserialize("", type)); + string exAsStr = ex.ToString(); + Assert.Contains("MyString", exAsStr); + Assert.Contains(type.ToString(), exAsStr); + Assert.Contains("JsonIncludeAttribute", exAsStr); + + ex = Assert.Throws(() => JsonSerializer.Serialize(Activator.CreateInstance(type), type)); + exAsStr = ex.ToString(); + Assert.Contains("MyString", exAsStr); + Assert.Contains(type.ToString(), exAsStr); + Assert.Contains("JsonIncludeAttribute", exAsStr); + } + + private class ClassWithPrivateProperty_WithJsonIncludeProperty + { + [JsonInclude] + private string MyString { get; set; } + } + + private class ClassWithInternalProperty_WithJsonIncludeProperty + { + [JsonInclude] + internal string MyString { get; } + } + + private class ClassWithProtectedProperty_WithJsonIncludeProperty + { + [JsonInclude] + protected string MyString { get; private set; } + } + } +} diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs index d7b4791b5ad75..e9f45a46a552e 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs @@ -9,7 +9,7 @@ namespace System.Text.Json.Serialization.Tests { - public static class PropertyVisibilityTests + public static partial class PropertyVisibilityTests { [Fact] public static void NoSetter() diff --git a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.Constructor.cs b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.Constructor.cs index 5358f7e61501b..8fce473d62993 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.Constructor.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.Constructor.cs @@ -1215,18 +1215,6 @@ public override void Write(Utf8JsonWriter writer, Point_3D value, JsonSerializer throw new NotImplementedException(); } } - public class ConverterForInt32 : JsonConverter - { - public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - return 25; - } - - public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) - { - throw new NotImplementedException(); - } - } public class Class_With_Ctor_With_64_Params : ITestClass { @@ -2124,14 +2112,6 @@ public override string ConvertName(string name) } } - public class SimpleSnakeCasePolicy : JsonNamingPolicy - { - public override string ConvertName(string name) - { - return string.Concat(name.Select((x, i) => i > 0 && char.IsUpper(x) ? "_" + x.ToString() : x.ToString())).ToLower(); - } - } - public class Point_With_Array : ITestClass { public int X { get; } diff --git a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs index 9be040589afe2..0eca0ba16706f 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs @@ -5,6 +5,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -1858,4 +1859,25 @@ public class ClassWithType { public Type Type { get; set; } } + + public class ConverterForInt32 : JsonConverter + { + public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return 25; + } + + public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) + { + throw new NotImplementedException(); + } + } + + public class SimpleSnakeCasePolicy : JsonNamingPolicy + { + public override string ConvertName(string name) + { + return string.Concat(name.Select((x, i) => i > 0 && char.IsUpper(x) ? "_" + x.ToString() : x.ToString())).ToLower(); + } + } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj index a0b35dc132747..e50cb2b34a512 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj @@ -82,6 +82,7 @@ + From 89ad8cf757b84d680fc4d666d693b32b2faa90ec Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 13 Apr 2020 12:51:23 -0700 Subject: [PATCH 2/3] Address review feedback --- .../Text/Json/Serialization/JsonClassInfo.cs | 253 +++++++++--------- 1 file changed, 121 insertions(+), 132 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index da21c2ae62c19..dd93db5aa6fd7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -97,8 +97,81 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) { CreateObject = options.MemberAccessorStrategy.CreateConstructor(type); - HandlePublicProperties(); - HandleNonPublicProperties(); + PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public); + + Dictionary cache = CreatePropertyCache(properties.Length); + + foreach (PropertyInfo propertyInfo in properties) + { + // Ignore indexers + if (propertyInfo.GetIndexParameters().Length > 0) + { + continue; + } + + // For now we only support public getters\setters + if (propertyInfo.GetMethod?.IsPublic == true || + propertyInfo.SetMethod?.IsPublic == true) + { + JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo, type, options); + Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); + + // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. + if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) + { + JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; + + if (other.ShouldDeserialize == false && other.ShouldSerialize == false) + { + // Overwrite the one just added since it has [JsonIgnore]. + cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; + } + else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true) + { + ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); + } + // else ignore jsonPropertyInfo since it has [JsonIgnore]. + } + } + } + + JsonPropertyInfo[] cacheArray; + if (DetermineExtensionDataProperty(cache)) + { + // Remove from cache since it is handled independently. + cache.Remove(DataExtensionProperty!.NameAsString!); + + cacheArray = new JsonPropertyInfo[cache.Count + 1]; + + // Set the last element to the extension property. + cacheArray[cache.Count] = DataExtensionProperty; + } + else + { + cacheArray = new JsonPropertyInfo[cache.Count]; + } + + // Set fields when finished to avoid concurrency issues. + PropertyCache = cache; + cache.Values.CopyTo(cacheArray, 0); + PropertyCacheArray = cacheArray; + + // Ensure `[JsonInclude]` is not used on non-public properties. + properties = type.GetProperties(BindingFlags.Instance | BindingFlags.NonPublic); + + foreach (PropertyInfo propertyInfo in properties) + { + // Ignore indexers + if (propertyInfo.GetIndexParameters().Length > 0) + { + continue; + } + + if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) + { + ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); + } + } if (converter.ConstructorIsParameterized) { @@ -130,90 +203,6 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) } } - private void HandlePublicProperties() - { - PropertyInfo[] properties = Type.GetProperties(BindingFlags.Instance | BindingFlags.Public); - - Dictionary cache = CreatePropertyCache(properties.Length); - - foreach (PropertyInfo propertyInfo in properties) - { - // Ignore indexers - if (propertyInfo.GetIndexParameters().Length > 0) - { - continue; - } - - // For now we only support public getters\setters - if (propertyInfo.GetMethod?.IsPublic == true || - propertyInfo.SetMethod?.IsPublic == true) - { - JsonPropertyInfo jsonPropertyInfo = AddProperty( - propertyInfo, - Type, - Options); - Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); - - // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. - if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) - { - JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; - - if (other.ShouldDeserialize == false && other.ShouldSerialize == false) - { - // Overwrite the one just added since it has [JsonIgnore]. - cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; - } - else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true) - { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); - } - // else ignore jsonPropertyInfo since it has [JsonIgnore]. - } - } - } - - JsonPropertyInfo[] cacheArray; - if (DetermineExtensionDataProperty(cache)) - { - // Remove from cache since it is handled independently. - cache.Remove(DataExtensionProperty!.NameAsString!); - - cacheArray = new JsonPropertyInfo[cache.Count + 1]; - - // Set the last element to the extension property. - cacheArray[cache.Count] = DataExtensionProperty; - } - else - { - cacheArray = new JsonPropertyInfo[cache.Count]; - } - - // Set fields when finished to avoid concurrency issues. - PropertyCache = cache; - cache.Values.CopyTo(cacheArray, 0); - PropertyCacheArray = cacheArray; - } - - private void HandleNonPublicProperties() - { - PropertyInfo[] properties = Type.GetProperties(BindingFlags.Instance | BindingFlags.NonPublic); - - foreach (PropertyInfo propertyInfo in properties) - { - // Ignore indexers - if (propertyInfo.GetIndexParameters().Length > 0) - { - continue; - } - - if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) - { - ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); - } - } - } - private void InitializeConstructorParameters(ConstructorInfo constructorInfo) { ParameterInfo[] parameters = constructorInfo!.GetParameters(); @@ -279,6 +268,52 @@ private void InitializeConstructorParameters(ConstructorInfo constructorInfo) PropertyCache = propertyCache; } + public bool DetermineExtensionDataProperty(Dictionary cache) + { + JsonPropertyInfo? jsonPropertyInfo = GetPropertyWithUniqueAttribute(Type, typeof(JsonExtensionDataAttribute), cache); + if (jsonPropertyInfo != null) + { + Type declaredPropertyType = jsonPropertyInfo.DeclaredPropertyType; + if (typeof(IDictionary).IsAssignableFrom(declaredPropertyType) || + typeof(IDictionary).IsAssignableFrom(declaredPropertyType)) + { + JsonConverter converter = Options.GetConverter(declaredPropertyType); + Debug.Assert(converter != null); + } + else + { + ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type, jsonPropertyInfo); + } + + DataExtensionProperty = jsonPropertyInfo; + return true; + } + + return false; + } + + private static JsonPropertyInfo? GetPropertyWithUniqueAttribute(Type classType, Type attributeType, Dictionary cache) + { + JsonPropertyInfo? property = null; + + foreach (JsonPropertyInfo jsonPropertyInfo in cache.Values) + { + Debug.Assert(jsonPropertyInfo.PropertyInfo != null); + Attribute? attribute = jsonPropertyInfo.PropertyInfo.GetCustomAttribute(attributeType); + if (attribute != null) + { + if (property != null) + { + ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(classType, attributeType); + } + + property = jsonPropertyInfo; + } + } + + return property; + } + private static JsonParameterInfo AddConstructorParameter( ParameterInfo parameterInfo, JsonPropertyInfo jsonPropertyInfo, @@ -361,51 +396,5 @@ public static JsonConverter GetConverter( return converter; } - - public bool DetermineExtensionDataProperty(Dictionary cache) - { - JsonPropertyInfo? jsonPropertyInfo = GetPropertyWithUniqueAttribute(Type, typeof(JsonExtensionDataAttribute), cache); - if (jsonPropertyInfo != null) - { - Type declaredPropertyType = jsonPropertyInfo.DeclaredPropertyType; - if (typeof(IDictionary).IsAssignableFrom(declaredPropertyType) || - typeof(IDictionary).IsAssignableFrom(declaredPropertyType)) - { - JsonConverter converter = Options.GetConverter(declaredPropertyType); - Debug.Assert(converter != null); - } - else - { - ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type, jsonPropertyInfo); - } - - DataExtensionProperty = jsonPropertyInfo; - return true; - } - - return false; - } - - private static JsonPropertyInfo? GetPropertyWithUniqueAttribute(Type classType, Type attributeType, Dictionary cache) - { - JsonPropertyInfo? property = null; - - foreach (JsonPropertyInfo jsonPropertyInfo in cache.Values) - { - Debug.Assert(jsonPropertyInfo.PropertyInfo != null); - Attribute? attribute = jsonPropertyInfo.PropertyInfo.GetCustomAttribute(attributeType); - if (attribute != null) - { - if (property != null) - { - ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(classType, attributeType); - } - - property = jsonPropertyInfo; - } - } - - return property; - } } } From 305f8f375e2b9fd521a7d7f503726f6b6451c809 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 13 Apr 2020 14:27:42 -0700 Subject: [PATCH 3/3] Remove extra reflection call for non-public properties --- .../Text/Json/Serialization/JsonClassInfo.cs | 37 ++++++++++--------- .../Serialization/PropertyVisibilityTests.cs | 24 ++++++++++++ 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index dd93db5aa6fd7..5c7f0f8dfc3f8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -97,7 +97,7 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) { CreateObject = options.MemberAccessorStrategy.CreateConstructor(type); - PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public); + PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); Dictionary cache = CreatePropertyCache(properties.Length); @@ -109,6 +109,17 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) continue; } + if (IsNonPublicProperty(propertyInfo)) + { + if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) + { + ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); + } + + // Non-public properties should not be included for (de)serialization. + continue; + } + // For now we only support public getters\setters if (propertyInfo.GetMethod?.IsPublic == true || propertyInfo.SetMethod?.IsPublic == true) @@ -156,23 +167,6 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) cache.Values.CopyTo(cacheArray, 0); PropertyCacheArray = cacheArray; - // Ensure `[JsonInclude]` is not used on non-public properties. - properties = type.GetProperties(BindingFlags.Instance | BindingFlags.NonPublic); - - foreach (PropertyInfo propertyInfo in properties) - { - // Ignore indexers - if (propertyInfo.GetIndexParameters().Length > 0) - { - continue; - } - - if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) - { - ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); - } - } - if (converter.ConstructorIsParameterized) { InitializeConstructorParameters(converter.ConstructorInfo!); @@ -203,6 +197,13 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) } } + private static bool IsNonPublicProperty(PropertyInfo propertyInfo) + { + MethodInfo? getMethod = propertyInfo.GetMethod; + MethodInfo? setMethod = propertyInfo.SetMethod; + return !((getMethod != null && getMethod.IsPublic) || (setMethod != null && setMethod.IsPublic)); + } + private void InitializeConstructorParameters(ConstructorInfo constructorInfo) { ParameterInfo[] parameters = constructorInfo!.GetParameters(); diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs index e9f45a46a552e..a60565b257112 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs @@ -781,5 +781,29 @@ public class ClassUsingIgnoreNeverAttribute [JsonIgnore(Condition = JsonIgnoreCondition.Never)] public Dictionary Dictionary { get; set; } = new Dictionary { ["Key"] = "Value" }; } + + [Fact] + public static void NonPublicMembersAreNotIncluded() + { + Assert.Equal("{}", JsonSerializer.Serialize(new ClassWithNonPublicProperties())); + + string json = @"{""MyInt"":1,""MyString"":""Hello"",""MyFloat"":2,""MyDouble"":3}"; + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(0, obj.MyInt); + Assert.Null(obj.MyString); + Assert.Equal(0, obj.GetMyFloat); + Assert.Equal(0, obj.GetMyDouble); + } + + private class ClassWithNonPublicProperties + { + internal int MyInt { get; set; } + internal string MyString { get; private set; } + internal float MyFloat { private get; set; } + private double MyDouble { get; set; } + + internal float GetMyFloat => MyFloat; + internal double GetMyDouble => MyDouble; + } } }