From 0a224f6d11b9f66040ff55c156eb03e76fb417e2 Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Sat, 2 May 2026 15:14:24 +0900 Subject: [PATCH 1/5] Fix scoped Utf8JsonReader to carry original position In JsonSerializer.GetReaderScopedToNextValue, capture the original reader's _lineNumber and _bytePositionInLine, rewind them per token type to point immediately before the value token, and pass them to the scoped reader through JsonReaderState. The scoped reader, after consuming its first token, lands on the same position as the original reader, so JsonException now reports positions relative to the original input. --- .../JsonSerializer.Read.Utf8JsonReader.cs | 43 +++++- .../Serialization/ReadValueTests.cs | 126 ++++++++++++++++++ 2 files changed, 167 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs index e84888c90087fa..94846f79afa20e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs @@ -325,6 +325,13 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read ReadOnlySpan valueSpan = default; ReadOnlySequence valueSequence = default; + // Capture the original reader's position so that the scoped reader, after consuming its + // first token, lands on the same position the original reader is currently at. + // The values captured here represent the position immediately after the current token, + // the per-case logic below rewinds them to the position immediately before the value token starts. + long lineNumber = reader.CurrentState._lineNumber; + long bytePositionInLine = reader.CurrentState._bytePositionInLine; + try { switch (reader.TokenType) @@ -341,6 +348,11 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read { ThrowHelper.ThrowJsonReaderException(ref reader, ExceptionResource.ExpectedOneCompleteToken); } + + // Because the reader has advanced, the reader's position have been updated, + // so we need to recapture them here. + lineNumber = reader.CurrentState._lineNumber; + bytePositionInLine = reader.CurrentState._bytePositionInLine; break; } } @@ -371,6 +383,8 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read valueSequence = sequence.Slice(startingOffset, totalLength); } + // Rewind by 1 byte to point right before the opening '{' or '['. + bytePositionInLine--; Debug.Assert(reader.TokenType is JsonTokenType.EndObject or JsonTokenType.EndArray); break; @@ -379,13 +393,16 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read case JsonTokenType.True: case JsonTokenType.False: case JsonTokenType.Null: + // Rewind by the length of the value token to point right before the start of the value. if (reader.HasValueSequence) { valueSequence = reader.ValueSequence; + bytePositionInLine -= valueSequence.Length; } else { valueSpan = reader.ValueSpan; + bytePositionInLine -= valueSpan.Length; } break; @@ -412,6 +429,9 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read $"Calculated span ends with {readerSpan[(int)reader.TokenStartIndex + payloadLength - 1]}"); valueSpan = readerSpan.Slice((int)reader.TokenStartIndex, payloadLength); + + // Rewind by payloadLength to point right before the opening quote. + bytePositionInLine -= payloadLength; } else { @@ -427,6 +447,9 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read Debug.Assert( valueSequence.ToArray()[payloadLength - 1] == (byte)'"', $"Calculated sequence ends with {valueSequence.ToArray()[payloadLength - 1]}"); + + // Rewind by payloadLength to point right before the opening quote. + bytePositionInLine -= payloadLength; } break; @@ -452,9 +475,25 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read Debug.Assert(!valueSpan.IsEmpty ^ !valueSequence.IsEmpty); + // Carry only the position information and reader options to the scoped reader + // so that any JsonException it raises reports a position relative to the original input. + var scopedCurrentState = new JsonReaderState + ( + lineNumber: lineNumber, + bytePositionInLine: bytePositionInLine, + inObject: default, + isNotPrimitive: default, + valueIsEscaped: default, + trailingCommaBeforeComment: default, + tokenType: default, + previousTokenType: default, + readerOptions: reader.CurrentState.Options, + bitStack: default + ); + return valueSpan.IsEmpty - ? new Utf8JsonReader(valueSequence, reader.CurrentState.Options) - : new Utf8JsonReader(valueSpan, reader.CurrentState.Options); + ? new Utf8JsonReader(valueSequence, isFinalBlock: true, state: scopedCurrentState) + : new Utf8JsonReader(valueSpan, isFinalBlock: true, state: scopedCurrentState); } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs index cd74eda2748fd6..d30fd9a54e552a 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs @@ -781,6 +781,132 @@ public static void ReadSimpleList_AllowMultipleValues_TrailingContent() List result = JsonSerializer.Deserialize>(ref reader); Assert.Equal([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20], result); } + + [Fact] + public static void ReaderPreservesPositionInfo() + { + var utf8 = """ + [ + 42 + ] + """u8.ToArray(); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8); + + reader.Read(); + reader.Read(); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(1, ex.LineNumber); + Assert.Equal(6, ex.BytePositionInLine); + } + + [Theory] + [InlineData("[ 42]", typeof(string), 0, 5)] + [InlineData("[true]", typeof(string), 0, 5)] + [InlineData("[false]", typeof(string), 0, 6)] + [InlineData("[null]", typeof(int), 0, 5)] + [InlineData("[\"hello\"]", typeof(int), 0, 8)] + [InlineData("[{\"key\":1}]", typeof(string), 0, 2)] + [InlineData("[[1,2]]", typeof(string), 0, 2)] + public static void ReaderPreservesPositionInfoSingleLineTokens( + string json, Type deserializeType, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + + JsonSerializer.Deserialize(ref reader, deserializeType); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } + + [Fact] + public static void ReaderPreservesPositionInfoNoneTokenType() + { + byte[] utf8 = "42"u8.ToArray(); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(0, ex.LineNumber); + Assert.Equal(2, ex.BytePositionInLine); + } + + [Fact] + public static void ReaderPreservesPositionInfoPropertyNameTokenType() + { + byte[] utf8 = "{\"val\": 42}"u8.ToArray(); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + Assert.Equal(JsonTokenType.PropertyName, reader.TokenType); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(0, ex.LineNumber); + Assert.Equal(10, ex.BytePositionInLine); + } + + [Fact] + public static void ReaderPreservesPositionInfoPropertyNameMultiLine() + { + byte[] utf8 = Encoding.UTF8.GetBytes("{\n \"val\":\n 42\n}"); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + Assert.Equal(JsonTokenType.PropertyName, reader.TokenType); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(2, ex.LineNumber); + Assert.Equal(4, ex.BytePositionInLine); + } + + [Theory] + [InlineData("[1234]", 2, typeof(string), 0, 5)] + [InlineData("[true]", 3, typeof(string), 0, 5)] + [InlineData("[\"hello\"]", 4, typeof(int), 0, 8)] + [InlineData("[{\"key\":1}]", 5, typeof(string), 0, 2)] + public static void ReaderPreservesPositionInfoMultiSegment(string json, int splitAt, Type deserializeType, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + ReadOnlySequence sequence = JsonTestHelper.CreateSegments(utf8, splitAt); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(sequence, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + + JsonSerializer.Deserialize(ref reader, deserializeType); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } } // From https://github.com/dotnet/runtime/issues/882 From 9f867e359ce7cbe614b8d9ca6806630810864d77 Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Sat, 2 May 2026 17:00:44 +0900 Subject: [PATCH 2/5] Add test case and fix grammatical error --- .../JsonSerializer.Read.Utf8JsonReader.cs | 2 +- .../Serialization/ReadValueTests.cs | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs index 94846f79afa20e..3fe446a9bf6538 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs @@ -349,7 +349,7 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read ThrowHelper.ThrowJsonReaderException(ref reader, ExceptionResource.ExpectedOneCompleteToken); } - // Because the reader has advanced, the reader's position have been updated, + // Because the reader has advanced, the reader's position has been updated, // so we need to recapture them here. lineNumber = reader.CurrentState._lineNumber; bytePositionInLine = reader.CurrentState._bytePositionInLine; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs index d30fd9a54e552a..98fb8bc24ee019 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs @@ -907,6 +907,49 @@ public static void ReaderPreservesPositionInfoMultiSegment(string json, int spli Assert.Equal(expectedLine, ex.LineNumber); Assert.Equal(expectedBytePosition, ex.BytePositionInLine); } + + [Fact] + public static void ReaderPreservesPositionInfoMultiByteUtf8String() + { + // "😀葛🀄" occupies 11 bytes in UTF-8 (4 + 3 + 4) including surrogate pairs, + // so the closing quote sits at byte index 13 and BytePositionInLine after the token is 14. + byte[] utf8 = Encoding.UTF8.GetBytes("[\"😀葛🀄\"]"); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + Assert.Equal(JsonTokenType.String, reader.TokenType); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(0, ex.LineNumber); + Assert.Equal(14, ex.BytePositionInLine); + } + + [Theory] + [InlineData("[\n {\"key\":1}\n]", typeof(string), 1, 3)] + [InlineData("[\n [1, 2]\n]", typeof(string), 1, 3)] + public static void ReaderPreservesPositionInfoMultiLineContainer( + string json, Type deserializeType, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: default); + reader.Read(); + reader.Read(); + Assert.True(reader.TokenType is JsonTokenType.StartObject or JsonTokenType.StartArray); + + JsonSerializer.Deserialize(ref reader, deserializeType); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } } // From https://github.com/dotnet/runtime/issues/882 From b9664bef017b2f6e921b8df09981b15515a1eeba Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Tue, 5 May 2026 09:44:49 +0900 Subject: [PATCH 3/5] Capture position information once after the first switch statement --- .../JsonSerializer.Read.Utf8JsonReader.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs index 3fe446a9bf6538..c751ec149253c5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs @@ -325,12 +325,8 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read ReadOnlySpan valueSpan = default; ReadOnlySequence valueSequence = default; - // Capture the original reader's position so that the scoped reader, after consuming its - // first token, lands on the same position the original reader is currently at. - // The values captured here represent the position immediately after the current token, - // the per-case logic below rewinds them to the position immediately before the value token starts. - long lineNumber = reader.CurrentState._lineNumber; - long bytePositionInLine = reader.CurrentState._bytePositionInLine; + long lineNumber = 0; + long bytePositionInLine = 0; try { @@ -348,15 +344,17 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read { ThrowHelper.ThrowJsonReaderException(ref reader, ExceptionResource.ExpectedOneCompleteToken); } - - // Because the reader has advanced, the reader's position has been updated, - // so we need to recapture them here. - lineNumber = reader.CurrentState._lineNumber; - bytePositionInLine = reader.CurrentState._bytePositionInLine; break; } } + // Capture the original reader's position so that the scoped reader, after consuming its + // first token, lands on the same position the original reader is currently at. + // The values captured here represent the position immediately after the current token, + // the per-case logic below rewinds them to the position immediately before the value token starts. + lineNumber = reader.CurrentState._lineNumber; + bytePositionInLine = reader.CurrentState._bytePositionInLine; + switch (reader.TokenType) { // Any of the "value start" states are acceptable. From 762cf7ec861c1312c1489975546a042e063c6f4c Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Tue, 5 May 2026 10:06:47 +0900 Subject: [PATCH 4/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../System.Text.Json.Tests/Serialization/ReadValueTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs index 98fb8bc24ee019..47649cdbd6434a 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs @@ -911,7 +911,7 @@ public static void ReaderPreservesPositionInfoMultiSegment(string json, int spli [Fact] public static void ReaderPreservesPositionInfoMultiByteUtf8String() { - // "😀葛🀄" occupies 11 bytes in UTF-8 (4 + 3 + 4) including surrogate pairs, + // "😀葛🀄" occupies 11 bytes in UTF-8 (4 + 3 + 4), // so the closing quote sits at byte index 13 and BytePositionInLine after the token is 14. byte[] utf8 = Encoding.UTF8.GetBytes("[\"😀葛🀄\"]"); From 88b871e5f77f275b207cf3561ca713c0d9784dbb Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Wed, 6 May 2026 15:25:39 +0900 Subject: [PATCH 5/5] Add Debug.Assert checking and test cases with skipped comments --- .../JsonSerializer.Read.Utf8JsonReader.cs | 2 + .../Serialization/ReadValueTests.cs | 78 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs index c751ec149253c5..2933c753c7ba69 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs @@ -472,6 +472,8 @@ private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader read } Debug.Assert(!valueSpan.IsEmpty ^ !valueSequence.IsEmpty); + Debug.Assert(lineNumber >= 0); + Debug.Assert(bytePositionInLine >= 0); // Carry only the position information and reader options to the scoped reader // so that any JsonException it raises reports a position relative to the original input. diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs index 47649cdbd6434a..3e1d3310cd5415 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReadValueTests.cs @@ -950,6 +950,84 @@ public static void ReaderPreservesPositionInfoMultiLineContainer( Assert.Equal(expectedLine, ex.LineNumber); Assert.Equal(expectedBytePosition, ex.BytePositionInLine); } + + [Theory] + [InlineData("[ /* comment */ 42 ]", typeof(string), 0, 18)] + [InlineData("[ // comment\n42 ]", typeof(string), 1, 2)] + [InlineData("[ /* comment */ true ]", typeof(int), 0, 20)] + [InlineData("[ /* comment */ false ]", typeof(int), 0, 21)] + [InlineData("[ /* comment */ null ]", typeof(int), 0, 20)] + [InlineData("[ /* comment */ \"hello\" ]", typeof(int), 0, 23)] + [InlineData("[ /* comment */ {\"key\":1} ]", typeof(string), 0, 17)] + [InlineData("[ /* comment */ [1,2] ]", typeof(string), 0, 17)] + [InlineData("[ /*\nmultiline\ncomment\n*/ 42 ]", typeof(string), 3, 5)] + [InlineData("[ /*\n*/ 42 ]", typeof(string), 1, 5)] + [InlineData("[ /*\nmultiline\n*/ {\"key\":1} ]", typeof(string), 2, 4)] + [InlineData("[ /*\nmultiline\n*/ [1,2] ]", typeof(string), 2, 4)] + [InlineData("[ /*\nmultiline\n*/ \"hello\" ]", typeof(int), 2, 10)] + public static void ReaderPreservesPositionInfoWithSkippedComments( + string json, Type deserializeType, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + var options = new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip }; + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: new JsonReaderState(options)); + reader.Read(); + reader.Read(); + + JsonSerializer.Deserialize(ref reader, deserializeType); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } + + [Theory] + [InlineData("{\"val\": /* comment */ 42}", 0, 24)] + [InlineData("{\"val\": // comment\n42}", 1, 2)] + [InlineData("{\"val\":\n/* comment */\n42}", 2, 2)] + [InlineData("{\"val\": /* comment */ {\"k\":1}}", 0, 23)] + public static void ReaderPreservesPositionInfoPropertyNameWithSkippedComments( + string json, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + var options = new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip }; + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: new JsonReaderState(options)); + reader.Read(); + reader.Read(); + Assert.Equal(JsonTokenType.PropertyName, reader.TokenType); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } + + [Theory] + [InlineData("/* comment */ 42", 0, 16)] + [InlineData("/*\ncomment\n*/ 42", 2, 5)] + public static void ReaderPreservesPositionInfoWithCommentBeforeNoneToken( + string json, long expectedLine, long expectedBytePosition) + { + byte[] utf8 = Encoding.UTF8.GetBytes(json); + var options = new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip }; + + JsonException ex = Assert.Throws(() => + { + var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state: new JsonReaderState(options)); + + JsonSerializer.Deserialize(ref reader); + }); + + Assert.Equal(expectedLine, ex.LineNumber); + Assert.Equal(expectedBytePosition, ex.BytePositionInLine); + } } // From https://github.com/dotnet/runtime/issues/882