dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.83k stars 4.62k forks source link

[API Proposal]: expose funtionality found in: public static bool JsonReaderHelper.UnescapeAndCompare(ReadOnlySpan<byte> utf8Source, ReadOnlySpan<byte> other) #102890

Open brentschmaltz opened 2 months ago

brentschmaltz commented 2 months ago

Background and motivation

https://github.com/dotnet/runtime/blob/1ded19e700f267048a190fd45f949ed492469c1c/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.net8.cs

JsonReaderHelper is internal, this method could be very helpful when reading a JsonWebToken where property values (claims) must be a specific value.

The Uff8JsonReader lets the caller know the value is escaped, but currently to check against the expected value, the user must transcode the uft8 -> utf16 then compare.

A security policy defines what values are acceptable. During Authorization, the security policy is applied to each security token that arrives (many, many, many to 1) Instead, if we transcoded the expected values in a security policy utf16 -> utf8 (once on startup), we could fail fast during deserialization OR keep the utf8 bytes around and check later.

There is a workaround of wrapping the bytes in a new Utf8JsonReader, but that adds allocations (small for sure) and extra logic.

I am not sure where the api would logically fit.

API Proposal

namespace System.Text.Json;

public static class JsonHelper
{
   public static bool Compare(ReadOnlySpan<byte> utf8Source, ReadOnlySpan<byte> utf8Other)
   {
        ...
   }

API Usage

// 'iss' value is available as a span as part of deserialization of a JsonWebToken
public bool ValidateIssuer(ReadOnlySpan<byte> issuer, SecurityPolicy policy)
{
    return JsonHelper.Compare(issuer, policy.Issuer);
}

Alternative Designs

No response

Risks

No response

dotnet-policy-service[bot] commented 2 months ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

halter73 commented 2 months ago

I think I like the current UnescapeAndCompare over Compare as a name since it makes it clear why you want a JSON-specific comparison. It might also make sense to rename utf8Source to escapedUtf8Source to make it clearer that it is the only parameter that gets unescaped unless we plan to unescape both parameters in the new public API.

I know we generally don't like to use "Helper" in public type names. Maybe @bartonjs has ideas on better names.

The current workaround pointed out by @eerhardt is to use Utf8JsonReader which works as long as the escaped source quoted.

var bytes = "\"Fo\u006F\""u8;
var reader = new Utf8JsonReader(bytes);
reader.Read();
Console.WriteLine(reader.ValueTextEquals("Foo"u8)); // Prints "True"
bartonjs commented 2 months ago

Lacking a better target identified by people who work on/with the JSON library, I'd say just having it on Utf8JsonReader is better than making a public "Helper" type.