JSON serializer should not support non-public properties by default

[YohDeadfall]

While working on #2192 observed that the serializer opt-ins non-public member support. At least it blocks field serialization support from behaving the same way as for properties. In worst case as @Drawaes said the issue leads to data leaks.

The issue exists starting from the first PR of JSON serializer (see #35609).

[ahsonkhan]

While working on #2192 observed that the serializer opt-ins non-public member support.

@YohDeadfall, do you have a test case that shows JSON serializer supporting non-public properties by default for some cases?

I don't believe this is a breaking change. The actual check for whether a property is public and should be supported is here:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs

Lines 48 to 58 in 89965be

	if (propertyInfo.GetMethod?.IsPublic == true)
	{
	HasGetter = true;
	Get = options.MemberAccessorStrategy.CreatePropertyGetter<TClass, TDeclaredProperty>(propertyInfo);
	}
	if (propertyInfo.SetMethod?.IsPublic == true)
	{
	HasSetter = true;
	Set = options.MemberAccessorStrategy.CreatePropertySetter<TClass, TDeclaredProperty>(propertyInfo);
	}

We only support public properties for both serialization and deserialization.

using System;
using System.Text.Json;

namespace OnlyPublicSupportedByDefault
{
    class Program
    {
        static void Main(string[] args)
        {
            var obj = new Accessor
            {
                Doing = 3
            };

            // Outputs {}
            Console.WriteLine(JsonSerializer.Serialize(obj));

            Accessor accessor = JsonSerializer.Deserialize<Accessor>("{\"Doing\": 5, \"Stuff\": \"foo\"}");

            // Outputs -1 (i.e. the default value of the property defined in the class)
            Console.WriteLine(accessor.Doing);
        }
    }

    public class Accessor
    {
        internal int Doing { get; set; } = -1;
        private string Stuff { get; set; } = "one";
    }
}

That said, we should fix the binding flags issue when getting all the properties that you identified. It shouldn't have any user facing impact though or cause the behavior to change in any way.

[YohDeadfall]

Indeed... In that case it's just a perf issue.

[YohDeadfall]

Since there is nothing to fix (the serializer doesn't support private properties), closing this. #2278 removes creation of info for non-public properties.

[YohDeadfall]

@ahsonkhan should this be listed under milestone 5.0?

[ahsonkhan]

I believe so, yes. I don't think any other milestone makes sense here and this issue was created while working on the current milestone.

[YohDeadfall]

Does it mean that to get a list all fixes and changes pull requests should be used instead? Am wondering how to determine was an issue resolved or was wrong without opening it.

[ahsonkhan]

Am wondering how to determine was an issue resolved or was wrong without opening it.

I don't know if there is a way to do this given our current labels/milestones. @danmosemsft, @karelz - what is the recommendation for filtering out issues that were "wrong" (or say "duplicate"), especially when setting the milestone/issue label? Do we need some other labels?

[danmoseley]

We do not use milestones consistently during most of the cycle. We use them more carefully near the end of the cycle when we are carefully gating fixes going into the release, and during servicing processes for the same reason.

To figure out whether a change gets into a release, the most reliable way is to know which branch the release is coming from, or the SHA it was generated from, and see whether the change was in that branch (git branch --contains <commit>)

Right now we haven't released a preview of 5.0 yet, but the first previews will come from master, or close to it. So if the change you want is in master today, it should be in the previews and final release.

I'm not sure of the question though - did that answer it?

[ahsonkhan]

I'm not sure of the question though - did that answer it?

I believe, the question is less about tracking code changes/commits, and more about issues on GitHub themselves. As a contributor, I want to file an issue and I see similar/related issues already closed with the 5.0 milestone. What do I do? Assume the issue I had has been fixed in that milestone? Still open the issue? Sometimes, all the milestone on the issue indicates is during which release the issue was filed and/or closed and doesn't convey any intent around "wont fix", "duplicate", "non-issue", etc.