Runtime: JsonSerializer should support private setters as an opt-in feature

Created on 3 Jun 2019  Â·  22Comments  Â·  Source: dotnet/runtime

Updated by @layomia:
There's a lot of overlap between this feature and field support wrt. public API proposal. See https://github.com/dotnet/runtime/issues/34558 for combined API proposal and discussion.


Non-public setters for public properties are not used when deserializing. Other serializers have this feature. As a workaround, user's have to write custom converter's for such POCOs, which isn't trivial in case of a complex object. This feature provides an opt-in for the serializer to use non-public setters.

Enabling non-public getter usage is included for parity with Newtonsoft.Json which supports this when the JsonProperty attribute is used, and to prevent complicating the API surface if this is ever desired in the future.

This feature was scoped out of v1 (3.x), as the objective was to support simple POCOs. We elected to make this feature opt-in due to security concerns with non-public support.

This feature does not include support for non-public properties.

A related feature that allows deserialization of immutable objects through parameterized constructors was recently added in https://github.com/dotnet/runtime/pull/33444.

area-System.Text.Json enhancement json-functionality-doc

Most helpful comment

Or add support for parameters in constructors which Json.Net supports. That would then support immutable classes without the need to have any setters, private or not

All 22 comments

Or add support for parameters in constructors which Json.Net supports. That would then support immutable classes without the need to have any setters, private or not

Deserialisation with constructor parameters would cover both readonly properties and properties with private setters. Private setter deserialisation would be nice too, especially to avoid writing constructors with massive parameter lists.

Can we consider making this a 3.0 milestone? A library with private or internal setters can't deserialize anything without it. If you deserialize something from a source which you do not control, it makes sense to have private or internal setters.

I spent couple weeks trying to debug everything on my side just to be led to this issue. Any ETA on this?

Failing in 4.6.0-preview8.19405.3 with internal setters:

[JsonPropertyName("FolderName")] public string FolderName { get; internal set; }

Works:
[JsonPropertyName("FolderName")] public string FolderName { get; set; }

Cheers

Yes. I am anxiously waiting for a fix.

On Thu, Aug 22, 2019, 06:03 Josep notifications@github.com wrote:

Failing in 4.6.0-preview8.19405.3 with internal setters:

[JsonPropertyName("FolderName")] public string FolderName { get; internal
set; }

Works:
[JsonPropertyName("FolderName")] public string FolderName { get; set; }

Cheers

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/38163?email_source=notifications&email_token=AHCFMKINZGCZ3LFEQQHIP4TQFZP55A5CNFSM4HSH4DC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD44SB4I#issuecomment-523837681,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHCFMKNKEUBNZNOSFBPHSSLQFZP55ANCNFSM4HSH4DCQ
.

Me too!

@steveharter @ahsonkhan Any more info on this issue? Milestone 3.1 perhaps?

I'd be happy to write this and add a PR. Love immutable request objects.

Immutable is a plus, but IMHO it must me reached without attributes.

use [JsonPropertyName("FolderName")] doesn't look a good option to me.

_From @johncrim in dotnet/corefx#42515:_

If a parent object doesn't contain setters for all sub-object properties, the sub-objects are not deserialized. This is inconsistent with other JSON serializers, and seems like a major oversight - current behavior requires a large amount of mutability in the deserialized object model.

It's also a normal pattern to provide sub-objects with default values before deserialization.

For example, in this class, the SubObject1 property won't be deserialized, and no exception is thrown.
```c#
public class Container1
{
// Not deserialized due to missing setter. No exceptions thrown.
public SubObject1 SubObject1 { get; } = new SubObject1();
}

Full test project:
```xml
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <Description>Unit tests demonstrating JsonSerializer bugs</Description>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <Platform>x64</Platform>
    <Platforms>x64</Platforms>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.4.0" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.1" PrivateAssets="All"/>
  </ItemGroup>

</Project>

