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

UIResponseWriter - Do not encode non-ASCII characters #1984

Merged
merged 1 commit into from
Aug 6, 2023

Conversation

cieciurm
Copy link
Contributor

@cieciurm cieciurm commented Aug 5, 2023

What this PR does / why we need it:

  • By default, the serializer escapes all non-ASCII characters
  • Currently non-ASCII characters are escaped, example: 你好 => \u4F60\u597D
  • This PR removes escaping for all Unicode characters

Which issue(s) this PR fixes: #1557

Special notes for your reviewer:

  • Test added

Does this PR introduce a user-facing change?: No.

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Unit tests passing
  • End-to-end tests passing
  • Extended the documentation
  • Provided sample for the feature

@cieciurm cieciurm changed the title UIResponseWriter - Do not encode unicode characters UIResponseWriter - Do not encode non-ASCII characters Aug 5, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1984 (2449b12) into master (1a9df92) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1984      +/-   ##
==========================================
+ Coverage   66.33%   66.35%   +0.02%     
==========================================
  Files         253      251       -2     
  Lines        8717     8693      -24     
  Branches      626      622       -4     
==========================================
- Hits         5782     5768      -14     
+ Misses       2772     2763       -9     
+ Partials      163      162       -1     
Flag Coverage Δ
Dapr ?
UI 65.92% <100.00%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/HealthChecks.UI.Client/UIResponseWriter.cs 73.80% <100.00%> (+0.63%) ⬆️

... and 3 files with indirect coverage changes

@@ -55,6 +57,7 @@ private static JsonSerializerOptions CreateJsonOptions()
{
AllowTrailingCommas = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
Encoder = JavaScriptEncoder.Create(UnicodeRanges.All),
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 Could be potentially breaking.

@sungam3r sungam3r merged commit 8f193a8 into Xabaril:master Aug 6, 2023
3 checks passed
@cieciurm cieciurm deleted the uiresponsewriter_encoding branch August 7, 2023 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants