Fix validation check within Utf8JsonWriter for writing a primitive value after anoth...
authorAhson Khan <ahson_ahmedk@yahoo.com>
Mon, 29 Jul 2019 04:06:03 +0000 (21:06 -0700)
committermsftbot[bot] <48340428+msftbot[bot]@users.noreply.github.com>
Mon, 29 Jul 2019 04:06:03 +0000 (04:06 +0000)
* Fix validation check within Utf8JsonWriter for writing a primitive
value.

* Update the unit test to pass in JsonValueKind as an inlinedata option.

* Test cleanup.

* Remove use of _isNotPrimitive as that is redundant and unnecessary.

* Some more test clean up - validate bytespending moves.

* More test clean up - ensure bytespending remains as is when we throw.

* Invert check order to rely on short-circuiting.

* Address feedback and add comment related tests.

Commit migrated from https://github.com/dotnet/corefx/commit/47c35fe385c18d5f0a4ceacea381f790db472ba2

src/libraries/System.Text.Json/src/Resources/Strings.resx
src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Helpers.cs
src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs

index b40292d..de53558 100644 (file)
   <data name="CannotWritePropertyAfterProperty" xml:space="preserve">
     <value>Cannot write a JSON property name following another property name. A JSON value is missing.</value>
   </data>
-  <data name="CannotWriteValueAfterPrimitive" xml:space="preserve">
-    <value>Cannot write a JSON value after a single JSON value. Current token type is '{0}'.</value>
+  <data name="CannotWriteValueAfterPrimitiveOrClose" xml:space="preserve">
+    <value>Cannot write a JSON value after a single JSON value or outside of an existing closed object/array. Current token type is '{0}'.</value>
   </data>
   <data name="CannotWriteValueWithinObject" xml:space="preserve">
     <value>Cannot write a JSON value within an object without a property name. Current token type is '{0}'.</value>
   <data name="FormatUInt16" xml:space="preserve">
     <value>Either the JSON value is not in a supported format, or is out of bounds for a UInt16.</value>
   </data>
-</root>
\ No newline at end of file
+</root>
index 267cad6..cbab677 100644 (file)
@@ -496,8 +496,8 @@ namespace System.Text.Json
                         SR.Format(SR.CannotWritePropertyAfterProperty) :
                         SR.Format(SR.CannotWritePropertyWithinArray, tokenType);
                     break;
-                case ExceptionResource.CannotWriteValueAfterPrimitive:
-                    message = SR.Format(SR.CannotWriteValueAfterPrimitive, tokenType);
+                case ExceptionResource.CannotWriteValueAfterPrimitiveOrClose:
+                    message = SR.Format(SR.CannotWriteValueAfterPrimitiveOrClose, tokenType);
                     break;
                 default:
                     Debug.Fail($"The ExceptionResource enum value: {resource} is not part of the switch. Add the appropriate case and exception message.");
@@ -623,7 +623,7 @@ namespace System.Text.Json
         CannotStartObjectArrayWithoutProperty,
         CannotStartObjectArrayAfterPrimitiveOrClose,
         CannotWriteValueWithinObject,
-        CannotWriteValueAfterPrimitive,
+        CannotWriteValueAfterPrimitiveOrClose,
         CannotWritePropertyWithinArray,
         ExpectedJsonTokens,
         TrailingCommaNotAllowedBeforeArrayEnd,
index 7afbce4..04f5793 100644 (file)
@@ -26,9 +26,11 @@ namespace System.Text.Json
                 else
                 {
                     Debug.Assert(_tokenType != JsonTokenType.PropertyName);
-                    if (!_isNotPrimitive && _tokenType != JsonTokenType.None)
+
+                    // It is more likely for CurrentDepth to not equal 0 when writing valid JSON, so check that first to rely on short-circuiting and return quickly.
+                    if (CurrentDepth == 0 && _tokenType != JsonTokenType.None)
                     {
-                        ThrowHelper.ThrowInvalidOperationException(ExceptionResource.CannotWriteValueAfterPrimitive, currentDepth: default, token: default, _tokenType);
+                        ThrowHelper.ThrowInvalidOperationException(ExceptionResource.CannotWriteValueAfterPrimitiveOrClose, currentDepth: default, token: default, _tokenType);
                     }
                 }
             }
index 07df584..329eab4 100644 (file)
@@ -48,7 +48,6 @@ namespace System.Text.Json
         private Memory<byte> _memory;
 
         private bool _inObject;
-        private bool _isNotPrimitive;
         private JsonTokenType _tokenType;
         private BitStack _bitStack;
 
@@ -116,7 +115,6 @@ namespace System.Text.Json
             _memory = default;
 
             _inObject = default;
-            _isNotPrimitive = default;
             _tokenType = default;
             _currentDepth = default;
             _options = options;
@@ -152,7 +150,6 @@ namespace System.Text.Json
             _memory = default;
 
             _inObject = default;
-            _isNotPrimitive = default;
             _tokenType = default;
             _currentDepth = default;
             _options = options;
