Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add support for trailing commas within Utf8JsonReader #36690

Merged
merged 4 commits into from
Apr 8, 2019
Merged

Add support for trailing commas within Utf8JsonReader #36690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b6e173
Add support for trailing commas with single-segment tests.
ahsonkhan Apr 8, 2019
aa52a13
Implement single-segment case and update tests.
ahsonkhan Apr 8, 2019
f1b91c8
Update multi-segment path and add tests.
ahsonkhan Apr 8, 2019
76a2a65
Add more tests, add to state, and update rollback logic.
ahsonkhan Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Implement single-segment case and update tests.
  • Loading branch information
@ahsonkhan
ahsonkhan committed Apr 8, 2019
commit aa52a13763bd25353bc63b5aea67c63fe5f40255
1 change: 1 addition & 0 deletions src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public Utf8JsonReader(in ReadOnlySequence<byte> jsonData, bool isFinalBlock, Jso
_sequence = jsonData;
HasValueSequence = false;
ValueSequence = ReadOnlySequence<byte>.Empty;
_trailingCommaBeforeComment = false;

if (jsonData.IsSingleSegment)
{
Expand Down
82 changes: 82 additions & 0 deletions src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public ref partial struct Utf8JsonReader
private bool _isLastSegment;
internal bool _stringHasEscaping;
private readonly bool _isMultiSegment;
private bool _trailingCommaBeforeComment;

private SequencePosition _nextPosition;
private SequencePosition _currentPosition;
Expand Down Expand Up @@ -200,6 +201,7 @@ public Utf8JsonReader(ReadOnlySpan<byte> jsonData, bool isFinalBlock, JsonReader
_totalConsumed = 0;
_isLastSegment = _isFinalBlock;
_isMultiSegment = false;
_trailingCommaBeforeComment = false;

ValueSpan = ReadOnlySpan<byte>.Empty;

Expand Down Expand Up @@ -529,6 +531,15 @@ private void EndObject()
if (!_inObject || _bitStack.CurrentDepth <= 0)
ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.MismatchedObjectArray, JsonConstants.CloseBrace);

if (_trailingCommaBeforeComment)
{
if (!_readerOptions.AllowTrailingCommas)
{
ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.MismatchedObjectArray, JsonConstants.CloseBrace); //TODO: Fix the message
}
_trailingCommaBeforeComment = false;
}

_tokenType = JsonTokenType.EndObject;
ValueSpan = _buffer.Slice(_consumed, 1);

Expand All @@ -554,6 +565,15 @@ private void EndArray()
if (_inObject || _bitStack.CurrentDepth <= 0)
ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.MismatchedObjectArray, JsonConstants.CloseBracket);

if (_trailingCommaBeforeComment)
{
if (!_readerOptions.AllowTrailingCommas)
{
ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.MismatchedObjectArray, JsonConstants.CloseBrace); //TODO: Fix the message
}
_trailingCommaBeforeComment = false;
}

_tokenType = JsonTokenType.EndArray;
ValueSpan = _buffer.Slice(_consumed, 1);

