-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Add UTF-8 APIs on TextEncoder and add JavascriptEncoder to JsonEncodedText #39415
Conversation
…orefx into AddJavascriptEncoder
/azp run corefx-ci |
Azure Pipelines successfully started running 1 pipeline(s). |
Build failures are occurring because system.text.json is inbox on uap but text.encoding.web is not.
To fix this, make s.t.e.w inbox on uap. See example 10fc6ee |
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.
The change to DecodeScalarValueFromUtf8
is the only thing that unsettled me, everything else is primarily just a nit.
src/System.Text.Encodings.Web/src/System/Text/Unicode/UnicodeHelpers.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.Escaping.cs
Show resolved
Hide resolved
@@ -55,12 +55,12 @@ public static void WritePrimitives() | |||
|
|||
{ | |||
Uri uri = new Uri("https://domain/path"); | |||
Assert.Equal(@"""https:\u002f\u002fdomain\u002fpath""", JsonSerializer.Serialize(uri)); | |||
Assert.Equal(@"""https:\/\/domain\/path""", JsonSerializer.Serialize(uri)); |
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.
Per the offline conversation, there was a desire to change '/' to go through unescaped by default. Feel free to make this change if you wish, otherwise it will eventually be addressed per https://github.com/dotnet/corefx/issues/39445.
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.
I thought it would be good to handle that in a separate PR.
src/System.Text.Encodings.Web/src/System/Text/Encodings/Web/TextEncoder.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Encodings.Web/src/System/Text/Encodings/Web/TextEncoder.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Encodings.Web/src/System/Text/Encodings/Web/TextEncoder.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Encodings.Web/src/System/Text/Encodings/Web/TextEncoder.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.Escaping.cs
Outdated
Show resolved
Hide resolved
95308ef
to
ca2cd26
Compare
…dText (dotnet/corefx#39415) Commit migrated from dotnet/corefx@bbbcac5
Replacement for PR #38947 which was started by @ahsonkhan
This is the first phase of https://github.com/dotnet/corefx/issues/37192; additional phases will allow the custom encoders to be specified in writer and serializer options.