Fix reading past comments after an end object or end array token within Utf8JsonReade...

* Fix reading past comments after an end object or end array token.

* Add more test cases and update unreachable code paths with
Debug.Asserts.

Commit migrated from https://github.com/dotnet/corefx/commit/247793c64816a887c4ff47a92acdd9b527b7873b

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
index 56b06b5..a8852dd 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
@@ -1846,48 +1846,36 @@ namespace System.Text.Json
             }
             else if (_tokenType == JsonTokenType.StartObject)
             {
-                if (first == JsonConstants.CloseBrace)
+                Debug.Assert(first != JsonConstants.CloseBrace);
+                if (first != JsonConstants.Quote)
                 {
-                    EndObject();
+                    ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
                 }
-                else
-                {
-                    if (first != JsonConstants.Quote)
-                    {
-                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
-                    }
 
-                    long prevTotalConsumed = _totalConsumed;
-                    int prevConsumed = _consumed;
-                    long prevPosition = _bytePositionInLine;
-                    long prevLineNumber = _lineNumber;
-                    if (!ConsumePropertyNameMultiSegment())
-                    {
-                        // roll back potential changes
-                        _consumed = prevConsumed;
-                        _tokenType = JsonTokenType.StartObject;
-                        _bytePositionInLine = prevPosition;
-                        _lineNumber = prevLineNumber;
-                        _totalConsumed = prevTotalConsumed;
-                        goto RollBack;
-                    }
-                    goto Done;
+                long prevTotalConsumed = _totalConsumed;
+                int prevConsumed = _consumed;
+                long prevPosition = _bytePositionInLine;
+                long prevLineNumber = _lineNumber;
+                if (!ConsumePropertyNameMultiSegment())
+                {
+                    // roll back potential changes
+                    _consumed = prevConsumed;
+                    _tokenType = JsonTokenType.StartObject;
+                    _bytePositionInLine = prevPosition;
+                    _lineNumber = prevLineNumber;
+                    _totalConsumed = prevTotalConsumed;
+                    goto RollBack;
                 }
+                goto Done;
             }
             else if (_tokenType == JsonTokenType.StartArray)
             {
-                if (first == JsonConstants.CloseBracket)
-                {
-                    EndArray();
-                }
-                else
+                Debug.Assert(first != JsonConstants.CloseBracket);
+                if (!ConsumeValueMultiSegment(first))
                 {
-                    if (!ConsumeValueMultiSegment(first))
-                    {
-                        goto RollBack;
-                    }
-                    goto Done;
+                    goto RollBack;
                 }
+                goto Done;
             }
             else if (_tokenType == JsonTokenType.PropertyName)
             {
@@ -1899,7 +1887,37 @@ namespace System.Text.Json
             }
             else
             {
-                goto RollBack;
+                Debug.Assert(_tokenType == JsonTokenType.EndArray || _tokenType == JsonTokenType.EndObject);
+                if (_inObject)
+                {
+                    Debug.Assert(first != JsonConstants.CloseBracket);
+                    if (first != JsonConstants.Quote)
+                    {
+                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
+                    }
+
+                    if (ConsumePropertyNameMultiSegment())
+                    {
+                        goto Done;
+                    }
+                    else
+                    {
+                        goto RollBack;
+                    }
+                }
+                else
+                {
+                    Debug.Assert(first != JsonConstants.CloseBracket);
+
+                    if (ConsumeValueMultiSegment(first))
+                    {
+                        goto Done;
+                    }
+                    else
+                    {
+                        goto RollBack;
+                    }
+                }
             }
 
         Done:

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
index 78f2c49..185eb7d 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
@@ -1980,46 +1980,34 @@ namespace System.Text.Json
             }
             else if (_tokenType == JsonTokenType.StartObject)
             {
-                if (first == JsonConstants.CloseBrace)
+                Debug.Assert(first != JsonConstants.CloseBrace);
+                if (first != JsonConstants.Quote)
                 {
-                    EndObject();
+                    ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
                 }
-                else
-                {
-                    if (first != JsonConstants.Quote)
-                    {
-                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
-                    }
 
-                    int prevConsumed = _consumed;
-                    long prevPosition = _bytePositionInLine;
-                    long prevLineNumber = _lineNumber;
-                    if (!ConsumePropertyName())
-                    {
-                        // roll back potential changes
-                        _consumed = prevConsumed;
-                        _tokenType = JsonTokenType.StartObject;
-                        _bytePositionInLine = prevPosition;
-                        _lineNumber = prevLineNumber;
-                        goto RollBack;
-                    }
-                    goto Done;
+                int prevConsumed = _consumed;
+                long prevPosition = _bytePositionInLine;
+                long prevLineNumber = _lineNumber;
+                if (!ConsumePropertyName())
+                {
+                    // roll back potential changes
+                    _consumed = prevConsumed;
+                    _tokenType = JsonTokenType.StartObject;
+                    _bytePositionInLine = prevPosition;
+                    _lineNumber = prevLineNumber;
+                    goto RollBack;
                 }
+                goto Done;
             }
             else if (_tokenType == JsonTokenType.StartArray)
             {
-                if (first == JsonConstants.CloseBracket)
-                {
-                    EndArray();
-                }
-                else
+                Debug.Assert(first != JsonConstants.CloseBracket);
+                if (!ConsumeValue(first))
                 {
-                    if (!ConsumeValue(first))
-                    {
-                        goto RollBack;
-                    }
-                    goto Done;
+                    goto RollBack;
                 }
+                goto Done;
             }
             else if (_tokenType == JsonTokenType.PropertyName)
             {
@@ -2031,7 +2019,37 @@ namespace System.Text.Json
             }
             else
             {
-                goto RollBack;
+                Debug.Assert(_tokenType == JsonTokenType.EndArray || _tokenType == JsonTokenType.EndObject);
+                if (_inObject)
+                {
+                    Debug.Assert(first != JsonConstants.CloseBracket);
+                    if (first != JsonConstants.Quote)
+                    {
+                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
+                    }
+
+                    if (ConsumePropertyName())
+                    {
+                        goto Done;
+                    }
+                    else
+                    {
+                        goto RollBack;
+                    }
+                }
+                else
+                {
+                    Debug.Assert(first != JsonConstants.CloseBracket);
+
+                    if (ConsumeValue(first))
+                    {
+                        goto Done;
+                    }
+                    else
+                    {
+                        goto RollBack;
+                    }
+                }
             }
 
         Done:

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
index bae80de..bdbda4d 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
@@ -1729,6 +1729,284 @@ namespace System.Text.Json.Tests
         }
 
         [Theory]
