Newtonsoft.Json icon indicating copy to clipboard operation
Newtonsoft.Json copied to clipboard

Problem with using `StringEnumConverter` with an expression-bodied member

Open silleknarf opened this issue 1 year ago • 1 comments

Source/destination types

enum TestProp { Value1 }

class TestClass
{
    public TestProp TestProp => Child.TestProp;
    public TestChildClass Child { get; set; }
}

class TestChildClass
{
    public TestProp TestProp { get; set; } = TestProp.Value1;
}

Source/destination JSON

{"TestProp":"Value1","Child":{"TestProp":"Value1"}}

Expected behavior

The two tests in the "steps to reproduce" section should pass.

Actual behavior

However It_should_serialize_prop_with_custom_getset fails with the error:

System.NullReferenceException: 'Object reference not set to an instance of an object.'

TestClass.Child.get returned null.

There seems to be a problem with using the StringEnumConverter with an expression-bodied member. The first test below fails but the second one passes.

Steps to reproduce

[Fact]
public void It_should_serialize_prop_with_custom_getset()
{
    var testClass = new TestClass { Child = new TestChildClass() };
    var settings = new JsonSerializerSettings { Converters = new List<JsonConverter> { new StringEnumConverter() } };
    var json = JsonConvert.SerializeObject(testClass, settings);
    // Test fails on the line below.
    var result = JsonConvert.DeserializeObject<TestClass>(json, settings);
}

[Fact]
public void It_should_serialize_prop_with_custom_getset_no_settings()
{
    var testClass = new TestClass { Child = new TestChildClass() };
    var json = JsonConvert.SerializeObject(testClass);
    // No issues if we don't use a `StringEnumConverter`.
    var result = JsonConvert.DeserializeObject<TestClass>(json);
}

silleknarf avatar Jul 11 '22 15:07 silleknarf

The problem is actually in the declaration of the TestClass. Nothing in that class ensures that the Child member is not null when the TestClass.TestProp get accessor is being invoked.

Newtonsoft.Json does not define in which particular order properties of an object are being deserialized, and thus you cannot rely on any particular order of members being populated/assigned in the target object. It is not a bug in Newtonsoft.Json if some code path of some Newtonsoft.Json class tries to access the TestClass.TestProp property, and then the TestClass.TestProp getter tries to access another member that is null. That the It_should_serialize_prop_with_custom_getset_no_settings test case is working is just lucky circumstances.

Who knows what other (de)serialization settings, JsonConverters and/or (custom) ContractResolvers, or perhaps even a future Newtonsoft.Json version might exhibit a similar deserialization behavior as your failing test case using StringEnumConverter. Instead of relying on poorly defined or outright undefined details of the deserialization behavior it is in my opinion a better approach to write the TestClass more robustly so that none of its code attempts to dereference possible null references...

elgonzo avatar Jul 11 '22 16:07 elgonzo