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

Fix to #33443 - JSON path is not properly formatted #33771

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Fix to #33443 - JSON path is not properly formatted #33771

merged 1 commit into from
Jun 3, 2024

Conversation

maumar
Copy link
Contributor

@maumar maumar commented May 21, 2024

Queries extracting data from JSON properties containing special characters must escape references to those properties in JSON path. Moreover, the values we use in JSON paths must correspond to how we store those properties in database. When we serialize JSON-mapped entity into JSON string, many special (unicode) characters are stored in the \uxxxx form. Lastly in the shaper, when reading json data from the stream we need to make sure to correctly match the property names using Utf8JsonReader. ValueTextEquals un-escapes the property names before comparison, the Span representing the property name must also be un-escaped.

Fixes #33443

@maumar maumar requested review from ajcvickers and roji May 21, 2024 07:17
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

This is tricky stuff....! LGTM (modulo nits/small stuff below).

I'm a bit worried about unescaped JSON that users put in their databases (without going through our SaveChanges). For people doing that, this PR will be a breaking change - the new query and shaper will start failing, if I understand correctly. I still think this is correct as our stuff should work consistently with itself, and we currently save escaped JSON to the database, but this may mean that we should implement #32152 for 9.0 as well, since that would give users a way forward (i.e. they'd disable escaping throughout our entire stack).

Another reason to do #32152 for 9.0 is to make sure the API design here is right and allows opting out of escaping (e.g. in RelationalSqlGenerationHelper).

Queries extracting data from JSON properties containing special characters must escape references to those properties in JSON path. Moreover, the values we use in JSON paths must correspond to how we store those properties in database.
When we serialize JSON-mapped entity into JSON string, many special (unicode) characters are stored in the \uxxxx form.
Lastly in the shaper, when reading json data from the stream we need to make sure to correctly match the property names using Utf8JsonReader. ValueTextEquals un-escapes the property names before comparison, the Span representing the property name must also be un-escaped.

Fixes #33443
@maumar maumar merged commit 32f11fc into main Jun 3, 2024
7 checks passed
@maumar maumar deleted the fix33443 branch June 3, 2024 20:15
@magahl
Copy link

magahl commented Oct 1, 2024

We had an existing model that had a string property (wich was serialized json) in a Json model. Before it encoded it with a \" but now it will be a \\u0022.

When i send back my data to the client (Blazor Server) for DeSerialization it now crashes. since the "string" property contains \u0022 instead of ".

I have searched the internet for an reason and this is the best lead so far, but im not certain this is the cause. All we really have done is upgrade the framework to net 9 RC1

Code used to dersilize:
var model = JSonSerializer.Serialize<Model>(jsonString)

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.

JSON path is not properly formatted.
3 participants