Add support for trailing commas within Utf8JsonReader (dotnet#36690)

* Add support for trailing commas with single-segment tests. * Implement single-segment case and update tests. * Update multi-segment path and add tests. * Add more tests, add to state, and update rollback logic.
stark-lang · Apr 8, 2019 · 87fdc75 · 87fdc75
1 parent 268812e
commit 87fdc75
Show file tree

Hide file tree

Showing 9 changed files with 683 additions and 1 deletion.
diff --git a/src/System.Text.Json/ref/System.Text.Json.cs b/src/System.Text.Json/ref/System.Text.Json.cs
@@ -119,6 +119,7 @@ public partial struct JsonReaderOptions
         private int _dummyPrimitive;
         public System.Text.Json.JsonCommentHandling CommentHandling { get { throw null; } set { } }
         public int MaxDepth { get { throw null; } set { } }
+        public bool AllowTrailingCommas { get { throw null; } set { } }
     }
     public partial struct JsonReaderState
     {

diff --git a/src/System.Text.Json/src/Resources/Strings.resx b/src/System.Text.Json/src/Resources/Strings.resx
@@ -312,4 +312,13 @@
   <data name="FormatGuid" xml:space="preserve">
     <value>The JSON value is not in a supported Guid format.</value>
   </data>
+  <data name="ExpectedStartOfPropertyOrValueAfterComment" xml:space="preserve">
+    <value>'{0}' is an invalid start of a property name or value, after a comment.</value>
+  </data>
+  <data name="TrailingCommaNotAllowedBeforeArrayEnd" xml:space="preserve">
+    <value>The JSON array contains a trailing comma at the end which is not supported in this mode. Change the reader options.</value>
+  </data>
+  <data name="TrailingCommaNotAllowedBeforeObjectEnd" xml:space="preserve">
+    <value>The JSON object contains a trailing comma at the end which is not supported in this mode. Change the reader options.</value>
+  </data>
 </root>
diff --git a/src/System.Text.Json/src/System/Text/Json/Reader/JsonReaderOptions.cs b/src/System.Text.Json/src/System/Text/Json/Reader/JsonReaderOptions.cs
@@ -34,5 +34,12 @@ public int MaxDepth
                 _maxDepth = value;
             }
         }
+
+        /// <summary>
+        /// Defines whether an extra comma at the end of a list of JSON values in an object or array
+        /// are allowed (and ignored) within the JSON payload being read.
+        /// By default, it's set to false, and the reader will throw a <exception cref="JsonReaderException"/> if it encounters a trailing comma.
+        /// </summary>
+        public bool AllowTrailingCommas { get; set; }
     }
 }
diff --git a/src/System.Text.Json/src/System/Text/Json/Reader/JsonReaderState.cs b/src/System.Text.Json/src/System/Text/Json/Reader/JsonReaderState.cs
@@ -21,6 +21,7 @@ public struct JsonReaderState
         internal bool _isNotPrimitive;
         internal char _numberFormat;
         internal bool _stringHasEscaping;
+        internal bool _trailingCommaBeforeComment;
         internal JsonTokenType _tokenType;
         internal JsonTokenType _previousTokenType;
         internal JsonReaderOptions _readerOptions;
@@ -64,6 +65,7 @@ public JsonReaderState(JsonReaderOptions options = default)
             _isNotPrimitive = default;
             _numberFormat = default;
             _stringHasEscaping = default;
+            _trailingCommaBeforeComment = default;
             _tokenType = default;
             _previousTokenType = default;
             _readerOptions = options;

