MediaTypeApiVersionReader to skip application/signed-exchange

[gimlichael]

Is there an existing issue for this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

When I am doing a fast test of various APIs for GET operations, I like to use a browser.

It seems that Chromium (both Edge and Chrome) has introduced the in title referenced header: https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#section-8.6

This "throws" a 400/InvalidApiVersion when encountered and my API is set to use v for version parameter name.

Describe the solution you'd like

If possible, tweak MediaTypeApiVersionReader to be aware of this IANA defined header (application/signed-exchange) and read pass it (eg. skip it).

Additional context

https://github.com/dotnet/aspnet-api-versioning/blob/main/src/Common/src/Common/MediaTypeApiVersionReader.cs

[commonsensesoftware]

This is an interesting case. I certainly haven't seen something else use a v parameter this way, but it's allowed and makes sense. Being aware of specific MIME types, IANA registered or not, doesn't make a lot of sense and would add unnecessary coupling (it would likely be a moving target too). In addition, the proposed RFC doesn't appear to be ratified.

It's plausible to add support to MediaTypeApiVersionReader support filtering for specific media types, but that still seems like a pretty one-off scenario that any individual application can already do today. The idea behind all IApiVersionReader implementations is to provide sensible defaults, but enable extension, customization, or replacement with ease.

MediaTypeApiVersionReader uses v as the default API version parameter, but it can be changed. Changing your design to use an alternate parameter name such as ver, version, or api-version could address your issue and is already available today.

High request rates increase the usage of IApiVersionReader implementations so it's important for them to be performant. I need to think about whether it makes sense for this to be an out-of-the-box feature, where the branching logic is included for everyone, or if it should remain limited to individual customization. 🤔

[gimlichael]

Thank you for your fast response.

Yes, I both acknowledge the workaround and extensibility part, but I also like you to have the opportunity to consider having a MIME type filter - either inclusive or exclusive - I think this would help greatly in spreading the MediaType approach.

That being said, I also reckon that you are a busy guy .. I will consider my options, and if I decide to add my own implementation, I can share it with you here; then you can consider if its viable to serve as inspiration to nudge your default implementation, or it should simply be an implementation I share in my extension library for your framework.

Could also be a Pull Request if you are interested?

[gimlichael]

I choose to opt-in for a generic and whitelist proposal which I have made part of my library.

The MediaTypeApiVersionReader I implemented can be found here: https://github.com/gimlichael/Cuemon/blob/development/src/Cuemon.Extensions.Asp.Versioning/RestfulApiVersionParser.cs, and it is being initialized with there three media types: "application/json", "application/xml" and "application/vnd".

This means that instead of looking through all media types, I decided to only look for the more common negotiation paths of an API.

For me the result is satisfying, and I can keep my v for versioning without triggering the InvalidApiVersion application/signed-exchange media type .

[commonsensesoftware]

Interesting... I've been thinking about this some more. I see that you went with an inclusive selection model. I was thinking exclusive. Funny how something so seemingly innocuous can go sideways.

I've tried to go enable Pay for Play. My general thought process is to add a FilteredMediaTypeApiVersionReader that can support inclusive and exclusive media types. That would keep the existing implementation ⚡ fast, but provide more options. Arguably, there could an InclusiveMediaTypeApiVersionReader and ExclusiveMediaTypeApiVersionReader, but I think that might be overkill. If that level of optimization is really needed, then someone would need to roll their own implementation.

I also noticed that you are using fuzzy inclusive matching a la StartsWith. I don't think I'd support that for general use. I believe that could be covered by the exclusive scenario anyway. It's not very clear which media types would be partially matched - maybe some, maybe all. There could even be another reader that specifically enables partial matches.

application/vnd+<?> has always been excluded because it has no consistency. There's no single format or magical RegEx that might work for everything. Perhaps there is an opportunity to possibly support each of these by combining them through decoration. 🤔 Now that IApiVersionParser has been extracted, perhaps that could be used to parse the value. An implementation could use RegEx or they can provide a more optimal solution.

These nuances can run amuck pretty quickly. It can be hard to judge which scenarios are common enough to bake in versus leave to customization. It's going to require some more thinking, but you might be on to something that would address a couple of edge cases.

[gimlichael]

Cool - glad if I at all have planted a seed 🌱

I agree to the application/vnd .. and this is also why I opt-in for StarsWith.
A proposal that solved my immediate need while still leaving room for individual configuration.

I like how you are already engineering the idea; looking forward to your proposal 😉

[commonsensesoftware]

I think I have a good baseline of what this would look like. I pushed the working branch. You can see the commit with all that support here. There is some serious sorcery🧙🏽‍♂️.

The new MediaTypeApiVersionReaderBuilder will support:

• Multiple parameters (if they vary by media type)
• Including specific media types
• Excluding specific media types
• Matching a version in a media type by regular expression (it's an escape hatch really)
• Matching a version in a media type by a template (wait... what?!)
• Disambiguate from multiple choices

Your scenario:

var builder = new MediaTypeApiVersionReaderBuilder()
var reader = builder
    .Parameter( "v" )
    .Include( "application/json" )
    .Include( "application/xml" )
    .Template( "application/vnd-v{ver}+json" )
    .Template( "application/vnd-v{ver}+xml" )
    .Build();

This provides a lot of flexibility. The more you add, the more evaluations there are per request, but that's kind of a given. This will consider:

• A parameter named v on any media type filtered to:
  • application/json
  • application/xml
• A media type matching the template application/vnd-v{ver}+json where ver is the user-defined parameter name
• A media type matching the template application/vnd-v{ver}+xml where ver is the user-defined parameter name

All considerations are made because there could be multiple in a request. If multiple are allowed, you can disambiguate through the Choose method. By default, no filtering is done. The raw API versions will be ordered by their true ApiVersion ranking. This leads to the extension method ChooseFirstOrDefault, which select the first API version. This would be highest or most current API version requested. For example:

var builder = new MediaTypeApiVersionReaderBuilder()
var reader = builder
    .Choose( versions => versions.Count == 0 ? versions : new[]{ versions[0] } )
    .ChooseFirstOrDefault() // shortcut for the same thing
    .Build();

Give it a peek and see what you think. I think this would cover your scenario and much more.

[gimlichael]

Most impressive work - I am fan!

Thank you for effort - what release are you planning this for?

[commonsensesoftware]

It will be in the next release. I thought it would just be 6.1 fixes, but it's shaping up to be 6.2. With any luck I may finish this weekend.

Added a few minor changes:

• Choose is now Select; parity with other behaviors
• Select now accepts HttpRequest and IReadOnlyList<string> as it might be needed for the decision making
• ChoseFirstOrDefault is now SelectFirstOrDefault
• Added SelectLastOrDefault
• Removed dependency on IApiVersionParser; it was only needed for sorting
  • Now uses ANSI string rules as the case for other readers
  • Users can choose to parse or sort in the Select method