Utf8JsonReader should be consistent when dealing with more than one JSON payload...

* Utf8JsonReader should be consistent when dealing with more than one JSON
payload.

* Resolve PR feedback - update tests (Assert.Equals, move out of loop,
AssertThrows)

* Remove unnecessary assert since AssertThrows validates that anyway.

Commit migrated from https://github.com/dotnet/corefx/commit/969270f5b9aa921b9ec956be87600d1f2e915403

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 782a86e42104794dbf630bd36b458afa0c54a240..ae7f8cd6461dd97e971ecd27641692d699713568 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
@@ -331,50 +331,13 @@ namespace System.Text.Json
                     }
                     _tokenType = JsonTokenType.Number;
                     _consumed += numberOfBytes;
+                    return true;
                 }
                 else if (!ConsumeValueMultiSegment(first))
                 {
                     return false;
                 }
 
-                // Cannot use HasMoreData since the JSON payload contains a single, non-primitive value
-                // and hence must be handled differently.
-                if (_consumed >= (uint)_buffer.Length)
-                {
-                    goto SetIsNotPrimitiveAndReturnTrue;
-                }
-
-                if (_buffer[_consumed] <= JsonConstants.Space)
-                {
-                    SkipWhiteSpaceMultiSegment();
-                    if (_consumed >= (uint)_buffer.Length)
-                    {
-                        goto SetIsNotPrimitiveAndReturnTrue;
-                    }
-                }
-
-                if (_readerOptions.CommentHandling != JsonCommentHandling.Disallow)
-                {
-                    if (_readerOptions.CommentHandling == JsonCommentHandling.Allow)
-                    {
-                        // This is necessary to avoid throwing when the user has 1 or more comments as the first token
-                        // OR if there is a comment after a single, non-primitive value.
-                        // In this mode, ConsumeValue consumes the comment and we need to return it as a token.
-                        // along with future comments in subsequeunt reads.
-                        if (_tokenType == JsonTokenType.Comment || _buffer[_consumed] == JsonConstants.Slash)
-                        {
-                            return true;
-                        }
-                    }
-                    else
-                    {
-                        Debug.Assert(_readerOptions.CommentHandling == JsonCommentHandling.Skip);
-                        goto SetIsNotPrimitiveAndReturnTrue;
-                    }
-                }
-                ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, _buffer[_consumed]);
-
-            SetIsNotPrimitiveAndReturnTrue:
                 if (_tokenType == JsonTokenType.StartObject || _tokenType == JsonTokenType.StartArray)
                 {
                     _isNotPrimitive = true;
@@ -1611,7 +1574,7 @@ namespace System.Text.Json
                 }
             }
 
-            if (!_isNotPrimitive)
+            if (CurrentDepth == 0)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, marker);
             }
@@ -1718,7 +1681,7 @@ namespace System.Text.Json
                 first = _buffer[_consumed];
             }
 
-            if (!_isNotPrimitive && _tokenType != JsonTokenType.None)
+            if (CurrentDepth == 0 && _tokenType != JsonTokenType.None)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, first);
             }
@@ -2008,7 +1971,7 @@ namespace System.Text.Json
                 }
                 goto Done;
             }
-            else if (!_isNotPrimitive)
+            else if (CurrentDepth == 0)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, marker);
             }

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 fdaacf6c2fccd90379ff4695ac6a99c1c4d8f3fa..767c8be26be4bedb1fea0804452cf57c89929570 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
@@ -729,54 +729,13 @@ namespace System.Text.Json
                     _tokenType = JsonTokenType.Number;
                     _consumed += numberOfBytes;
                     _bytePositionInLine += numberOfBytes;
+                    return true;
                 }
                 else if (!ConsumeValue(first))
                 {
                     return false;
                 }
 
