JsonEncodedText: Allow skipping and customizing escaping while writing JSON

[ahsonkhan]

Motivation:

1. If the user wants to write string literals (or known strings), checking to escape is an unnecessary expense.

   • The previous solution (to provide writer method overloads that takes an escape bool) allowed callers to skip validation, and that meant embedded quotes or invalid UTF-8 could be written, in attempts to get performance
   • A benchmark like this regresses 10-15% without having the ability to skip escaping: https://github.com/dotnet/performance/blob/49d604c4b4f637b286f37f1eaa696c4d5785edfe/src/benchmarks/micro/corefx/System.Text.Json/Utf8JsonWriter/Perf.Basic.cs#L52-L83
   • Today, for best performance, users have to store the UTF-16 string literals in static UTF-8 encoded byte[] to avoid the transcoding step. See https://github.com/aspnet/AspNetCore/blob/56c064bd53ed064d82ffb0b5c4c776da87c02326/src/SignalR/common/SignalR.Common/src/Protocol/HandshakeProtocol.cs#L21-L65

2. Today, for defense in depth, the writer over-escapes by default. This includes the characters that is required to be escaped according to the JSON spec, but also all non-ASCII characters, and some within the ASCII range as well (+, &, etc.).

   • That might be too aggressive for some scenarios. For example, users may want to opt-out of non-ASCII escaping
   • Aside from human readability (which is primarily cosmetic), over-escaping does end up affecting the payload size (since it could be a 2-6x expansion depending on the payload)
   • See https://github.com/dotnet/corefx/issues/35385 / https://github.com/dotnet/corefx/issues/34958

API Proposal:

namespace System.Text.Json
{
    // JsonEncodedString? JsonEscapedString/Text/Value?
    public sealed class JsonEncodedText
    {
        // Solves motivation 1: Default escaping (internally uses equivalent to JavaScriptEncoder.Default)

        public JsonEncodedText(string value);
        public JsonEncodedText(ReadOnlySpan<char> value);
        public JsonEncodedText(ReadOnlySpan<byte> utf8Value);

        // Solves motivation 2: Custom escaping via JavaScriptEncoder

        // OR optional encoder parameter where null means default?
        public JsonEncodedText(string value, JavaScriptEncoder encoder);
        public JsonEncodedText(ReadOnlySpan<char> value, JavaScriptEncoder encoder);
        public JsonEncodedText(ReadOnlySpan<byte> utf8Value, JavaScriptEncoder encoder);
        public byte[] EncodedUtf8String { get ; } // OR ReadOnlySpan<byte>?
        public string EncodedString { get ; } // OR ReadOnlySpan<char>?
        public JavaScriptEncoder Encoder { get ; }
        public bool IsAlreadyEncoded(JavaScriptEncoder encoder);
        public override string ToString();
    }

    public partial struct JsonWriterOptions
    {
        // Exists:
        public bool Indented { get; set { } }
        public bool SkipValidation { get; set { } }

        // New:
        public JavaScriptEncoder Encoder { get; set { } }
    }

    public sealed partial class Utf8JsonWriter : System.IDisposable
    {
        // ...
        // other overloads
        // ...
        public void WriteStringValue(string value) { }

        // All overloads that take string get one that takes JsonEncodedText
        public void WriteStringValue(JsonEncodedText value) { }

        public void WriteString(JsonEncodedText propertyName, DateTime value) { }
        public void WriteString(JsonEncodedText propertyName, DateTimeOffset value) { }
        public void WriteString(JsonEncodedText propertyName, Guid value) { }

        public void WriteString(string propertyName, JsonEncodedText value) { }
        public void WriteString(JsonEncodedText propertyName, ReadOnlySpan<byte> utf8Value) { }
        public void WriteString(JsonEncodedText propertyName, ReadOnlySpan<char> value) { }
        public void WriteString(JsonEncodedText propertyName, string value) { }
        public void WriteString(JsonEncodedText propertyName, JsonEncodedText value) { }
        public void WriteString(ReadOnlySpan<char> propertyName, JsonEncodedText value) { }
        public void WriteString(ReadOnlySpan<byte> utf8PropertyName, JsonEncodedText value) { }

        public void WriteStartObject(JsonEncodedText propertyName) { }
        public void WriteStartArray(JsonEncodedText propertyName) { }

        public void WriteBoolean(JsonEncodedText propertyName, bool value) { }
        public void WriteNull(JsonEncodedText propertyName) { }

        public void WriteNumber(JsonEncodedText propertyName, decimal value) { }
        public void WriteNumber(JsonEncodedText propertyName, double value) { }
        public void WriteNumber(JsonEncodedText propertyName, int value) { }
        public void WriteNumber(JsonEncodedText propertyName, long value) { }
        public void WriteNumber(JsonEncodedText propertyName, float value) { }
        [System.CLSCompliantAttribute(false)]
        public void WriteNumber(JsonEncodedText propertyName, uint value) { }
        [System.CLSCompliantAttribute(false)]
        public void WriteNumber(JsonEncodedText propertyName, ulong value) { }
    }
}

Sample usage within JsonWriter:

public void WriteStringValue(JsonEncodedText value)
{
    // skip escaping check, skip escaping
    if (value.IsAlreadyEncoded(Options.Encoder))
    {
        WriteStringValueHelper(value.EncodedUtf8String);
    }
    else
    {
        // Existing UTF-8 write string API, which checks for escaping/etc.
        WriteStringValue(value.EncodedUtf8String);
    }
}

Sample user code:

