Utf8JsonWriter does not respect RFC8259

[ycrumeyrolle]

The RFC8259 define as follow the character escaping:
https://tools.ietf.org/html/rfc8259#section-7

All Unicode characters may be placed within the quotation marks, except for the characters that MUST be escaped: quotation mark, reverse solidus, and the control characters (U+0000 through U+001F).

unescaped = %x20-21 / %x23-5B / %x5D-10FFFF
Meaning that the escaped characters MUST be %x00-19 / %x22 / %x5C

Currently, the Utf8JsonWriter is escaping too much:
unescaped = %x00-19 / %x22 / %x26 / %x27 %x2B / %x3C / %x3E / %x5C / %x61 / %x7F / %x80-FF

If the reason is for security, you may propose as a JsonWriterOptions a strict character table, and a safer character table.

Related to #28567
@ahsonkhan

[ycrumeyrolle]

The following RFC is impacted by the current behaviour:
https://tools.ietf.org/html/rfc8417#section-2.4

The JSON {"typ":"secevent+jwt","alg":"none"} should be represented as: eyJ0eXAiOiJzZWNldmVudCtqd3QiLCJhbGciOiJub25lIn0 (Base64-URL representation)

Currently it is represented as:
eyJ0eXAiOiJzZWNldmVudFx1MDAyYmp3dCIsImFsZyI6Im5vbmUifQ

[ycrumeyrolle]

@ahsonkhan are you open for a PR ?
I think to a way to change the JsonWriterHelper.AllowList.

[ahsonkhan]

Firstly, to clarify, the original concern that Utf8JsonWriter does not respect RFC8259 is invalid.

The spec states what must be escaped (i.e. provides a lower bound). It doesn't prohibit what additional characters can be escaped.

Any character may be escaped.

We could have the writer escape every character (including ASCII) and still be RFC compliant. Therefore, by default, we would continue to keep the escaping behavior we have currently as is (for defense in depth).

If the reason is for security, you may propose as a JsonWriterOptions a strict character table, and a safer character table.

Exactly. That is a viable first step to a solution. You might have noticed we got rid of the escape bool on the write methods all together (dotnet/corefx#36961). I am currently working towards a design/proposal that will help address this concern/scenario in general as an extensibility point. It would depend on a JsonWriterOption that accepts a custom JavaScriptEncoder (TextEncoder - https://apisof.net/catalog/System.Text.Encodings.Web.JavaScriptEncoder) to do the escaping. Please give me a week (roughly) to flesh that out. I will submit it for API-review then and would love your feedback on it.

are you open for a PR ?

Once we have a viable design, and you are up for working on it, I would definitely welcome a PR from you and help review it (which would be awesome)!

The JSON {"typ":"secevent+jwt","alg":"none"} should be represented as: eyJ0eXAiOiJzZWNldmVudCtqd3QiLCJhbGciOiJub25lIn0 (Base64-URL representation)

Currently it is represented as:
eyJ0eXAiOiJzZWNldmVudFx1MDAyYmp3dCIsImFsZyI6Im5vbmUifQ

That is an interesting example and the culprit here is us over-escaping the +. By default, we likely will keep escaping it, but the user can opt-out of this behavior. cc @GrabYourPitchforks, what are your thoughts on this? Should we aim to support JSON shown in the Security Event Token RFC, by default, or let this remain an opt-in feature by the user where required?

[ycrumeyrolle]

We could have the writer escape every character (including ASCII) and still be RFC compliant. Therefore, by default, we would continue to keep the escaping behavior we have currently as is (for defense in depth).

You are right, the only issue is

Any character **may** be escaped.

and not

Any character **must** be escaped.

So your latest work with a custom JavaScriptEncoder is on track :)

Please give me a week (roughly) to flesh that out. I will submit it for API-review then and would love your feedback on it.

With pleasure!

[GrabYourPitchforks]

The JSON stack could absolutely accept a custom escaper in its options argument, but the default implementation will continue to escape non-ASCII characters and certain things like '+'. Creating a custom escaper unfortunately is a bit difficult since subclassing the type requires dropping down to C-style code. But we can figure out how to adjust the factory to make it easier to use.

Aside: ASP.NET should absolutely be passing an escaper instance with "allow everything" when they know for a fact the response encoding is UTF-8 and the JSON payload is the entirety of the response body. We just can't do this by default because we don't know how the caller intends on using the output.

[steveharter]

Closing this as dotnet/corefx#37192 addresses this by passing a custom escaper to the writer.