Skip to content
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

Improve Json serialization by always reading a complete object. #2491

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

brentschmaltz
Copy link
Member

@brentschmaltz brentschmaltz commented Feb 13, 2024

Basically, when reading a JsonToken read the entire token and position the reader at the next token.
When reading a JsonObject, the reader was positioned at a JsonTokenType.EndObject after reading.
If the runtime was currently reading a JsonObject, the exit condition of seeing a JsonTokenType.EndObject would be true and the serialization would exit. Same was true for JsonArray.

This will make it very difficult for embedded objects.

Tests were added to ensure that Arrays and Objects placed in the beginning, middle or end of the Json text will be processed correctly.

Test code was removed that was used to understand how the previous internal Newtonsoft, System.Text.Json and external Newtonsoft would work together.
That code is no longer needed as we are dealing with issues as they arise.

@keegan-caruso keegan-caruso added this to the 7.3.2 milestone Feb 13, 2024
@brentschmaltz brentschmaltz force-pushed the brentsch/OidcSerialization branch from 308c541 to 652a173 Compare February 20, 2024 23:22
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Remove test code that is no longer needed.
@twogood
Copy link

twogood commented Feb 23, 2024

I'm wishing for a release with this fix ASAP, been spending days trying to understand and workaround incorrect parsing of "mtls_endpoint_aliases":{} in .well-known/openid-configuration

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

Successfully merging this pull request may close these issues.

5 participants