Łukasz Łukasz - 2 months ago 12
C# Question

Newtonsoft.Json parses incorrect json

I encountered a weird problem: given such string

{"text":"s","cursorPosition":189,"dataSource":"json_northwind",
which is not a correct json, it still gets succesfully parsed.

this is the class:

public class CompletionDataRequest
{
public CompletionDataRequest(string text, int cursorPosition, string dataSource, string project)
{
Text = text;
CursorPosition = cursorPosition;
DataSource = dataSource;
Project = project;
}

public string Text { get; }
public int CursorPosition { get; }
public string DataSource { get; }
public string Project { get; }
}


Here is test that surprisingly succeeds:

var s = @"{""text"":""s"",""cursorPosition"":189,""dataSource"":""json_northwind"",";
var request = JsonConvert.DeserializeObject<CompletionDataRequest>(s);
request.Text.Should().Be("s");
request.CursorPosition.Should().Be(189);
request.DataSource.Should().Be("json_northwind");
request.Project.Should().BeNull();


does the library have some loosened parsing rules or maybe this is a bug? I am library version 9.0.1

dbc dbc
Answer

You have found a bug in Json.NET. It arises only when your object is constructed with a parameterized constructor. If I modify your object to have a non-parameterized constructor:

public class CompletionDataRequest
{
    public CompletionDataRequest(string text, int cursorPosition, string dataSource, string project)
    {
        Text = text;
        CursorPosition = cursorPosition;
        DataSource = dataSource;
        Project = project;
    }

    [JsonConstructor]
    private CompletionDataRequest()
    {
    }

    [JsonProperty]
    public string Text { get; private set; }
    [JsonProperty]
    public int CursorPosition { get; private set; }
    [JsonProperty]
    public string DataSource { get; private set; }
    [JsonProperty]
    public string Project { get; private set; }
}

Then Json.NET will correctly throw a JsonSerializationException.

The cause of the bug is as follows. When creating an object with a parameterless constructor, Json.NET first constructs the object, then populates it with JsonSerializerInternalReader.PopulateObject(). This method has the following (simplified) logic:

    private object PopulateObject(object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, string id)
    {
        bool finished = false;
        do
        {
            switch (reader.TokenType)
            {
                case JsonToken.PropertyName:
                {
                    // Read and process the property.
                }
                case JsonToken.EndObject:
                    finished = true;
                    break;
                case JsonToken.Comment:
                    // ignore
                    break;
                default:
                    throw JsonSerializationException.Create(reader, "Unexpected token when deserializing object: " + reader.TokenType);
            }
        } while (!finished && reader.Read());

        if (!finished)
        {
            ThrowUnexpectedEndException(reader, contract, newObject, "Unexpected end when deserializing object.");
        }

        return newObject;
     }

As you can see, there is logic if (!finished) to verify that the object is actually closed.

However, when creating an object with a parameterized constructor, the properties are read before the object is constructed, using JsonSerializerInternalReader.ResolvePropertyAndCreatorValues():

    private List<CreatorPropertyContext> ResolvePropertyAndCreatorValues(JsonObjectContract contract, JsonProperty containerProperty, JsonReader reader, Type objectType)
    {
        List<CreatorPropertyContext> propertyValues = new List<CreatorPropertyContext>();
        bool exit = false;
        do
        {
            switch (reader.TokenType)
            {
                case JsonToken.PropertyName:
                    // Read and process the property.
                    break;
                case JsonToken.Comment:
                    break;
                case JsonToken.EndObject:
                    exit = true;
                    break;
                default:
                    throw JsonSerializationException.Create(reader, "Unexpected token when deserializing object: " + reader.TokenType);
            }
        } while (!exit && reader.Read());

        return propertyValues;
    }

As you can see there's no equivalent check for exit being true.

An issue Deserializing unclosed object succeeds when the object has a parameterized constructor. #1038 was opened for this.