Expand Down Expand Up @@ -798,6 +818,9 @@ private bool ConsumeValue(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 ConsumeString();
Expand Down Expand Up @@ -978,6 +1001,8 @@ private bool ConsumeNumber()

private bool ConsumePropertyName()
{
_trailingCommaBeforeComment = false;

if (!ConsumeString())
{
return false;
Expand Down Expand Up @@ -1526,19 +1551,30 @@ private ConsumeTokenResult ConsumeNextToken(byte marker)

if (_readerOptions.CommentHandling == JsonCommentHandling.Allow && first == JsonConstants.Slash)
{
_trailingCommaBeforeComment = true;
return ConsumeComment() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
}

if (_inObject)
{
if (first != JsonConstants.Quote)
{
if (_readerOptions.AllowTrailingCommas && first == JsonConstants.CloseBrace)
{
EndObject();
return ConsumeTokenResult.Success;
}
ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
}
return ConsumePropertyName() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
}
else
{
if (_readerOptions.AllowTrailingCommas && first == JsonConstants.CloseBracket)
{
EndArray();
return ConsumeTokenResult.Success;
}
return ConsumeValue(first) ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
}
}
Expand All @@ -1559,6 +1595,9 @@ private ConsumeTokenResult ConsumeNextToken(byte marker)

private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentToken()
{
Debug.Assert(_readerOptions.CommentHandling == JsonCommentHandling.Allow);
Debug.Assert(_tokenType == JsonTokenType.Comment);

if (JsonReaderHelper.IsTokenTypePrimitive(_previousTokenType))
{
_tokenType = _inObject ? JsonTokenType.StartObject : JsonTokenType.StartArray;
Expand Down Expand Up @@ -1597,6 +1636,12 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentToken()

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.ExpectedStartOfPropertyOrValueNotFound);
}

_consumed++;
_bytePositionInLine++;

Expand Down Expand Up @@ -1624,10 +1669,29 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentToken()
first = _buffer[_consumed];
}

if (first == JsonConstants.Slash)
{
_trailingCommaBeforeComment = true;
if (ConsumeComment())
{
goto Done;
}
else
{
goto RollBack;
}
}

if (_inObject)
{
if (first != JsonConstants.Quote)
{
if (_readerOptions.AllowTrailingCommas && first == JsonConstants.CloseBrace)
{
EndObject();
goto Done;
}

ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, first);
}
if (ConsumePropertyName())
Expand All @@ -1641,6 +1705,12 @@ private ConsumeTokenResult ConsumeNextTokenFromLastNonCommentToken()
}
else
{
if (_readerOptions.AllowTrailingCommas && first == JsonConstants.CloseBracket)
{
EndArray();
goto Done;
}

if (ConsumeValue(first))
{
goto Done;
Expand Down Expand Up @@ -1905,12 +1975,24 @@ private ConsumeTokenResult ConsumeNextTokenUntilAfterAllCommentsAreSkipped(byte
{
if (marker != JsonConstants.Quote)
{
if (_readerOptions.AllowTrailingCommas && marker == JsonConstants.CloseBrace)
{
EndObject();
goto Done;
}

ThrowHelper.ThrowJsonReaderException(ref this, ExceptionResource.ExpectedStartOfPropertyNotFound, marker);
}
return ConsumePropertyName() ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
}
else
{
if (_readerOptions.AllowTrailingCommas && marker == JsonConstants.CloseBracket)
{
EndArray();
goto Done;
}

return ConsumeValue(marker) ? ConsumeTokenResult.Success : ConsumeTokenResult.NotEnoughDataRollBackState;
}
}
Expand Down
99 changes: 51 additions & 48 deletions src/System.Text.Json/tests/Utf8JsonReaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2130,6 +2130,8 @@ private static void TestReadTokenWithExtra(byte[] utf8, JsonCommentHandling comm
[InlineData("{\"name\": null,}")]
[InlineData("{\"name\": [{},],}")]
[InlineData("{\"first\" : \"value\", \"name\": [{},], \"last\":2 ,}")]
[InlineData("{\"prop\":{\"name\": 1,\"last\":2,},}")]
[InlineData("{\"prop\":[1,2,],}")]
[InlineData("[\"value\",]")]
[InlineData("[1,]")]
[InlineData("[true,]")]
Expand All @@ -2138,36 +2140,33 @@ private static void TestReadTokenWithExtra(byte[] utf8, JsonCommentHandling comm
[InlineData("[{},]")]
[InlineData("[{\"name\": [],},]")]
[InlineData("[1, {\"name\": [],},2 , ]")]
[InlineData("[[1,2,],]")]
[InlineData("[{\"name\": 1,\"last\":2,},]")]
public static void JsonWithValidCommas(string jsonString)
{
byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

{
JsonReaderState state = default;
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);
}

{
var state = new JsonReaderState(options: default);
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);
}

foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
{
var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);
}

foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
{
var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling, AllowTrailingCommas = true });
var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state);

Assert.True(state.Options.AllowTrailingCommas);
Assert.True(reader.CurrentState.Options.AllowTrailingCommas);

while (reader.Read())
{ }
bool allowTrailingCommas = true;
var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling, AllowTrailingCommas = allowTrailingCommas });
TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: false);
}
}

Expand All @@ -2182,40 +2181,28 @@ public static void JsonWithValidCommas(string jsonString)
[InlineData("null,")]
[InlineData("{,}")]
[InlineData("{\"name\": 1,,}")]
[InlineData("{\"name\": 1,,\"last\":2,}")]
[InlineData("[,]")]
[InlineData("[1,,]")]
[InlineData("[1,,2,]")]
public static void JsonWithInvalidCommas(string jsonString)
{
byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
{
var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);
}

foreach (JsonCommentHandling commentHandling in Enum.GetValues(typeof(JsonCommentHandling)))
{
bool allowTrailingCommas = true;
var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = commentHandling, AllowTrailingCommas = allowTrailingCommas });
TrailingCommasHelper(utf8, state, allowTrailingCommas);
TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: true);
}
}