// Custom encoder optional
private static readonly JsonEncodedText FirstJson = new JsonEncodedText("first");
private static readonly JsonEncodedText LastJson = new JsonEncodedText("last");
private static readonly JsonEncodedText AgeJson = new JsonEncodedText("age", JavaScriptEncoder.Default);

public void WriteBasicJsonEncoded(ArrayBufferWriter<byte> output)
{
    // Default encoder is `JavaScriptEncoder.Default`
    using (var json = new Utf8JsonWriter(output, new JsonWriterOptions { Indented = false, SkipValidation = true, Encoder = JavaScriptEncoder.Default }))
    {
        json.WriteStartObject();
        json.WriteNumber(AgeJson, 42);
        json.WriteString(FirstJson, "John");
        json.WriteString(LastJson, "Smith");
        json.WriteEndObject();
        json.Flush();
    }
}

Questions:

1. Should JsonEncodedText override the encoder passed in to the Utf8JsonWriter via JsonWriterOptions or should there be no escaping?
   • If the Utf8JsonWriter always wins, we don't need the IsAlreadyEncoded.
   • struct or class?
2. Do we want optional ctor arguments that take JavaScriptEncoder or just overloads?
3. Should we return ROS<char>/ROS<byte> or regular string/byte[]?
4. Type names? JsonEncodedString? JsonEscapedString/Text/Value?
5. Should this new JsonWriterOption be exposed via the serializer?
6. Should this implement IEquatable<JsonEncodedText>?

Notes:

• This proposal depends on TextEncoder/JavaScriptEncoder being in-box so that System.Text.Json can reference it.
• It also depends on new UTF-8 specific functionality on the JavaScriptEncoder: https://github.com/dotnet/corefx/issues/34830 / https://github.com/dotnet/corefx/issues/33509
• Here's a prototype implementation: dotnet/corefx@master...ahsonkhan:JsonEncodedString

cc @ycrumeyrolle, @ccnxpt, @GrabYourPitchforks, @steveharter, @bartonjs, @BrennanConroy, @JamesNK, @rynowak, @KrzysztofCwalina, @stephentoub

[steveharter]

That might be too aggressive for some scenarios. For example, users may want to opt-out of non-ASCII escaping

We should consider not over-escaping property names (on a POCO object) if a property name only contains unicode characters and not the 7 or so special ASCII characters.

[steveharter]

Can you also add the JavaScriptEncoder knob to the JsonSerializerOptions

[steveharter]

Also per discussion on performance -- there are cases where the serializer can't statically cache the property name such as data from an IDictionary (where dictionary key = property name). Thus having it be fast (only do one conversion on construction) plus low alloc is important.

However if we have an option to use the JavaScriptEncoder knob for property names too (not just values), then that isn't a concern.

[ahsonkhan]

there are cases where the serializer can't statically cache the property name such as data from an IDictionary (where dictionary key = property name)

Can you link to the code snippet from the serializer relevant to that case just so I have more context?

Can you also add the JavaScriptEncoder knob to the JsonSerializerOptions

Certainly!

[steveharter]

Here's a link to the Dictionary code...

As mentioned earlier, if the JavaScriptEncoder knob works with property names (in addition to values of course) then the serializer would just pass in the uft8 bytes and let the writer do the escape. Otherwise, the serializer would construct a JsonEncodedText as pass it in the for property name (key name) thus it should be performant.

[ahsonkhan]

For reference, based on API-review discussion, here's the approved surface area for JsonEncodedText:

1. Static factory methods rather than ctor.
2. Optional JavascriptEncoder parameter rather than overloads.
3. Return ROS<byte> instead of byte[] and rename the property to EncodedUtf8Bytes. Remove string property.
4. Keep IEquatable, add Equals(obj) and GetHashCode().
5. Struct instead of class.

The proposed changes to JsonWriterOptions and overloads on Utf8JsonWriter are fine as-is.

    public readonly struct JsonEncodedText : IEquatable<JsonEncodedText>
    {
        public ReadOnlySpan<byte> EncodedUtf8Bytes { get { throw null; } }
        public static JsonEncodedText Encode(ReadOnlySpan<byte> utf8Value, JavaScriptEncoder encoder = null) { throw null; }
        public static JsonEncodedText Encode(ReadOnlySpan<char> value, JavaScriptEncoder encoder = null)) { throw null; }
        public static JsonEncodedText Encode(string value, JavaScriptEncoder encoder = null)) { throw null; }
        public override bool Equals(object obj) { throw null; }
        public bool Equals(JsonEncodedText other) { throw null; }
        public override int GetHashCode() { throw null; }
        public override string ToString() { throw null; }
    }

    public sealed partial class JsonSerializerOptions
    {
        // Exists:
        public bool AllowTrailingCommas { get; set; }
        public int DefaultBufferSize { get; set; }
        // ... Other options ...
        public System.Text.Json.JsonCommentHandling ReadCommentHandling { get; set; }
        public bool WriteIndented { get; set; }

        // New:
        public JavaScriptEncoder Encoder { get; set; }
    }

[pranavkm]

In MVC, we have a scenario where we use a user configured JsonSerializerOptions, but need to reset the JavaScriptEncoder to ensure that it always encodes HTML text. This way MVC can preserve a user's choice for things such as naming and indentation, but ensure it's safe to be injected in to HTML.

Could we add a copy constructor to JsonSerializerOptions so that MVC isn't responsible for setting it up correctly?

var options = new JsonSerializerOptions(UserConfiguredOptions) {  Encoder = JavaScriptEncoder.Default };

Here's the Json.Net equivalent: https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonHelper.cs#L82-L88