Incorrect JsonException.Path when using non-leaf custom JsonConverters #67403
Description
Description
When using System.Text.Json with a custom non-leaf JsonConverter and the input JSON is invalid, the JsonException.Path property is invalid.
Reproduction Steps
The next repro shows how the path becomes wrong when using a custom converter. Remove the brokenJson lines to observe the source JSON is identical.
The reason why we need a custom converter is because we are implementing the JSON:API specification, which dictates that the data element can contain a single item, null, or an array. Depending on context, the data element is reused at several places in the spec. It would be impractical for us to define separate data structures for all of the possible combinations that can occur.
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using JsonExceptionPathBugRepro.Converters;
using JsonExceptionPathBugRepro.Objects;
namespace JsonExceptionPathBugRepro
{
internal static class Program
{
public static void Main()
{
DeserializeWithoutCustomConverterForItem();
DeserializeWithoutCustomConverterForArray();
DeserializeWithCustomConverterForItem();
DeserializeWithCustomConverterForArray();
}
private static void DeserializeWithoutCustomConverterForItem()
{
var root = new SimpleRootDocumentWithItem
{
Data = new ResourceObject
{
Id = "1",
Attributes = new Dictionary<string, object?>
{
["name"] = "John Doe"
}
}
};
var options = new JsonSerializerOptions
{
WriteIndented = true
};
string json = JsonSerializer.Serialize(root, options);
string brokenJson = json.Replace("John Doe", "John \"Doe");
try
{
JsonSerializer.Deserialize<SimpleRootDocumentWithItem>(brokenJson, options);
}
catch (JsonException exception)
{
// prints correct path: "$.data.attributes.name"
Console.WriteLine(exception.Path);
}
}
private static void DeserializeWithoutCustomConverterForArray()
{
var root = new SimpleRootDocumentWithArray
{
Data = new List<ResourceObject>
{
new()
{
Id = "1",
Attributes = new Dictionary<string, object?>
{
["name"] = "John Doe"
}
}
}
};
var options = new JsonSerializerOptions
{
WriteIndented = true
};
string json = JsonSerializer.Serialize(root, options);
string brokenJson = json.Replace("John Doe", "John \"Doe");
try
{
JsonSerializer.Deserialize<SimpleRootDocumentWithArray>(brokenJson, options);
}
catch (JsonException exception)
{
// prints correct path: "$.data[0].attributes.name"
Console.WriteLine(exception.Path);
}
}
private static void DeserializeWithCustomConverterForItem()
{
var root = new RootDocument
{
Data = new SingleOrManyData<ResourceObject>(new ResourceObject
{
Id = "1",
Attributes = new Dictionary<string, object?>
{
["name"] = "John Doe"
}
})
};
var options = new JsonSerializerOptions
{
WriteIndented = true,
Converters =
{
new SingleOrManyDataConverterFactory()
}
};
string json = JsonSerializer.Serialize(root, options);
string brokenJson = json.Replace("John Doe", "John \"Doe");
try
{
JsonSerializer.Deserialize<RootDocument>(brokenJson, options);
}
catch (JsonException exception)
{
// prints "$.data" instead of "$.data.attributes.name"
Console.WriteLine(exception.Path);
}
}
private static void DeserializeWithCustomConverterForArray()
{
var root = new RootDocument
{
Data = new SingleOrManyData<ResourceObject>(new List<ResourceObject>
{
new()
{
Id = "1",
Attributes = new Dictionary<string, object?>
{
["name"] = "John Doe"
}
}
})
};
var options = new JsonSerializerOptions
{
WriteIndented = true,
Converters =
{
new SingleOrManyDataConverterFactory()
}
};
string json = JsonSerializer.Serialize(root, options);
string brokenJson = json.Replace("John Doe", "John \"Doe");
try
{
JsonSerializer.Deserialize<RootDocument>(brokenJson, options);
}
catch (JsonException exception)
{
// prints "$.data" instead of "$.data[0].attributes.name"
Console.WriteLine(exception.Path);
}
}
}
namespace Objects
{
public sealed class ResourceObject
{
[JsonPropertyName("id")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public string? Id { get; set; }
[JsonPropertyName("attributes")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public IDictionary<string, object?>? Attributes { get; set; }
}
public sealed class SimpleRootDocumentWithItem
{
[JsonPropertyName("data")]
public ResourceObject? Data { get; set; }
}
public sealed class SimpleRootDocumentWithArray
{
[JsonPropertyName("data")]
public List<ResourceObject>? Data { get; set; }
}
public readonly struct SingleOrManyData<T>
where T : class, new()
{
public object? Value => ManyValue != null ? ManyValue : SingleValue;
[JsonIgnore]
public bool IsAssigned { get; }
[JsonIgnore]
public T? SingleValue { get; }
[JsonIgnore]
public IList<T>? ManyValue { get; }
public SingleOrManyData(object? value)
{
IsAssigned = true;
if (value is IEnumerable<T> manyData)
{
ManyValue = manyData.ToList();
SingleValue = null;
}
else
{
ManyValue = null;
SingleValue = (T?)value;
}
}
}
public sealed class RootDocument
{
[JsonPropertyName("data")]
public SingleOrManyData<ResourceObject> Data { get; set; }
}
}
namespace Converters
{
/// <summary>
/// Converts <see cref="SingleOrManyData{T}" /> to/from JSON.
/// </summary>
public sealed class SingleOrManyDataConverterFactory : JsonConverterFactory
{
public override bool CanConvert(Type typeToConvert)
{
return typeToConvert.IsGenericType && typeToConvert.GetGenericTypeDefinition() == typeof(SingleOrManyData<>);
}
public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
{
Type objectType = typeToConvert.GetGenericArguments()[0];
Type converterType = typeof(SingleOrManyDataConverter<>).MakeGenericType(objectType);
return (JsonConverter)Activator.CreateInstance(converterType, BindingFlags.Instance | BindingFlags.Public, null, null, null)!;
}
private sealed class SingleOrManyDataConverter<T> : JsonObjectConverter<SingleOrManyData<T>>
where T : class, new()
{
public override SingleOrManyData<T> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions serializerOptions)
{
var objects = new List<T?>();
bool isManyData = false;
bool hasCompletedToMany = false;
do
{
switch (reader.TokenType)
{
case JsonTokenType.EndArray:
{
hasCompletedToMany = true;
break;
}
case JsonTokenType.Null:
{
if (isManyData)
{
objects.Add(new T());
}
break;
}
case JsonTokenType.StartObject:
{
var resourceObject = ReadSubTree<T>(ref reader, serializerOptions);
objects.Add(resourceObject);
break;
}
case JsonTokenType.StartArray:
{
isManyData = true;
break;
}
}
}
while (isManyData && !hasCompletedToMany && reader.Read());
object? data = isManyData ? objects : objects.FirstOrDefault();
return new SingleOrManyData<T>(data);
}
public override void Write(Utf8JsonWriter writer, SingleOrManyData<T> value, JsonSerializerOptions options)
{
WriteSubTree(writer, value.Value, options);
}
}
}
public abstract class JsonObjectConverter<TObject> : JsonConverter<TObject>
{
protected static TValue? ReadSubTree<TValue>(ref Utf8JsonReader reader, JsonSerializerOptions options)
{
if (typeof(TValue) != typeof(object) && options.GetConverter(typeof(TValue)) is JsonConverter<TValue> converter)
{
return converter.Read(ref reader, typeof(TValue), options);
}
return JsonSerializer.Deserialize<TValue>(ref reader, options);
}
protected static void WriteSubTree<TValue>(Utf8JsonWriter writer, TValue value, JsonSerializerOptions options)
{
if (typeof(TValue) != typeof(object) && options.GetConverter(typeof(TValue)) is JsonConverter<TValue> converter)
{
converter.Write(writer, value, options);
}
else
{
JsonSerializer.Serialize(writer, value, options);
}
}
}
}
}Expected behavior
The JsonException.Path value is the same for the same error in the source JSON, irrespective of whether a custom converter is used.
Actual behavior
Because the current position is tracked inside the built-in JsonConverters, there's no way for a custom converter to interact with them. And therefore there is no way to report the correct path from inside JsonException. The built-in converters just "forget" the inner scope when the path is built.
Regression?
We didn't have this problem when we were using Newtonsoft.Json.
Known Workarounds
We currently place sentinel error values in the deserialized object tree to reduce the chance of causing a JsonException. After deserialization has completed, we traverse the returned object tree, scanning for the sentinel values and throwing an exception with the proper path ourselves. We've basically re-implemented position tracking. However, this does not work in all cases (as shown in the example).
Configuration
The latest version of .NET 6. The same issue occurs in .NET 5.
Other information
I believe this is a design flaw. JsonConverter was initially only intended for leaf values. But using System.Text.Json in practice turns out to be so limiting, compared to Newtonsoft.Json, that the MSDN documentation is full of "you'll need to write a custom converter to make that work".
To reduce the pain a bit, we keep separate JsonSerializerOptions for reading and writing. That way, our write-only custom converters do not need to be registered when reading, and vice versa. It would be nice if converters could be read-only or write-only.