private static void TrailingCommasHelper(byte[] utf8, JsonReaderState state, bool allow)
{
var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state);

Assert.Equal(allow, state.Options.AllowTrailingCommas);
Assert.Equal(allow, reader.CurrentState.Options.AllowTrailingCommas);

JsonTestHelper.AssertThrows<JsonReaderException>(reader, (jsonReader) =>
{
while (jsonReader.Read())
{ }
});
}

[Theory]
[InlineData("{\"name\": \"value\"/*comment*/,/*comment*/}")]
[InlineData("{\"name\": []/*comment*/,/*comment*/}")]
Expand All @@ -2225,6 +2212,8 @@ private static void TrailingCommasHelper(byte[] utf8, JsonReaderState state, boo
[InlineData("{\"name\": null/*comment*/,/*comment*/}")]
[InlineData("{\"name\": [{},]/*comment*/,/*comment*/}")]
[InlineData("{\"first\" : \"value\", \"name\": [{},], \"last\":2 /*comment*/,/*comment*/}")]
[InlineData("{\"prop\":{\"name\": 1,\"last\":2,}/*comment*/,}")]
[InlineData("{\"prop\":[1,2,]/*comment*/,}")]
[InlineData("[\"value\"/*comment*/,/*comment*/]")]
[InlineData("[1/*comment*/,/*comment*/]")]
[InlineData("[true/*comment*/,/*comment*/]")]
Expand All @@ -2233,34 +2222,24 @@ private static void TrailingCommasHelper(byte[] utf8, JsonReaderState state, boo
[InlineData("[{}/*comment*/,/*comment*/]")]
[InlineData("[{\"name\": [],}/*comment*/,/*comment*/]")]
[InlineData("[1, {\"name\": [],},2 /*comment*/,/*comment*/ ]")]
[InlineData("[[1,2,]/*comment*/,]")]
[InlineData("[{\"name\": 1,\"last\":2,}/*comment*/,]")]
public static void JsonWithValidCommasWithComments(string jsonString)
{
byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Allow });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);

state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);

bool allowTrailingCommas = true;
state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Allow, AllowTrailingCommas = allowTrailingCommas });
var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state);

Assert.True(state.Options.AllowTrailingCommas);
Assert.True(reader.CurrentState.Options.AllowTrailingCommas);

while (reader.Read())
{ }

TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: false);

state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = allowTrailingCommas });
reader = new Utf8JsonReader(utf8, isFinalBlock: true, state);

Assert.True(state.Options.AllowTrailingCommas);
Assert.True(reader.CurrentState.Options.AllowTrailingCommas);

while (reader.Read())
{ }
TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: false);
}

[Theory]
Expand All @@ -2274,24 +2253,48 @@ public static void JsonWithValidCommasWithComments(string jsonString)
[InlineData("null/*comment*/,/*comment*/")]
[InlineData("{/*comment*/,/*comment*/}")]
[InlineData("{\"name\": 1/*comment*/,/*comment*/,/*comment*/}")]
[InlineData("{\"name\": 1,/*comment*/,\"last\":2,}")]
[InlineData("[/*comment*/,/*comment*/]")]
[InlineData("[1/*comment*/,/*comment*/,/*comment*/]")]
[InlineData("[1,/*comment*/,2,]")]
public static void JsonWithInvalidCommasWithComments(string jsonString)
{
byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

var state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Allow });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);

state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allow: false, expectThrow: true);

bool allowTrailingCommas = true;
state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Allow, AllowTrailingCommas = allowTrailingCommas });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: true);

state = new JsonReaderState(options: new JsonReaderOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = allowTrailingCommas });
TrailingCommasHelper(utf8, state, allow: false);
TrailingCommasHelper(utf8, state, allowTrailingCommas, expectThrow: true);
}

private static void TrailingCommasHelper(byte[] utf8, JsonReaderState state, bool allow, bool expectThrow)
{
var reader = new Utf8JsonReader(utf8, isFinalBlock: true, state);

Assert.Equal(allow, state.Options.AllowTrailingCommas);
Assert.Equal(allow, reader.CurrentState.Options.AllowTrailingCommas);

if (expectThrow)
{
JsonTestHelper.AssertThrows<JsonReaderException>(reader, (jsonReader) =>
{
while (jsonReader.Read())
;
});
}
else
{
while (reader.Read())
;
}
}

public static IEnumerable<object[]> TestCases
Expand Down