-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Stop escaping Unicode characters unnecessarily in relational JSON #36166
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: AndriySvyryd <[email protected]>
@copilot Call |
Done. I've executed both Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
=> $"'{EscapeSqlLiteral(JsonSerializer.Serialize(value))}'"; | ||
{ | ||
var jsonString = value is string str ? str : JsonSerializer.Serialize(value); | ||
return $"'{EscapeSqlLiteral(jsonString)}'"; |
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.
This doesn't fix the issue, see #32152. The root cause is the use of WriteStringValue
in ToJsonTyped
. We'd need to specify JavaScriptEncoder.UnsafeRelaxedJsonEscaping via JsonSerializerOptions
to avoid escaping. However, this could break users that rely on the characters being escaped, so we should only do it if AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32152", out var enabled32152) && enabled32152
is true
. We can store this value in a private static readonly field called UseOldBehavior32152
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.
You're absolutely right. I've implemented the proper fix as you suggested. Instead of modifying GenerateNonNullSqlLiteral
, I:
- Added
JavaScriptEncoder.UnsafeRelaxedJsonEscaping
toJsonStringReaderWriter.ToJsonTyped
- Added
UseOldBehavior32152
AppContext switch forMicrosoft.EntityFrameworkCore.Issue32152
- When the switch is enabled (old behavior), it uses the original escaping; otherwise uses relaxed escaping
This addresses the root cause in WriteStringValue
and fixes Chinese character escaping while maintaining backward compatibility. Commit: 4fedef5
…Context switch Co-authored-by: AndriySvyryd <[email protected]>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
Issue
When updating JSON columns in SQL Server, Chinese characters (and other Unicode characters) were being double-escaped, causing them to appear as Unicode escape sequences instead of the actual characters. For example, "测试2" would become "\u6D4B\u8BD52" after an update operation.
Root Cause
The issue was in the
SqlServerOwnedJsonTypeMapping.GenerateNonNullSqlLiteral
method, which was callingJsonSerializer.Serialize()
on all values, including those that were already JSON strings from theCollectionToJsonStringConverter
. This caused JSON strings to be serialized twice, leading to double escaping of Unicode characters.Solution
Modified the
GenerateNonNullSqlLiteral
method to check if the input value is already a string (indicating it's already serialized JSON) and use it directly instead of serializing it again:This surgical fix:
Testing
Added a comprehensive test case
Edit_single_property_with_chinese_characters
that verifies:Fixes #32152.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.