-                // Cannot use HasMoreData since the JSON payload contains a single, non-primitive value
-                // and hence must be handled differently.
-                if (_consumed >= (uint)localBuffer.Length)
-                {
-                    goto SetIsNotPrimitiveAndReturnTrue;
-                }
-
-                if (localBuffer[_consumed] <= JsonConstants.Space)
-                {
-                    SkipWhiteSpace();
-                    if (_consumed >= (uint)localBuffer.Length)
-                    {
-                        goto SetIsNotPrimitiveAndReturnTrue;
-                    }
-                }
-
-                if (_readerOptions.CommentHandling != JsonCommentHandling.Disallow)
-                {
-                    if (_readerOptions.CommentHandling == JsonCommentHandling.Allow)
-                    {
-                        // This is necessary to avoid throwing when the user has 1 or more comments as the first token
-                        // OR if there is a comment after a single, non-primitive value.
-                        // In this mode, ConsumeValue consumes the comment and we need to return it as a token.
-                        // along with future comments in subsequeunt reads.
-                        if (_tokenType == JsonTokenType.Comment || localBuffer[_consumed] == JsonConstants.Slash)
-                        {
-                            return true;
-                        }
-                    }
-                    else
-                    {
-                        Debug.Assert(_readerOptions.CommentHandling == JsonCommentHandling.Skip);
-                        if (_tokenType == JsonTokenType.StartObject || _tokenType == JsonTokenType.StartArray)
-                        {
-                            _isNotPrimitive = true;
-                        }
-                        goto SetIsNotPrimitiveAndReturnTrue;
-                    }
-                }
-                ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, localBuffer[_consumed]);
-
-            SetIsNotPrimitiveAndReturnTrue:
                 if (_tokenType == JsonTokenType.StartObject || _tokenType == JsonTokenType.StartArray)
                 {
                     _isNotPrimitive = true;
@@ -1515,7 +1474,7 @@ namespace System.Text.Json
                 }
             }
 
-            if (!_isNotPrimitive)
+            if (CurrentDepth == 0)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, marker);
             }
@@ -1613,7 +1572,7 @@ namespace System.Text.Json
                 first = _buffer[_consumed];
             }
 
-            if (!_isNotPrimitive && _tokenType != JsonTokenType.None)
+            if (CurrentDepth == 0 && _tokenType != JsonTokenType.None)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, first);
             }
@@ -1888,7 +1847,7 @@ namespace System.Text.Json
                 }
                 goto Done;
             }
-            else if (!_isNotPrimitive)
+            else if (CurrentDepth == 0)
             {
                 ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedEndAfterSingleJson, marker);
             }

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
index 2614e9518d2a6777a31de6b02a824e64b0177087..7d3a6e079566594ed7195fbc9a7620ffb7b2f9fb 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
@@ -661,5 +661,88 @@ namespace System.Text.Json.Tests
                 VerifyReadLoop(ref json, null);
             }
         }