@@ -249,7 +246,6 @@ namespace System.Text.Json
             _memory = default;
 
             _inObject = default;
-            _isNotPrimitive = default;
             _tokenType = default;
             _currentDepth = default;
 
@@ -470,7 +466,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
         }
 
         private void WriteStartMinimized(byte token)
@@ -524,7 +519,9 @@ namespace System.Text.Json
             {
                 Debug.Assert(_tokenType != JsonTokenType.PropertyName);
                 Debug.Assert(_tokenType != JsonTokenType.StartObject);
-                if (_tokenType != JsonTokenType.None && (!_isNotPrimitive || CurrentDepth == 0))
+
+                // It is more likely for CurrentDepth to not equal 0 when writing valid JSON, so check that first to rely on short-circuiting and return quickly.
+                if (CurrentDepth == 0 && _tokenType != JsonTokenType.None)
                 {
                     ThrowHelper.ThrowInvalidOperationException(ExceptionResource.CannotStartObjectArrayAfterPrimitiveOrClose, currentDepth: default, token: default, _tokenType);
                 }
@@ -602,7 +599,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
         }
 
         /// <summary>
@@ -627,7 +623,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
             _tokenType = JsonTokenType.StartArray;
         }
 
@@ -653,7 +648,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
             _tokenType = JsonTokenType.StartObject;
         }
 
@@ -772,7 +766,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
             _tokenType = JsonTokenType.StartArray;
         }
 
@@ -798,7 +791,6 @@ namespace System.Text.Json
 
             _currentDepth &= JsonConstants.RemoveFlagsBitMask;
             _currentDepth++;
-            _isNotPrimitive = true;
             _tokenType = JsonTokenType.StartObject;
         }
 
index eeeb70b..2e290ed 100644 (file)
@@ -12,6 +12,7 @@ using System.IO.Pipelines;
 using System.Collections.Generic;
 using System.Text.Encodings.Web;
 using System.Text.Unicode;
+using System.Diagnostics;
 
 namespace System.Text.Json.Tests
 {
@@ -858,6 +859,342 @@ namespace System.Text.Json.Tests
         }
 
         [Theory]
