Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix MaxDepth problems introduced between 12.0.3 and 13.0.1 #2504

Closed
wants to merge 1 commit into from

Conversation

mcon
Copy link

@mcon mcon commented Mar 25, 2021

Fixes #2501 - have verified that the example provided in F# fails with no change and is resolved with the change.

Between 12.0.3 and 13.0.1 looks like _maxDepth = 64; was set in the JsonReader constructor - any inheritors which use this default constructor end up with a max depth of 64 as a result - from the existing tests on max depth, it seems that this setting is not actually required in order to ensure the max depth is respected.

I've also updated a few doc comments which erroneously claim the max depth as 128.

Update: as much as the initial fix (#2462) prevents a DOS attack, it does create other problems, such as 2501 - max depth is a bit tricky throughout the library, one option would be to make it mandatory on the JsonReader perhaps.

@Smaug123
Copy link

Suggest adding MaxDepth tests to verify that you can extend MaxDepth to be bigger than the default?

@JamesNK
Copy link
Owner

JamesNK commented Mar 27, 2021

Thanks for looking at this.

I've fixed this in a different way here #2505

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MaxDepth default value of null or 100 is not respected
3 participants