+
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValue))]
+        public static void ReadJsonTokenWithExtraValueMultiSegment(string jsonString)
+        {
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+            ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(utf8, 1);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: false);
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: true);
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValueAndComments))]
+        public static void ReadJsonTokenWithExtraValueAndCommentsMultiSegment(string jsonString)
+        {
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+            ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(utf8, 1);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: false);
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: true);
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValueAndComments))]
+        public static void ReadJsonTokenWithExtraValueAndCommentsAppendedMultiSegment(string jsonString)
+        {
+            jsonString = "  /* comment */  /* comment */  " + jsonString;
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+            ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(utf8, 1);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: false, commentsAppended: true);
+                TestReadTokenWithExtra(sequence, commentHandling, isFinalBlock: true, commentsAppended: true);
+            }
+        }
+
+        private static void TestReadTokenWithExtra(ReadOnlySequence<byte> sequence, JsonCommentHandling commentHandling, bool isFinalBlock, bool commentsAppended = false)
+        {
+            JsonReaderState state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling });
+            Utf8JsonReader reader = new Utf8JsonReader(sequence, isFinalBlock, state);
+
+            if (commentsAppended && commentHandling == JsonCommentHandling.Allow)
+            {
+                Assert.True(reader.Read());
+                Assert.Equal(JsonTokenType.Comment, reader.TokenType);
+                Assert.True(reader.Read());
+                Assert.Equal(JsonTokenType.Comment, reader.TokenType);
+            }
+
+            Assert.True(reader.Read());
+            if (reader.TokenType == JsonTokenType.StartArray || reader.TokenType == JsonTokenType.StartObject)
+            {
+                Assert.True(reader.Read());
+                Assert.Contains(reader.TokenType, new[] { JsonTokenType.EndArray, JsonTokenType.EndObject });
+            }
+
+            JsonTestHelper.AssertThrows<JsonReaderException>(reader, (jsonReader) =>
+            {
+                jsonReader.Read();
+                if (commentHandling == JsonCommentHandling.Allow && jsonReader.TokenType == JsonTokenType.Comment)
+                {
+                    jsonReader.Read();
+                }
+            });
+        }
     }
 }

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.cs b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.cs
index 431b6c5c8a91e3a14f5ca1e751af94fd6ee2a035..d02109d644f666ac0216814e362e11d59a84f83f 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.cs
@@ -1965,6 +1965,86 @@ namespace System.Text.Json.Tests
             }
         }
 
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValue))]
+        public static void ReadJsonTokenWithExtraValue(string jsonString)
+        {
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: false);
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: true);
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValueAndComments))]
+        public static void ReadJsonTokenWithExtraValueAndComments(string jsonString)
+        {
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: false);
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: true);
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonTokenWithExtraValueAndComments))]
+        public static void ReadJsonTokenWithExtraValueAndCommentsAppended(string jsonString)
+        {
+            jsonString = "  /* comment */  /* comment */  " + jsonString;
+            byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);
+
+            foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
+            {
+                if (commentHandling == JsonCommentHandling.Disallow)
+                {
+                    continue;
+                }
+
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: false, commentsAppended: true);
+                TestReadTokenWithExtra(utf8, commentHandling, isFinalBlock: true, commentsAppended: true);
+            }
+        }
+
+        private static void TestReadTokenWithExtra(byte[] utf8, JsonCommentHandling commentHandling, bool isFinalBlock, bool commentsAppended = false)
+        {
+            JsonReaderState state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling });
+            Utf8JsonReader reader = new Utf8JsonReader(utf8, isFinalBlock, state);
+
+            if (commentsAppended && commentHandling == JsonCommentHandling.Allow)
+            {
+                Assert.True(reader.Read());
+                Assert.Equal(JsonTokenType.Comment, reader.TokenType);
+                Assert.True(reader.Read());
+                Assert.Equal(JsonTokenType.Comment, reader.TokenType);
+            }
+
+            Assert.True(reader.Read());
+            if (reader.TokenType == JsonTokenType.StartArray || reader.TokenType == JsonTokenType.StartObject)
+            {
+                Assert.True(reader.Read());
+                Assert.Contains(reader.TokenType, new[] { JsonTokenType.EndArray, JsonTokenType.EndObject });
+            }
+
+            JsonTestHelper.AssertThrows<JsonReaderException>(reader, (jsonReader) =>
+            {
+                jsonReader.Read();
+                if (commentHandling == JsonCommentHandling.Allow && jsonReader.TokenType == JsonTokenType.Comment)
+                {
+                    jsonReader.Read();
+                }
+            });
+        }
+
         public static IEnumerable<object[]> TestCases
         {
             get
@@ -2101,6 +2181,95 @@ namespace System.Text.Json.Tests
             }
         }
 