+        [InlineData(JsonValueKind.Array, true, true)]
+        [InlineData(JsonValueKind.Array, true, false)]
+        [InlineData(JsonValueKind.Array, false, true)]
+        [InlineData(JsonValueKind.Array, false, false)]
+        [InlineData(JsonValueKind.Object, true, true)]
+        [InlineData(JsonValueKind.Object, true, false)]
+        [InlineData(JsonValueKind.Object, false, true)]
+        [InlineData(JsonValueKind.Object, false, false)]
+        [InlineData(JsonValueKind.String, true, true)]
+        [InlineData(JsonValueKind.String, true, false)]
+        [InlineData(JsonValueKind.String, false, true)]
+        [InlineData(JsonValueKind.String, false, false)]
+        [InlineData(JsonValueKind.Number, true, true)]
+        [InlineData(JsonValueKind.Number, true, false)]
+        [InlineData(JsonValueKind.Number, false, true)]
+        [InlineData(JsonValueKind.Number, false, false)]
+        [InlineData(JsonValueKind.True, true, true)]
+        [InlineData(JsonValueKind.True, true, false)]
+        [InlineData(JsonValueKind.True, false, true)]
+        [InlineData(JsonValueKind.True, false, false)]
+        [InlineData(JsonValueKind.False, true, true)]
+        [InlineData(JsonValueKind.False, true, false)]
+        [InlineData(JsonValueKind.False, false, true)]
+        [InlineData(JsonValueKind.False, false, false)]
+        [InlineData(JsonValueKind.Null, true, true)]
+        [InlineData(JsonValueKind.Null, true, false)]
+        [InlineData(JsonValueKind.Null, false, true)]
+        [InlineData(JsonValueKind.Null, false, false)]
+        public void InvalidJsonDueToWritingMultipleValues(JsonValueKind kind, bool formatted, bool skipValidation)
+        {
+            var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
+            var output = new ArrayBufferWriter<byte>(1024);
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartObject(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartObject("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartArray(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteEndObject(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteEndArray(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WritePropertyName("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteString("key", "foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStringValue("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNumber("key", 123), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNumberValue(123), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBoolean("key", true), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBooleanValue(true), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBoolean("key", false), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBooleanValue(false), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNull("key"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNullValue(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind);
+                // Writing a comment after any preamable is valid (even when skipValidation is false)
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteCommentValue("some comment"), skipValidation: true);
+            }
+        }
+
+        [Theory]
+        [InlineData(JsonValueKind.Array, true, true)]
+        [InlineData(JsonValueKind.Array, true, false)]
+        [InlineData(JsonValueKind.Array, false, true)]
+        [InlineData(JsonValueKind.Array, false, false)]
+        [InlineData(JsonValueKind.Object, true, true)]
+        [InlineData(JsonValueKind.Object, true, false)]
+        [InlineData(JsonValueKind.Object, false, true)]
+        [InlineData(JsonValueKind.Object, false, false)]
+        [InlineData(JsonValueKind.String, true, true)]
+        [InlineData(JsonValueKind.String, true, false)]
+        [InlineData(JsonValueKind.String, false, true)]
+        [InlineData(JsonValueKind.String, false, false)]
+        [InlineData(JsonValueKind.Number, true, true)]
+        [InlineData(JsonValueKind.Number, true, false)]
+        [InlineData(JsonValueKind.Number, false, true)]
+        [InlineData(JsonValueKind.Number, false, false)]
+        [InlineData(JsonValueKind.True, true, true)]
+        [InlineData(JsonValueKind.True, true, false)]
+        [InlineData(JsonValueKind.True, false, true)]
+        [InlineData(JsonValueKind.True, false, false)]
+        [InlineData(JsonValueKind.False, true, true)]
+        [InlineData(JsonValueKind.False, true, false)]
+        [InlineData(JsonValueKind.False, false, true)]
+        [InlineData(JsonValueKind.False, false, false)]
+        [InlineData(JsonValueKind.Null, true, true)]
+        [InlineData(JsonValueKind.Null, true, false)]
+        [InlineData(JsonValueKind.Null, false, true)]
+        [InlineData(JsonValueKind.Null, false, false)]
+        public void InvalidJsonDueToWritingMultipleValuesWithComments(JsonValueKind kind, bool formatted, bool skipValidation)
+        {
+            var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
+            var output = new ArrayBufferWriter<byte>(1024);
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartObject(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartObject("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStartArray(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteEndObject(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteEndArray(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WritePropertyName("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteString("key", "foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteStringValue("foo"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNumber("key", 123), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNumberValue(123), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBoolean("key", true), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBooleanValue(true), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBoolean("key", false), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteBooleanValue(false), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNull("key"), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteNullValue(), skipValidation);
+            }
+
+            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            {
+                WritePreamble(jsonUtf8, kind, addComments: true);
+                // Writing a comment after any preamable is valid (even when skipValidation is false)
+                ValidateAction(jsonUtf8, () => jsonUtf8.WriteCommentValue("some comment"), skipValidation: true);
+            }
+        }
+
+        private void WritePreamble(Utf8JsonWriter writer, JsonValueKind kind, bool addComments = false)
+        {
+            Debug.Assert(writer.BytesCommitted == 0 && writer.BytesPending == 0 && writer.CurrentDepth == 0 && kind != JsonValueKind.Undefined);
+
+            if (addComments)
+            {
+                writer.WriteCommentValue(" comment value before ");
+            }
+
+            switch (kind)
+            {
+                case JsonValueKind.Object:
+                    writer.WriteStartObject();
+                    writer.WriteEndObject();
+                    break;
+                case JsonValueKind.Array:
+                    writer.WriteStartArray();
+                    writer.WriteEndArray();
+                    break;
+                case JsonValueKind.String:
+                    writer.WriteStringValue("foo");
+                    break;
+                case JsonValueKind.Number:
+                    writer.WriteNumberValue(1);
+                    break;
+                case JsonValueKind.True:
+                    writer.WriteBooleanValue(true);
+                    break;
+                case JsonValueKind.False:
+                    writer.WriteBooleanValue(false);
+                    break;
+                case JsonValueKind.Null:
+                    writer.WriteNullValue();
+                    break;
+                default:
+                    Debug.Fail($"Invalid JsonValueKind passed in '{kind}'.");
+                    break;
+            }
+
+            if (addComments)
+            {
+                writer.WriteCommentValue(" comment value after ");
+            }
+        }
+
+        private void ValidateAction(Utf8JsonWriter writer, Action action, bool skipValidation)
+        {
+            int originalBytesPending = writer.BytesPending;
+            if (skipValidation)
+            {
+                action.Invoke();
+                Assert.NotEqual(originalBytesPending, writer.BytesPending);
+            }
+            else
+            {
+                Assert.Throws<InvalidOperationException>(action);
+                Assert.Equal(originalBytesPending, writer.BytesPending);
+            }
+        }
+
+        [Theory]
         [InlineData(true, true)]
         [InlineData(true, false)]
         [InlineData(false, true)]
@@ -5295,9 +5632,9 @@ namespace System.Text.Json.Tests
 
                 ReadOnlySpan<char> nullStringSpan = nullString.AsSpan();
                 charSpanAction(writer, nullStringSpan, value);
-            
+
                 byteSpanAction(writer, ReadOnlySpan<byte>.Empty, value);
-                
+
                 writer.WriteEndObject();
                 writer.Flush();
             }
@@ -5385,7 +5722,7 @@ namespace System.Text.Json.Tests
         {
             var output = new ArrayBufferWriter<byte>(1024);
             string nullString = null;
-            
+
             using (var writer = new Utf8JsonWriter(output))
             {
                 writer.WriteStartArray();