+        [InlineData("{,}")]
+        [InlineData("[,]")]
+        [InlineData("{// comment\n,}")]
+        [InlineData("[// comment\n,]")]
+        [InlineData("123, ")]
+        [InlineData("\"abc\", ")]
+        [InlineData("123, // comment\n")]
+        [InlineData("\"abc\", // comment\n")]
+        [InlineData("123 // comment\n,")]
+        [InlineData("\"abc\" // comment\n,")]
+        [InlineData("{\"Property1\": [ 42], 5}")]
+        [InlineData("{\"Property1\": [ 42], // comment\n5}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r5}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r\n5}")]
+        [InlineData("{\"Property1\": [ 42], /*comment*/5}")]
+        [InlineData("{\"Property1\": [ 42] // comment\n,5}")]
+        [InlineData("{\"Property1\": [ 42], // comment\n // comment\n5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, 5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r\n5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, /*comment*/5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42} // comment\n,5}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n // comment\n5}")]
+        [InlineData("{// comment\n5}")]
+        public static void ReadInvalidJsonStringsWithComments(string jsonString)
+        {
+            byte[] input = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                var options = new JsonReaderOptions() { CommentHandling = commentHandling };
+                var optionsWithTrailing = new JsonReaderOptions() { CommentHandling = commentHandling, AllowTrailingCommas = true };
+
+                var reader = new Utf8JsonReader(input, options);
+                ValidateThrows(ref reader);
+
+                reader = new Utf8JsonReader(input, optionsWithTrailing);
+                ValidateThrows(ref reader);
+
+                ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(input, 1);
+                reader = new Utf8JsonReader(sequence, options);
+                ValidateThrows(ref reader);
+
+                reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                ValidateThrows(ref reader);
+
+                for (int splitLocation = 0; splitLocation < input.Length; splitLocation++)
+                {
+                    sequence = JsonTestHelper.CreateSegments(input, splitLocation);
+                    reader = new Utf8JsonReader(sequence, options);
+                    ValidateThrows(ref reader);
+
+                    reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                    ValidateThrows(ref reader);
+                }
+
+                for (int firstSplit = 0; firstSplit < input.Length; firstSplit++)
+                {
+                    for (int secondSplit = firstSplit; secondSplit < input.Length; secondSplit++)
+                    {
+                        sequence = JsonTestHelper.CreateSegments(input, firstSplit, secondSplit);
+                        reader = new Utf8JsonReader(sequence, options);
+                        ValidateThrows(ref reader);
+
+                        reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                        ValidateThrows(ref reader);
+                    }
+                }
+            }
+        }
+
+        private static void ValidateThrows(ref Utf8JsonReader reader)
+        {
+            JsonTestHelper.AssertThrows<JsonException>(reader, (jsonReader) =>
+            {
+                while (jsonReader.Read())
+                    ;
+            });
+        }
+
+        [Theory]
+        [InlineData("123 ")]
+        [InlineData("\"abc\" ")]
+        [InlineData("123 // comment\n")]
+        [InlineData("\"abc\" // comment\n")]
+        [InlineData("{// comment\n}")]
+        [InlineData("[// comment\n]")]
+        [InlineData("{\"Property1\": [ 42], \"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42], // comment\n\"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r\"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r\n\"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42], /*comment*/\"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42] // comment\n,\"Property2\": 42}")]
+        [InlineData("{\"Property1\": [ 42], // comment\n // comment\n\"Property2\": 42}")]
+        [InlineData("[[ 42], // comment\n 42]")]
+        [InlineData("[[ 42] // comment\n, 42]")]
+        [InlineData("[[ 42, // comment\n 43], // comment\n 42]")]
+        [InlineData("[[ \"42\", // comment\n 43], // comment\n 42]")]
+        [InlineData("[[ true, // comment\n 43], // comment\n 42]")]
+        [InlineData("[[ false, // comment\n 43], // comment\n 42]")]
+        [InlineData("[[ null, // comment\n 43], // comment\n 42]")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, \"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n\"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r\"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r\n\"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, /*comment*/\"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42} // comment\n,\"Property2\": 42}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n // comment\n\"Property2\": 42}")]
+        [InlineData("[{\"Property1\": 42}, // comment\n 42]")]
+        [InlineData("[{\"Property1\": 42} // comment\n, 42]")]
+        [InlineData("[{\"Property1\": 42, // comment\n \"prop\": 43 }, // comment\n 42]")]
+        [InlineData("[{\"Property1\": \"42\", // comment\n \"prop\": 43 }, // comment\n 42]")]
+        [InlineData("[{\"Property1\": true, // comment\n \"prop\": 43 }, // comment\n 42]")]
+        [InlineData("[{\"Property1\": false, // comment\n \"prop\": 43 }, // comment\n 42]")]
+        [InlineData("[{\"Property1\": null, // comment\n \"prop\": 43 }, // comment\n 42]")]
+        public static void ReadJsonStringsWithComments(string jsonString)
+        {
+            byte[] input = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                var options = new JsonReaderOptions() { CommentHandling = commentHandling };
+                var optionsWithTrailing = new JsonReaderOptions() { CommentHandling = commentHandling, AllowTrailingCommas = true };
+
+                var reader = new Utf8JsonReader(input, options);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                reader = new Utf8JsonReader(input, optionsWithTrailing);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(input, 1);
+                reader = new Utf8JsonReader(sequence, options);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                for (int splitLocation = 0; splitLocation < input.Length; splitLocation++)
+                {
+                    sequence = JsonTestHelper.CreateSegments(input, splitLocation);
+                    reader = new Utf8JsonReader(sequence, options);
+                    ReadWithCommentsHelper(ref reader, input.Length);
+
+                    reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                    ReadWithCommentsHelper(ref reader, input.Length);
+                }
+
+                for (int firstSplit = 0; firstSplit < input.Length; firstSplit++)
+                {
+                    for (int secondSplit = firstSplit; secondSplit < input.Length; secondSplit++)
+                    {
+                        sequence = JsonTestHelper.CreateSegments(input, firstSplit, secondSplit);
+                        reader = new Utf8JsonReader(sequence, options);
+                        ReadWithCommentsHelper(ref reader, input.Length);
+
+                        reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                        ReadWithCommentsHelper(ref reader, input.Length);
+                    }
+                }
+            }
+        }
+
+        [Theory]
+        [InlineData("{\"Property1\": [ 42], }")]
+        [InlineData("{\"Property1\": [ 42], // comment\n}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r}")]
+        [InlineData("{\"Property1\": [ 42], // comment\r\n}")]
+        [InlineData("{\"Property1\": [ 42], /*comment*/}")]
+        [InlineData("{\"Property1\": [ 42] // comment\n,}")]
+        [InlineData("{\"Property1\": [ 42], // comment\n // comment\n}")]
+        [InlineData("[[ 42], // comment\n ]")]
+        [InlineData("[[ 42] // comment\n, ]")]
+        [InlineData("[[ 42, // comment\n ], // comment\n ]")]
+        [InlineData("[[ \"42\", // comment\n ], // comment\n ]")]
+        [InlineData("[[ true, // comment\n ], // comment\n ]")]
+        [InlineData("[[ false, // comment\n ], // comment\n ]")]
+        [InlineData("[[ null, // comment\n ], // comment\n ]")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, }")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\r\n}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, /*comment*/}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42} // comment\n,}")]
+        [InlineData("{\"Property1\": {\"Property1.1\": 42}, // comment\n // comment\n}")]
+        [InlineData("[{\"Property1\": 42}, // comment\n ]")]
+        [InlineData("[{\"Property1\": 42} // comment\n, ]")]
+        [InlineData("[{\"Property1\": 42, // comment\n }, // comment\n ]")]
+        [InlineData("[{\"Property1\": \"42\", // comment\n }, // comment\n ]")]
+        [InlineData("[{\"Property1\": true, // comment\n }, // comment\n ]")]
+        [InlineData("[{\"Property1\": false, // comment\n }, // comment\n ]")]
+        [InlineData("[{\"Property1\": null, // comment\n }, // comment\n ]")]
+        public static void ReadJsonStringsWithCommentsAndTrailingCommas(string jsonString)
+        {
+            byte[] input = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                var options = new JsonReaderOptions() { CommentHandling = commentHandling };
+                var optionsWithTrailing = new JsonReaderOptions() { CommentHandling = commentHandling, AllowTrailingCommas = true };
+
+                var reader = new Utf8JsonReader(input, options);
+                ReadWithCommentsHelper(ref reader, -1, validateThrows: true);
+
+                reader = new Utf8JsonReader(input, optionsWithTrailing);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(input, 1);
+                reader = new Utf8JsonReader(sequence, options);
+                ReadWithCommentsHelper(ref reader, -1, validateThrows: true);
+
+                reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                ReadWithCommentsHelper(ref reader, input.Length);
+
+                for (int splitLocation = 0; splitLocation < input.Length; splitLocation++)
+                {
+                    sequence = JsonTestHelper.CreateSegments(input, splitLocation);
+                    reader = new Utf8JsonReader(sequence, options);
+                    ReadWithCommentsHelper(ref reader, input.Length, validateThrows: true);
+
+                    reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                    ReadWithCommentsHelper(ref reader, input.Length);
+                }
+
+                for (int firstSplit = 0; firstSplit < input.Length; firstSplit++)
+                {
+                    for (int secondSplit = firstSplit; secondSplit < input.Length; secondSplit++)
+                    {
+                        sequence = JsonTestHelper.CreateSegments(input, firstSplit, secondSplit);
+                        reader = new Utf8JsonReader(sequence, options);
+                        ReadWithCommentsHelper(ref reader, input.Length, validateThrows: true);
+
+                        reader = new Utf8JsonReader(sequence, optionsWithTrailing);
+                        ReadWithCommentsHelper(ref reader, input.Length);
+                    }
+                }
+            }
+        }
+
+        private static void ReadWithCommentsHelper(ref Utf8JsonReader reader, int expectedConsumed, bool validateThrows = false)
+        {
+            if (validateThrows)
+            {
+                try
+                {
+                    while (reader.Read())
+                        ;
+                    Assert.True(false, "Expected JsonException was not thrown when reading JSON with trailing commas.");
+                }
+                catch (JsonException ex)
+                {
+                    Assert.Contains("trailing", ex.Message);
+                }
+            }
+            else
+            {
+                while (reader.Read())
+                    ;
+                Assert.Equal(expectedConsumed, reader.BytesConsumed);
+            }
+        }
+
+        [Theory]
         [MemberData(nameof(SingleJsonTokenStartIndex))]
         public static void TestTokenStartIndexMultiSegment_SingleValue(string jsonString, int expectedIndex)
         {