diff --git a/src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs b/src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
@@ -36,6 +36,7 @@ public Utf8JsonReader(in ReadOnlySequence<byte> jsonData, bool isFinalBlock, Jso
             _isNotPrimitive = state._isNotPrimitive;
             _numberFormat = state._numberFormat;
             _stringHasEscaping = state._stringHasEscaping;
+            _trailingCommaBeforeComment = state._trailingCommaBeforeComment;
             _tokenType = state._tokenType;
             _previousTokenType = state._previousTokenType;
             _readerOptions = state._readerOptions;
@@ -373,6 +374,10 @@ private bool ConsumeValueMultiSegment(byte marker)
         {
             while (true)
             {
+                Debug.Assert((_trailingCommaBeforeComment && _readerOptions.CommentHandling == JsonCommentHandling.Allow) || !_trailingCommaBeforeComment);
+                Debug.Assert((_trailingCommaBeforeComment && marker != JsonConstants.Slash) || !_trailingCommaBeforeComment);
+                _trailingCommaBeforeComment = false;
+
                 if (marker == JsonConstants.Quote)
                 {
                     return ConsumeStringMultiSegment();
@@ -657,6 +662,8 @@ private bool ConsumeNumberMultiSegment()
 
         private bool ConsumePropertyNameMultiSegment()
         {
+            _trailingCommaBeforeComment = false;
+
             if (!ConsumeStringMultiSegment())
             {
                 return false;
@@ -1531,6 +1538,7 @@ private bool ConsumeNextTokenOrRollbackMultiSegment(byte marker)
             long prevLineNumber = _lineNumber;
             JsonTokenType prevTokenType = _tokenType;
             SequencePosition prevSequencePosition = _currentPosition;
+            bool prevTrailingCommaBeforeComment = _trailingCommaBeforeComment;
             ConsumeTokenResult result = ConsumeNextTokenMultiSegment(marker);
             if (result == ConsumeTokenResult.Success)
             {
@@ -1544,6 +1552,7 @@ private bool ConsumeNextTokenOrRollbackMultiSegment(byte marker)
                 _lineNumber = prevLineNumber;
                 _totalConsumed = prevTotalConsumed;
                 _currentPosition = prevSequencePosition;
+                _trailingCommaBeforeComment = prevTrailingCommaBeforeComment;
             }
             return false;
         }
@@ -1619,19 +1628,38 @@ private ConsumeTokenResult ConsumeNextTokenMultiSegment(byte marker)
 
                 if (_readerOptions.CommentHandling == JsonCommentHandling.Allow && first == JsonConstants.Slash)
                 {
+                    _trailingCommaBeforeComment = true;
                     return ConsumeCommentMultiSegment() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
                 }
 
                 if (_inObject)
                 {
                     if (first != JsonConstants.Quote)
                     {
+                        if (first == JsonConstants.CloseBrace)
+                        {
+                            if (_readerOptions.AllowTrailingCommas)
+                            {
+                                EndObject();
+                                return ConsumeTokenResult.Success;
+                            }
+                            ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeObjectEnd);
+                        }
                         ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
                     }
                     return ConsumePropertyNameMultiSegment() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
                 }
                 else
                 {
+                    if (first == JsonConstants.CloseBracket)
+                    {
+                        if (_readerOptions.AllowTrailingCommas)
+                        {
+                            EndArray();
+                            return ConsumeTokenResult.Success;
+                        }
+                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeArrayEnd);
+                    }
                     return ConsumeValueMultiSegment(first) ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
                 }
             }
@@ -1652,6 +1680,9 @@ private ConsumeTokenResult ConsumeNextTokenMultiSegment(byte marker)
 
         private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentTokenMultiSegment()
         {
+            Debug.Assert(_readerOptions.CommentHandling == JsonCommentHandling.Allow);
+            Debug.Assert(_tokenType == JsonTokenType.Comment);
+
             if (JsonReaderHelper.IsTokenTypePrimitive(_previousTokenType))
             {
                 _tokenType = _inObject ? JsonTokenType.StartObject : JsonTokenType.StartArray;
@@ -1690,6 +1721,12 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentTokenMultiSegment()
 
             if (first == JsonConstants.ListSeparator)
             {
+                // A comma without some JSON value preceding it is invalid
+                if (_previousTokenType <= JsonTokenType.StartObject || _previousTokenType == JsonTokenType.StartArray || _trailingCommaBeforeComment)
+                {
+                    ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyOrValueAfterComment, first);
+                }
+
                 _consumed++;
                 _bytePositionInLine++;
 
@@ -1726,10 +1763,33 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentTokenMultiSegment()
                     first = _buffer[_consumed];
                 }
 
+                if (first == JsonConstants.Slash)
+                {
+                    _trailingCommaBeforeComment = true;
+                    if (ConsumeCommentMultiSegment())
+                    {
+                        goto Done;
+                    }
+                    else
+                    {
+                        goto RollBack;
+                    }
+                }
+
                 if (_inObject)
                 {
                     if (first != JsonConstants.Quote)
                     {
+                        if (first == JsonConstants.CloseBrace)
+                        {
+                            if (_readerOptions.AllowTrailingCommas)
+                            {
+                                EndObject();
+                                goto Done;
+                            }
+                            ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeObjectEnd);
+                        }
+
                         ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
                     }
                     if (ConsumePropertyNameMultiSegment())
@@ -1743,6 +1803,16 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentTokenMultiSegment()
                 }
                 else
                 {
+                    if (first == JsonConstants.CloseBracket)
+                    {
+                        if (_readerOptions.AllowTrailingCommas)
+                        {
+                            EndArray();
+                            goto Done;
+                        }
+                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeArrayEnd);
+                    }
+
                     if (ConsumeValueMultiSegment(first))
                     {
                         goto Done;
@@ -2022,12 +2092,31 @@ private ConsumeTokenResult ConsumeNextTokenUntilAfterAllCommentsAreSkippedMultiS
                 {
                     if (marker != JsonConstants.Quote)
                     {
+                        if (marker == JsonConstants.CloseBrace)
+                        {
+                            if (_readerOptions.AllowTrailingCommas)
+                            {
+                                EndObject();
+                                goto Done;
+                            }
+                            ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeObjectEnd);
+                        }
+
                         ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, marker);
                     }
                     return ConsumePropertyNameMultiSegment() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
                 }
                 else
                 {
+                    if (marker == JsonConstants.CloseBracket)
+                    {
+                        if (_readerOptions.AllowTrailingCommas)
+                        {
+                            EndArray();
+                            goto Done;
+                        }
+                        ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.TrailingCommaNotAllowedBeforeArrayEnd);
+                    }
                     return ConsumeValueMultiSegment(marker) ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
                 }
             }