Repository navigation
Description
I have continued my investigation into discrepancies between single segment mode and multi segment mode in Utf8JsonSerializer. I found a few cases where the values for BytesConsumed, BytePositionInLine or the error message deviate between modes. Depending on the segmentation chunking, the error message and position numbers can be different for the same JSON. This could be an issue for debuggability and diagnosing production errors. It seems desirable that the segment mode should not affect parsing outcome.
It seems there are two issues:
- Processing of literals such as "true".
- Comment handling.
Here are the test cases:
static class ReproProgramCompare
{
public static void Run()
{
RunTestCase("fals");
RunTestCase("tb:");
RunTestCase("{\"\":tr");
RunTestCase("[[n{\"a\":");
RunTestCase("f-2.2e-2,-");
RunTestCase("/+");
RunTestCase("{/");
RunTestCase("{/s");
RunTestCase("{ /");
RunTestCase("{} /");
}
static void RunTestCase(string jsonString)
{
var result = GetResultCompare(new JsonInput(Encoding.UTF8.GetBytes(jsonString), true, false, JsonCommentHandling.Skip));
Console.WriteLine($"Test case: " + jsonString);
var messageOutput = $"BytesConsumed: {result.Result1.BytesConsumed}, CurrentDepth: {result.Result1.CurrentDepth}, TokenStartIndex: {result.Result1.TokenStartIndex}, TokenType: {result.Result1.TokenType}, Exception: {result.Result1.Exception?.Message}";
messageOutput += Environment.NewLine + $"BytesConsumed: {result.Result2.BytesConsumed}, CurrentDepth: {result.Result2.CurrentDepth}, TokenStartIndex: {result.Result2.TokenStartIndex}, TokenType: {result.Result2.TokenType}, Exception: {result.Result2.Exception?.Message}";
Console.WriteLine(messageOutput);
Console.WriteLine();
}
static CompareResult GetResultCompare(JsonInput jsonInput)
{
var result1 = GetResultJsonReaderSingleSegment(jsonInput);
var result2 = GetResultJsonReaderMultiSegment(jsonInput);
string differenceString = null;
var hasExceptionDifference =
(result1.Exception != null) != (result2.Exception != null) ||
(result1.Exception != null && result2.Exception != null && (result1.Exception.GetType() != result2.Exception.GetType() || result1.Exception.Message != result2.Exception.Message));
if (
hasExceptionDifference ||
result1.BytesConsumed != result2.BytesConsumed ||
result1.CurrentDepth != result2.CurrentDepth ||
result1.TokenStartIndex != result2.TokenStartIndex ||
result1.TokenType != result2.TokenType ||
false)
{
differenceString = $"Exception: {hasExceptionDifference}, BytesConsumed: {result1.BytesConsumed != result2.BytesConsumed}, CurrentDepth: {result1.CurrentDepth != result2.CurrentDepth}, TokenStartIndex: {result1.TokenStartIndex != result2.TokenStartIndex}, TokenType: {result1.TokenType != result2.TokenType}";
}
return new CompareResult(result1, result2, differenceString);
}
class CompareResult
{
public JsonResult Result1 { get; }
public JsonResult Result2 { get; }
public string DifferenceString { get; }
public CompareResult(JsonResult result1, JsonResult result2, string differenceString)
{
Result1 = result1;
Result2 = result2;
DifferenceString = differenceString;
}
public override string ToString()
{
return $"{nameof(Result1)}: {Result1}, {nameof(Result2)}: {Result2}, {nameof(DifferenceString)}: {DifferenceString}";
}
}
static JsonResult GetResultJsonReaderMultiSegment(JsonInput jsonInput)
{
static IEnumerable<Memory<byte>> SplitMemory(Memory<byte> memory, int chunkSize)
{
for (int startIndex = 0; startIndex < memory.Length; startIndex += chunkSize)
yield return memory.Slice(startIndex, Math.Min(chunkSize, memory.Length - startIndex));
}
var memories = SplitMemory(jsonInput.JsonBytes, 1);
var jsonReader = new Utf8JsonReader(CreateReadOnlySequence(memories), jsonInput.IsFinalBlock, new JsonReaderState(jsonInput.GetJsonReaderOptions()));
var jsonResult = ConsumeJsonReader(jsonReader, jsonInput.JsonBytes.Length);
return jsonResult;
}
static JsonResult GetResultJsonReaderSingleSegment(JsonInput jsonInput)
{
var utf8JsonGuardPage = jsonInput.JsonBytes;
var jsonReader = new Utf8JsonReader(utf8JsonGuardPage, jsonInput.IsFinalBlock, new JsonReaderState(jsonInput.GetJsonReaderOptions()));
return ConsumeJsonReader(jsonReader, utf8JsonGuardPage.Length);
}
static JsonResult ConsumeJsonReader(Utf8JsonReader jsonReader, int inputLength)
{
Exception exception = null;
try
{
long lastBytesConsumed = 0;
long lastTokenStartIndex = -1;
JsonTokenType lastTokenType = JsonTokenType.None;
while (jsonReader.Read())
{
if (jsonReader.BytesConsumed <= lastBytesConsumed)
throw new Exception("State: BytesConsumed.");
if (jsonReader.TokenStartIndex <= lastTokenStartIndex)
throw new Exception("State: TokenStartIndex.");
if (jsonReader.TokenType == lastTokenType &&
(lastTokenType == JsonTokenType.False ||
lastTokenType == JsonTokenType.True ||
lastTokenType == JsonTokenType.Null ||
lastTokenType == JsonTokenType.False ||
lastTokenType == JsonTokenType.String ||
lastTokenType == JsonTokenType.Number ||
lastTokenType == JsonTokenType.PropertyName ||
false))
throw new Exception("State: TokenType.");
lastBytesConsumed = jsonReader.BytesConsumed;
lastTokenStartIndex = jsonReader.TokenStartIndex;
lastTokenType = jsonReader.TokenType;
}
if (jsonReader.IsFinalBlock && jsonReader.BytesConsumed != inputLength)
throw new Exception("State: Incomplete.");
if (jsonReader.IsFinalBlock &&
(jsonReader.TokenType == JsonTokenType.StartArray ||
jsonReader.TokenType == JsonTokenType.StartObject ||
jsonReader.TokenType == JsonTokenType.None ||
false))
throw new Exception("State: Final TokenType.");
}
catch (Exception ex)
{
exception = ex;
}
return new JsonResult(exception, jsonReader.BytesConsumed, jsonReader.CurrentDepth, jsonReader.TokenStartIndex, jsonReader.TokenType);
}
readonly struct JsonInput
{
public byte[] JsonBytes { get; }
public bool IsFinalBlock { get; }
public bool AllowTrailingCommas { get; }
public JsonCommentHandling CommentHandling { get; }
public JsonReaderOptions GetJsonReaderOptions() => new JsonReaderOptions() { AllowTrailingCommas = AllowTrailingCommas, CommentHandling = CommentHandling };
public JsonInput(byte[] jsonBytes, bool isFinalBlock, bool allowTrailingCommas, JsonCommentHandling commentHandling)
{
JsonBytes = jsonBytes;
IsFinalBlock = isFinalBlock;
AllowTrailingCommas = allowTrailingCommas;
CommentHandling = commentHandling;
}
}
class JsonResult
{
public Exception Exception { get; }
public long BytesConsumed { get; }
public int CurrentDepth { get; }
public long TokenStartIndex { get; }
public JsonTokenType TokenType { get; }
public JsonResult(Exception exception, long bytesConsumed, int currentDepth, long tokenStartIndex, JsonTokenType tokenType)
{
Exception = exception;
BytesConsumed = bytesConsumed;
CurrentDepth = currentDepth;
TokenStartIndex = tokenStartIndex;
TokenType = tokenType;
}
public override string ToString()
{
return $"{nameof(Exception)}: {Exception}, {nameof(BytesConsumed)}: {BytesConsumed}, {nameof(CurrentDepth)}: {CurrentDepth}, {nameof(TokenStartIndex)}: {TokenStartIndex}, {nameof(TokenType)}: {TokenType}";
}
}
public static ReadOnlySequence<T> CreateReadOnlySequence<T>(IEnumerable<Memory<T>> buffers) => SimpleReadOnlySequenceSegment<T>.Create(buffers);
class SimpleReadOnlySequenceSegment<T> : ReadOnlySequenceSegment<T>
{
internal static ReadOnlySequence<T> Create(IEnumerable<Memory<T>> buffers)
{
SimpleReadOnlySequenceSegment<T> segment = null;
SimpleReadOnlySequenceSegment<T> first = null;
foreach (Memory<T> buffer in buffers)
{
var newSegment = new SimpleReadOnlySequenceSegment<T>()
{
Memory = buffer,
};
if (segment != null)
{
segment.Next = newSegment;
newSegment.RunningIndex = segment.RunningIndex + segment.Memory.Length;
}
else
{
first = newSegment;
}
segment = newSegment;
}
if (first == null)
{
first = segment = new SimpleReadOnlySequenceSegment<T>();
}
return new ReadOnlySequence<T>(first, 0, segment, segment.Memory.Length);
}
}
}
Output:
Test case: fals
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'fals' is an invalid JSON literal. Expected the literal 'false'. LineNumber: 0 | BytePositionInLine: 4.
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'fal' is an invalid JSON literal. Expected the literal 'false'. LineNumber: 0 | BytePositionInLine: 4.
Test case: tb:
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'tb:' is an invalid JSON literal. Expected the literal 'true'. LineNumber: 0 | BytePositionInLine: 1.
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'tb' is an invalid JSON literal. Expected the literal 'true'. LineNumber: 0 | BytePositionInLine: 1.
Test case: {"":tr
BytesConsumed: 4, CurrentDepth: 1, TokenStartIndex: 4, TokenType: PropertyName, Exception: 'tr' is an invalid JSON literal. Expected the literal 'true'. LineNumber: 0 | BytePositionInLine: 6.
BytesConsumed: 4, CurrentDepth: 1, TokenStartIndex: 4, TokenType: PropertyName, Exception: 't' is an invalid JSON literal. Expected the literal 'true'. LineNumber: 0 | BytePositionInLine: 6.
Test case: [[n{"a":
BytesConsumed: 2, CurrentDepth: 1, TokenStartIndex: 2, TokenType: StartArray, Exception: 'n{"a":' is an invalid JSON literal. Expected the literal 'null'. LineNumber: 0 | BytePositionInLine: 3.
BytesConsumed: 2, CurrentDepth: 1, TokenStartIndex: 2, TokenType: StartArray, Exception: 'n{' is an invalid JSON literal. Expected the literal 'null'. LineNumber: 0 | BytePositionInLine: 3.
Test case: f-2.2e-2,-
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'f-2.2e-2,-' is an invalid JSON literal. Expected the literal 'false'. LineNumber: 0 | BytePositionInLine: 1.
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: 'f-' is an invalid JSON literal. Expected the literal 'false'. LineNumber: 0 | BytePositionInLine: 1.
Test case: /+
BytesConsumed: 0, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: '/' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 0.
BytesConsumed: 1, CurrentDepth: 0, TokenStartIndex: 0, TokenType: None, Exception: '+' is invalid after '/' at the beginning of the comment. Expected either '/' or '*'. LineNumber: 0 | BytePositionInLine: 1.
Test case: {/
BytesConsumed: 1, CurrentDepth: 0, TokenStartIndex: 1, TokenType: StartObject, Exception: '/' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 1.
BytesConsumed: 2, CurrentDepth: 0, TokenStartIndex: 1, TokenType: StartObject, Exception: Unexpected end of data while reading a comment. LineNumber: 0 | BytePositionInLine: 2.
Test case: {/s
BytesConsumed: 1, CurrentDepth: 0, TokenStartIndex: 1, TokenType: StartObject, Exception: '/' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 1.
BytesConsumed: 2, CurrentDepth: 0, TokenStartIndex: 1, TokenType: StartObject, Exception: 's' is invalid after '/' at the beginning of the comment. Expected either '/' or '*'. LineNumber: 0 | BytePositionInLine: 2.
Test case: { /
BytesConsumed: 2, CurrentDepth: 0, TokenStartIndex: 2, TokenType: StartObject, Exception: '/' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 2.
BytesConsumed: 3, CurrentDepth: 0, TokenStartIndex: 2, TokenType: StartObject, Exception: Unexpected end of data while reading a comment. LineNumber: 0 | BytePositionInLine: 3.
Test case: {} /
BytesConsumed: 3, CurrentDepth: 0, TokenStartIndex: 3, TokenType: EndObject, Exception: '/' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 3.
BytesConsumed: 4, CurrentDepth: 0, TokenStartIndex: 3, TokenType: EndObject, Exception: Unexpected end of data while reading a comment. LineNumber: 0 | BytePositionInLine: 4.
Activity
ahsonkhan commentedon Aug 28, 2019
Thanks for the investigation and issue, @GSPP
Looks like all the discrepancies you mentioned are for invalid JSON and exception messages. This is something we should fix for vNext. I have updated the issue title to reflect that. Please feel free to update it as you see fit (FYI, these issues related to the
Utf8JsonReader, not serializer - so I updated that too).Did you find any issues with
BytesConsumedfor valid JSON or any other such discrepancy which could lead to correctness issues?[-]Discrepancies in Utf8JsonSerializer between single- and multi-segment modes[/-][+]Discrepancies in Utf8JsonReader between single- and multi-segment modes for invalid JSON[/+]GSPP commentedon Aug 28, 2019
I did not find any (further) issues with valid JSON. I understand that invalid JSON is of lower priority.
WinCPP commentedon Sep 10, 2019
@ahsonkhan @GSPP I was thinking of looking into this, if no one is...
EDIT: It was a conflict of 3.x vs 5.x for TargetFramework. Looks like I am able to proceed with generation of console app using the above test code...
I need a help, though. I am creating aself-containedapplication using my local build and for that I am first doing the steps at this link https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/dogfooding.md#option-2-self-contained The self-contained app is to use the test code shared by @GSPPHowever runningdotnet restore, I get these errors. Appreciate inputs on how to use the local build.GSPP commentedon Oct 19, 2021
This issue had previously been scheduled for a fix but that seems to have been postponed. Is it really the right choice to let the bot close this? I suggest that a team member makes an explicit decision on whether this is to be fixed or by design.
The discrepancy in
BytesConsumedseems to me to be the most critical point here. I don't know what this value is typically used for by callers. Maybe it is being used to advance a stream or pipe? In that case, that value would need to be accurate.eiriktsarpalis commentedon Oct 21, 2021
Related to #30751 and #27949.
eiriktsarpalis commentedon Apr 18, 2022
We won't have time to look at this during the 7.0 timeframe, moving to Future.
GSPP commentedon Apr 19, 2022
When you fix this, I have way of improving my systematic testing system and I can provide more such validation. I might also be able to find bugs that are truly problematic. Right now, I'm kind of waiting because the logs are polluted with such low-priority items. My point being that if these low-priority issues are fixed this might enable me to find more serious issues.
Of course, I respect your scheduling decision.
layomia commentedon Dec 2, 2022
@GSPP since you did the initial deep dive on finding this issue, would you consider offering a fix? This way the library gets better and you are unblocked for further investigation.
GSPP commentedon Dec 4, 2022
@layomia I'm really not set up to contribute right now but I might very well resume the investigation when unblocked. Lots more to try.