Added failing test case for issue #37610 - [System.Text.Json] Serializing class that has array of children of the same class throws StackOverflowException#37611
Added failing test case for issue #37610 - [System.Text.Json] Serializing class that has array of children of the same class throws StackOverflowException#37611steveharter merged 6 commits intodotnet:masterfrom oskardudycz:master
Conversation
…that has array of children of the same class throws StackOverflowException
|
|
| TestClassWithArrayOfElementsOfTheSameClass obj = new TestClassWithArrayOfElementsOfTheSameClass(); | ||
|
|
||
| //It shouldn't throw when there is no real cycle reference, and just empty object is created | ||
| Assert.Null(Record.Exception(() => JsonSerializer.ToString(obj))); |
There was a problem hiding this comment.
I believe the xunit [ActiveIssue] attribute is used in cases like this.
There was a problem hiding this comment.
Yes, please update the test to reflect what should happen, and then add [ActiveIssue] to skip it for now.
There was a problem hiding this comment.
@steveharter @ahsonkhan updated accordingly to suggestions.
…lementsOfTheSameClassWithoutCyclesDoesNotFail
| TestClassWithArrayOfElementsOfTheSameClass obj = new TestClassWithArrayOfElementsOfTheSameClass(); | ||
|
|
||
| //It shouldn't throw when there is no real cycle reference, and just empty object is created | ||
| Assert.Throws<StackOverflowException>(() => JsonSerializer.ToString(obj)); |
There was a problem hiding this comment.
Since we are skipping this test anyway, we shouldn't assert that this throws stack overflow exception (that isn't the behavior we want).
We should assert what the comment states, that an empty object is created/returned, or whatever error behavior we expect in this case (maybe throw InvalidOperationException or NotSupported).
There was a problem hiding this comment.
Ok, my bad. I misunderstood previous suggestion.
Now it should be fine.
…ameClassWithoutCyclesDoesNotFail
|
|
||
| //It shouldn't throw when there is no real cycle reference, and just empty object is created | ||
| string json = JsonSerializer.ToString(obj); | ||
| Assert.Equal(@"{}", json); |
There was a problem hiding this comment.
Empty object for certain. This is a legal and useful way to define a data-type 👍
There was a problem hiding this comment.
Empty object for certain
Is there any value in the output being something like the following instead of {} (if it was feasible)?
{"Array":[]}
There was a problem hiding this comment.
That should be empty object or empty object with inner array initialized with undefined value. For sure it shouldn't be empty array, as property was not initialized.
There was a problem hiding this comment.
I haven't tried, but does this reproduce with a Dictionary property:
public TestClassWithDictionaryOfElementsOfTheSameClass
{
public string Dictionary<string, TestClassWithDictionaryOfElementsOfTheSameClass> Values { get; set; }
}Might be useful to also capture this as a test scenario to make sure this works too.
There was a problem hiding this comment.
I think that it's happening for all collections. My initial issue was for arrays (https://github.com/dotnet/corefx/issues/37610) while it's duplicate of the issue with lists (https://github.com/dotnet/corefx/issues/37313)
|
Btw. there are lot of other cases related to that - eg. empty array or array initialized but with objects that don't have cycles. So lot of scenarios are currently missed, that should be checked before releasing package, I can add few more if you'd like. I could also try to provide some fix for that if you give me some hints on where to start. |
Yes, please!
@steveharter, can you provide some hints or guidance? |
[System.Text.Json] Serializing class that has array of children of the same class throws StackOverflowException (dotnet/corefx#37611) Commit migrated from dotnet/corefx@3748331
6 participants
Added failing test case for issue #37610 - Serializing class that has array of children of the same class throws StackOverflowException.
JsonSerializer.ToStringshould not fail when there is no real cycle reference, and just empty object is created, even if class has array of element of the same class.