Skip to content

Commit

Permalink
remove depth checks from the converter layer
Browse files Browse the repository at this point in the history
  • Loading branch information
eiriktsarpalis committed Nov 16, 2021
1 parent 0eb4f70 commit 0e43092
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 23 deletions.
5 changes: 1 addition & 4 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,6 @@
<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>
<data name="SerializerCycleDetected" xml:space="preserve">
<value>A possible object cycle was detected. This can either be due to a cycle or if the object depth is larger than the maximum allowed depth of {0}. Consider using ReferenceHandler.Preserve on JsonSerializerOptions to support cycles.</value>
</data>
<data name="InvalidLeadingZeroInNumber" xml:space="preserve">
<value>Invalid leading zero before '{0}'.</value>
</data>
Expand Down Expand Up @@ -620,4 +617,4 @@
<data name="NoMetadataForTypeCtorParams" xml:space="preserve">
<value>'JsonSerializerContext' '{0}' did not provide constructor parameter metadata for type '{1}'.</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,6 @@ internal override sealed bool TryReadAsObject(ref Utf8JsonReader reader, JsonSer

internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions options, ref WriteStack state)
{
if (writer.CurrentDepth >= options.EffectiveMaxDepth)
{
ThrowHelper.ThrowJsonException_SerializerCycleDetected(options.EffectiveMaxDepth);
}

if (default(T) is null && !HandleNullOnWrite && IsNull(value))
{
// We do not pass null values to converters unless HandleNullOnWrite is true. Null values for properties were
Expand Down Expand Up @@ -510,11 +505,6 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json
return TryWrite(writer, value, options, ref state);
}

if (writer.CurrentDepth >= options.EffectiveMaxDepth)
{
ThrowHelper.ThrowJsonException_SerializerCycleDetected(options.EffectiveMaxDepth);
}

bool isContinuation = state.IsContinuation;
bool success;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,6 @@ public static void ThrowJsonException_SerializationConverterWrite(JsonConverter?
throw new JsonException(SR.Format(SR.SerializationConverterWrite, converter)) { AppendPathInformation = true };
}

[DoesNotReturn]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void ThrowJsonException_SerializerCycleDetected(int maxDepth)
{
throw new JsonException(SR.Format(SR.SerializerCycleDetected, maxDepth)) { AppendPathInformation = true };
}

[DoesNotReturn]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void ThrowJsonException(string? message = null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ public static void WriteCyclicFail(int objectHierarchyDepth, int maxDepth, int e
var options = new JsonSerializerOptions();
options.MaxDepth = maxDepth;
JsonException ex = Assert.Throws<JsonException>(() => JsonSerializer.Serialize(rootObj, options));
InvalidOperationException innerEx = Assert.IsType<InvalidOperationException>(ex.InnerException);

// Exception should contain the path and MaxDepth.
// Since the last Parent property is null, the serializer moves onto the Children property.
string expectedPath = "$" + string.Concat(Enumerable.Repeat(".Parent", expectedPathDepth));
Assert.Contains(expectedPath, ex.Path);
Assert.Contains(effectiveMaxDepth.ToString(), ex.Message);
Assert.Contains(effectiveMaxDepth.ToString(), innerEx.Message);
}

private static TestClassWithCycle CreateObjectHierarchy(int i, int max, TestClassWithCycle previous)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,8 @@ public static void CustomMaxDepth_DepthExceedsLimit_Fails(int maxDepth)

Peano value = Peano.CreateFromNumber(effectiveMaxDepth + 1);
JsonException exn = Assert.Throws<JsonException>(() => JsonSerializer.Serialize(value, options));
Assert.Contains("A possible object cycle was detected", exn.Message);
InvalidOperationException innerExn = Assert.IsType<InvalidOperationException>(exn.InnerException);
Assert.Contains("is equal to or larger than the maximum allowed depth", innerExn.Message);
}

public class Peano
Expand Down

0 comments on commit 0e43092

Please sign in to comment.