-
Notifications
You must be signed in to change notification settings - Fork 494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Serialization: Fixes default JsonSerializerSettings #3313
Conversation
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosJsonSeriliazerUnitTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GitHub advisory I believe is wrong.
Newtonsoft actually changed the MaxDepth to 64 not 128 as a reaction of the latest vulnerability.
Please change it to 64 as well.
...zure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Batch/BatchSinglePartitionKeyTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Related to GHSA-5crp-9r3c-p9vr
Quoting:
To mitigate the issue one either need to update Newtonsoft.Json to 13.0.1 or set MaxDepth parameter in the JsonSerializerSettings.
Mirroring JamesNK/Newtonsoft.Json@b6dc05b#diff-c35f33ecfb40348a47f9e6cee6c162ef4cad59d70fb1216d62180dbe59562aa4
We are setting the MaxDepth to 64.
Because bumping the Newtonsoft.Json dependency from 10.0.2 to 13.0.1 might incur into Newtonsoft breaking changes, we opt for applying the change in the JsonSerializerSettings we create in the SDK.
Closes #3312