+        public static IEnumerable<object[]> JsonTokenWithExtraValue
+        {
+            get
+            {
+                return new List<object[]>
+                {
+                    new object[] {"  true  5 "},
+                    new object[] {"  false  5 "},
+                    new object[] {"  null  5 "},
+                    new object[] {"  5  5 "},
+                    new object[] {"  5.1234e-4  5 "},
+                    new object[] {"  \"hello\"  5 "},
+                    new object[] {"  \"hello\"  \"hello\" "},
+                    new object[] {"  [  ]  5 "},
+                    new object[] {"  [  ]  [] "},
+                    new object[] {"  [  ]  {} "},
+                    new object[] {"  { }  5 "},
+                    new object[] {"  { }  [] "},
+                    new object[] {"  { }  {} "},
+                    new object[] {"  [  ]5 "},
+                    new object[] {"  [  ][] "},
+                    new object[] {"  [  ]{} "},
+                    new object[] {"  { }5 "},
+                    new object[] {"  { }[] "},
+                    new object[] {"  { }{} "},
+                    new object[] {"  { }  5.1234e-4"},
+                    new object[] {"  { }  null "},
+                    new object[] {"  { }  false "},
+                    new object[] {"  { }  true "},
+                    new object[] {"  { }  \"hello\" " },
+                    new object[] {"  { },  5 "},
+                    new object[] {"  { },  [] "},
+                    new object[] {"  { },  {} "},
+                    new object[] {"  { },  5.1234e-4"},
+                    new object[] {"  { },  null "},
+                    new object[] {"  { },  false "},
+                    new object[] {"  { },  true "},
+                    new object[] {"  { },  \"hello\" " },
+                };
+            }
+        }
+
+        public static IEnumerable<object[]> JsonTokenWithExtraValueAndComments
+        {
+            get
+            {
+                return new List<object[]>
+                {
+                    new object[] {"  true  /* comment */ 5 "},
+                    new object[] {"  false  /* comment */ 5 "},
+                    new object[] {"  null  /* comment */ 5 "},
+                    new object[] {"  5  /* comment */ 5 "},
+                    new object[] {"  5.1234e-4  /* comment */ 5 "},
+                    new object[] {"  \"hello\"  /* comment */ 5 "},
+                    new object[] {"  \"hello\"  /* comment */ \"hello\" "},
+                    new object[] {"  \"hello\"  // comment \n \"hello\" "},
+                    new object[] {"  [  ]  /* comment */ 5 "},
+                    new object[] {"  [  ]  /* comment */ [ ]"},
+                    new object[] {"  [  ]  /* comment */ { }"},
+                    new object[] {"  [  ]  // comment \n 5 "},
+                    new object[] {"  { }  /* comment */ 5 "},
+                    new object[] {"  { }  /* comment */ [] "},
+                    new object[] {"  { }  /* comment */ {} "},
+                    new object[] {"  [  ]/* comment */5 "},
+                    new object[] {"  [  ]/* comment */[ ]"},
+                    new object[] {"  [  ]/* comment */{ }"},
+                    new object[] {"  [  ]// comment \n5 "},
+                    new object[] {"  { }/* comment */5 "},
+                    new object[] {"  { }/* comment */[] "},
+                    new object[] {"  { }/* comment */{} "},
+                    new object[] {"  { }  /* comment */ 5.1234e-4"},
+                    new object[] {"  { }  /* comment */ null "},
+                    new object[] {"  { }  /* comment */ false "},
+                    new object[] {"  { }  /* comment */ true "},
+                    new object[] {"  { }  /* comment */ \"hello\" "},
+                    new object[] {"  { }  // comment \n \"hello\" "},
+                    new object[] {"  { },  /* comment */ 5 "},
+                    new object[] {"  { },  /* comment */ [] "},
+                    new object[] {"  { },  /* comment */ {} "},
+                    new object[] {"  { },  /* comment */ 5.1234e-4"},
+                    new object[] {"  { },  /* comment */ null "},
+                    new object[] {"  { },  /* comment */ false "},
+                    new object[] {"  { },  /* comment */ true "},
+                    new object[] {"  { },  /* comment */ \"hello\" "},
+                    new object[] {"  { },  // comment \n \"hello\" "},
+                };
+            }
+        }
+
         public static IEnumerable<object[]> InvalidJsonStrings
         {
             get