Skip to content

Commit

Permalink
Fix validation check within Utf8JsonWriter for writing a primitive v…
Browse files Browse the repository at this point in the history
…alue after another complete value has been written. (dotnet/corefx#39796)

* 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 dotnet/corefx@47c35fe
  • Loading branch information
ahsonkhan authored and msftbot[bot] committed Jul 29, 2019
1 parent 13fc889 commit 351fb30
Show file tree
Hide file tree
Showing 5 changed files with 353 additions and 22 deletions.
6 changes: 3 additions & 3 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@
<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>
Expand Down Expand Up @@ -423,4 +423,4 @@
<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>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ private static string GetResourceString(ExceptionResource resource, int currentD
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.");
Expand Down Expand Up @@ -623,7 +623,7 @@ internal enum ExceptionResource
CannotStartObjectArrayWithoutProperty,
CannotStartObjectArrayAfterPrimitiveOrClose,
CannotWriteValueWithinObject,
CannotWriteValueAfterPrimitive,
CannotWriteValueAfterPrimitiveOrClose,
CannotWritePropertyWithinArray,
ExpectedJsonTokens,
TrailingCommaNotAllowedBeforeArrayEnd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ private void ValidateWritingValue()
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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public sealed partial class Utf8JsonWriter : IDisposable, IAsyncDisposable
private Memory<byte> _memory;

private bool _inObject;
private bool _isNotPrimitive;
private JsonTokenType _tokenType;
private BitStack _bitStack;

Expand Down Expand Up @@ -116,7 +115,6 @@ public Utf8JsonWriter(IBufferWriter<byte> bufferWriter, JsonWriterOptions option
_memory = default;

_inObject = default;
_isNotPrimitive = default;
_tokenType = default;
_currentDepth = default;
_options = options;
Expand Down Expand Up @@ -152,7 +150,6 @@ public Utf8JsonWriter(Stream utf8Json, JsonWriterOptions options = default)
_memory = default;

_inObject = default;
_isNotPrimitive = default;
_tokenType = default;
_currentDepth = default;
_options = options;
Expand Down Expand Up @@ -249,7 +246,6 @@ private void ResetHelper()
_memory = default;

_inObject = default;
_isNotPrimitive = default;
_tokenType = default;
_currentDepth = default;

Expand Down Expand Up @@ -470,7 +466,6 @@ private void WriteStart(byte token)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
}

private void WriteStartMinimized(byte token)
Expand Down Expand Up @@ -524,7 +519,9 @@ private void ValidateStart()
{
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);
}
Expand Down Expand Up @@ -602,7 +599,6 @@ private void WriteStartHelper(ReadOnlySpan<byte> utf8PropertyName, byte token)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
}

/// <summary>
Expand All @@ -627,7 +623,6 @@ public void WriteStartArray(ReadOnlySpan<byte> utf8PropertyName)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
_tokenType = JsonTokenType.StartArray;
}

Expand All @@ -653,7 +648,6 @@ public void WriteStartObject(ReadOnlySpan<byte> utf8PropertyName)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
_tokenType = JsonTokenType.StartObject;
}

Expand Down Expand Up @@ -772,7 +766,6 @@ public void WriteStartArray(ReadOnlySpan<char> propertyName)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
_tokenType = JsonTokenType.StartArray;
}

Expand All @@ -798,7 +791,6 @@ public void WriteStartObject(ReadOnlySpan<char> propertyName)

_currentDepth &= JsonConstants.RemoveFlagsBitMask;
_currentDepth++;
_isNotPrimitive = true;
_tokenType = JsonTokenType.StartObject;
}

Expand Down
Loading

0 comments on commit 351fb30

Please sign in to comment.