JsonConvert.DeserializeObject<JToken> creates "undefined" literal rather than null for property with missing value

[Ansssss]

Steps to reproduce

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
public class Program
{
    public static void Main()
    {
        var expectedOutput = JsonConvert.DeserializeObject<JToken>("{ \"prop1\": , \"prop2\": 1 }");
        Console.WriteLine(expectedOutput.ToString());
    }
}

Expected behavior

In version 13.0.1, the output was

{
    "prop1": null,
    "prop2": 1
}

Actual behavior

but now in 13.0.2 the output is

{
    "prop1": undefined,
    "prop2": 1
}

Possibly related issues

https://github.com/JamesNK/Newtonsoft.Json/issues/2133 https://github.com/JamesNK/Newtonsoft.Json/issues/2274

[elgonzo]

I kind of suspect this being a deliberate change and not a bug (although the release notes not mentioning anything in this regard), but i don't speak with any authority as i am not related to the Newtonsoft.Json project in any way.

Note that in both issues you linked to (one being from 2019, the other from 2020), the observed results mentioned in these report show an undefined token for an invalid missing json value, not a null token.

And, if your example json is being read into a JToken representation by using JToken.Parse instead of JsonConvert.DeserializeObject<JToken>, the result is undefined and not null, even with Newtonsoft 13.0.1 or 12.0.x (dotnetfiddle using 13.0.1 showing the discrepancy: https://dotnetfiddle.net/rVcJgF). So it could be that making the results of JsonConvert.DeserializeObject<JToken> and JToken.Parse congruous could therefore be a deliberate change. (Or not; it's just my suspicion/speculation...)

(Side note: Not arguing for or against maintaining behavior of the library -- that's for the author of the library to decide, but producing "undefined" here seems to be more appropriate. Considering that the json format features a null literal for null values, but there is no null literal in your json example, the token type JValue.Null for the missing value of "prop1" seems to me a technically incorrect representation. But then again, the json example is not valid and not adhering to the json format to begin with, so it would perhaps be technically (in)correct either way...)

[Ansssss]

@elgonzo yeah, it is kind of a garbage-in-garbage-out scenario, the input is not valid JSON. That being said, it is something the author apparently wants to parse, so I guess the question is should it be garbage-out (i.e. having the invalid undefined literal value for the property) or output valid JSON (null for undefined properties). I just want to know the plan so I can either wait for the next release or start fixing up our code where it might be passing in such JSON.

[JamesNK]

The new behavior is more correct. You could work around it by doing this:

public class NoUndefinedJsonReader : JsonTextReader
{
    public NoUndefinedJsonReader(TextReader reader) : base(reader)
    {
    }

    public override JsonToken TokenType
    {
        get
        {
            var tokenType = base.TokenType;
            if (tokenType == JsonToken.Undefined)
            {
                return JsonToken.Null;
            }
            return tokenType;
        }
    }
}

var reader = new NoUndefinedJsonReader(new StringReader("{ \"prop1\": , \"prop2\": 1 }"));
var serializer = new JsonSerializer();

var expectedOutput = serializer.Deserialize<JToken>(reader);
Console.WriteLine(expectedOutput.ToString());

[Ansssss]

Workaround sounds good. I'll close this issue

[barsh]

@Ansssss: Request to re-open this issue.

13.0.2 introduced this breaking change. A null value should not be converted to undefined. JSON.parse in a browser can parse a null value, but it throws an error when parsing undefined as you can see here:

[Ansssss]

@barsh I'm not sure if I should be the one to re-open the issue. If you want to, feel free. I'm not an author/contributor - I just use the library (and ended up fixing our input to be valid JSON where possible, and using the provided workaround where not).

[elgonzo]

@barsh

"A null value should not be converted to undefined." There is no null value anywhere in the json input here regarding this issue report. Hence, there is no null value being converted to undefined. Your request to re-open this issue seems to me to be a misunderstanding of the reported issue. (The issue report here is about a malformed json input that is missing a value for a property and not some json input with some null value(s).)

[barsh]

@elgonzo

Here is a screenshot of the original issue (from above):

[elgonzo]

@barsh don't show me any random screenshots. Point me at some null value being converted into undefined as you claim ;-)

To make it short, there is no conversion from a null value to undefined happening here. As @Ansssss so eloquently put it, it is a garbage-in-garbage-out scenario, looking at the result of feeding malformed illegal json into the deserializer.

[Rich-biomni]

@JamesNK

This is breaking change! I can't imagine it impacts many but still.

In our application an error was occuring in the browser after a RestAPI call. Some how the JSON returned from the RestAPI call was invalid, it included a property with undefined, thus the browser javascript JSON.parse threw.

How did the invalid JSON get there?

The application provides a low code / no code configurable workflow one the actions of the workflow allows calling external APIS returning data and storing them in workflow variables. The workflow engine uses Json.net to store the dynamic variable data as JTokens. In turns out that on one of these customer workflows called a very old 3rd party api and that api was returning JSON with a property undefined.

The workflow engine carried on running on the server with out error as Json.net acccepted the undefined value. It was not until down stream when looking at the run history of the workflow did and error occur.

Eventually we figured out that this only started happening since updating json.net

I wonder what the impetus was to change the behavior?

is it possible to have a new JsonSerializerSettings

UndefinedValueHandling Throw, KeepAsUndefined, ConvertToNull

Thoughts?

Note we are trying to reach out and geting the offending old 3rd party api fixed.