-
Notifications
You must be signed in to change notification settings - Fork 323
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
Mitigate the usage of JsonConvert.DefaultSettings in user code #4581
Conversation
…lue as possible. this helps mitigate if user code sets JsonConvert.DefaultSettings
note that the contributing guideline ask that there be an issue discussed and approved before a pull request is created, #1360 is closed, but the last comment is a solicitation for a contribution to fix the problem, if you need an open issue, let me know and I can create one. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@microsoft-github-policy-service agree company="General Dynamics" |
is there anything I need to do on that test failure? it appears that that particular test often fails with that error. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Nice, the tests finally passed, I will have to have a proper look at that code. Everything is relying on this. It would also be nice to have an acceptance test that helps us not regress this. Would you be able to add one? |
I would like to, however my system is not longer able to build successfully (we updated vs recently, maybe that), so not sure when i can get to it, don't have time to debug. |
updated existing test for default settings to clear `DefaultSettings` after use, and to use multiple versions for serialize
…4/vstest into issue-1360-testcase
i figured out how codespaces worked, so no more using the CI to check my tests (sorry about that). |
I verified that the tests I added fail in main prior to my changes. |
I still did not get to this, sorry :/ |
I think it is safe to merge, thank you! |
Description
!1881 removed direct usage of
JsonConvert.DefaultSettings
but the serialization was still sensitive to user code changing this value due to the use ofJsonConvert.Serialize
andJsonConvert.Deserialize
during fast serialization/deserializationRelated issue
Fixes #1360