```c#
using System.Text.Json;
using Xunit;

namespace JsonSerializer_SubObjects
{
public class JsonSerializerTest
{
[Fact] // Fails
public void DeserializesSubObjectsWithoutSetters()
{
const string jsonFooValue = "str1";
var json = "{ \"SubObject1\": { \"Foo\": \"" + jsonFooValue + "\" }}";

        var c1 = JsonSerializer.Deserialize<Container1>(json);

        // BUG dotnet/corefx#1: Value isn't deserialized (this assertion fails)
        Assert.Equal(jsonFooValue, c1.SubObject1.Foo);
        // BUG dotnet/corefx#2: If value can't be deserialized, exception should be thrown
    }

    [Fact] // Passes
    public void DeserializesSubObjectsWithSetters()
    {
        const string jsonFooValue = "str1";
        var json = "{ \"SubObject1\": { \"Foo\": \"" + jsonFooValue + "\" }}";

        var c2 = JsonSerializer.Deserialize<Container2>(json);

        Assert.Equal(jsonFooValue, c2.SubObject1.Foo);
    }

}

public class Container1
{
    // Not deserialized due to no setter. No exceptions thrown.
    public SubObject1 SubObject1 { get; } = new SubObject1();
}

public class Container2
{
    public SubObject1 SubObject1 { get; set; } = new SubObject1();
}

public class SubObject1
{
    public string Foo { get; set; } = "foo";
}

}
```

As far as I'm concerned, this makes System.Text.Json unusable in its current state. Thankfully, we can keep using JSON.NET, but I just wanted to give this a bump, and really encourage this be made a top development priority.

Moving to 5.0.

Please add also list properties (with type List) and only getter

As I commented on dotnet/corefx#42515 - the feature to deserialize into existing sub-objects (which I described in dotnet/corefx#42515) is not the same as supporting private setters. I think the ability to deserialize into existing sub-objects is more important than private setters, but both features are important.

The desired behavior with dotnet/corefx#42515 is that the existing sub-object be deserialized into (the sub-object is obtained from the getter), instead of requiring that the deserializer create a new sub-object, deserialize into it, and then pass it into the parent object setter (whether private or internal, non-existent should also work).

Doing it this way allows developers to set default values before deserializing, to handle values that aren't present in the JSON. It also allows developers to create less mutable objects, and use non-nullable reference types for properties that should never be null.

Completely agree with @chrisdpratt about the library being rather unusable in many regards due to this lack of a feature(s). Honestly, I am a bit surprised that this was included in a release with this basic feature missing, perhaps due to deadlines and time constraints. The API looks very promising, look forward to giving it another try when it is more complete, though do a feel a bit annoyed that it is being touted as a viable replacement of existing libraries in its current state.

work around:
use Obsolete attribute in order to prevent direct code access (serialization is using reflection therefore ignore it)

    private string _microServiceVersion = string.Empty;
    public string MicroServiceVersion
    {
        get => _microServiceVersion;
        [Obsolete("for serialization", true)]
        set => _microServiceVersion = value; 
    }

@bnayae That's not a workaround. The point is to not expose a setter at all.

About the best "workaround" I've found for this is using interfaces. All my public methods return the interface, which only exposes a getter, and then the actual implementation has a setter for the sake of serialization. It doesn't stop someone from casting to the concrete type to access the setter, but using an interface is at least explicit that it should be treated as readonly.

Will this also work for init-only properties?

I could not get init properties to work with the .NET 5 preview 6, so I assumed that they simply aren't supported yet in the preview build. Therefore unfortunately I was unable to test how System.Text.Json interacts with them myself.

This will support init modifier of C# 9?

I could not get init properties to work with the .NET 5 preview 6, so I assumed that they simply aren't supported yet in the preview build. Therefore unfortunately I was unable to test how System.Text.Json interacts with them myself.

Are you referring to using a record type and having the serializer calling the appropriate constructor? If so, that was fixed in preview 8 via https://github.com/dotnet/runtime/issues/38539.

If you just are using the init modifier directly on a property (not a record type) that should have worked in preview 6 since the runtime doesn't prevent reflection or IL Emit'd code from calling the setter AFAIK.

If you just are using the init modifier directly on a property (not a record type) that should have worked in preview 6 since the runtime doesn't prevent reflection or IL Emit'd code from calling the setter AFAIK.

I was running into this issue which has since been resolved: https://stackoverflow.com/q/62648189/3688648

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nalywa picture nalywa  Â·  3Comments

EgorBo picture EgorBo  Â·  3Comments

bencz picture bencz  Â·  3Comments

omajid picture omajid  Â·  3Comments

chunseoklee picture chunseoklee  Â·  3Comments