RequestDelegateGenerator should use case-insensitive parsing for enum route parameters

[austindrenski]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

RequestDelegateGenerator should use case-insensitive parsing for enum route parameters.

This has been brought up before, most recently by @aradalvand in #48346, but those previous issues were treated as opportunities for future extensibility, whereas I would like to make the case that this is a bug with user-facing impact well-documented pinch point that should be easy to patch in a post-RDG world.

My tinfoil hat thesis is that this is one of those long-lingering, little frictions that causes far more grief than ever gets reported because it's easy enough to work around, but enums are a first-class primitive and they deserve better support out-of-the-box, especially with the advent of RDG for minimal APIs.

Argument 1: prior art from other primitives with lenient default parsing rules

Enums (unfortunately) didn't receive the same IParsable<T> treatment for net8.0 as other first-class primitives, which means that RDG has to make a choice about which overload of Enum.TryParse(...) to call, as opposed to delegating to IParsable<T>.TryParse(...).

But if we look around at how IParsable<T> was implemented/delegated for other primitives, we can find examples erring on the side of more lenient parsing rules by default.

For example, here are the first few lines of the implementation for IParsable<bool>.TryParse(...):

public static bool TryParse(ReadOnlySpan<char> value, out bool result)
{
    // Boolean.{Try}Parse allows for optional whitespace/null values before and
    // after the case-insensitive "true"/"false", but we don't expect those to
    // be the common case. We check for "true"/"false" case-insensitive in the
    // fast, inlined call path, and then only if neither match do we fall back
    // to trimming and making a second post-trimming attempt at matching those
    // same strings.

While there's nothing to stop anyone from chaotically defining enum members that differ only in casing, it's a broadly respected convention (backed on Roslyn analyzers) for .NET enums to have members whose names differ by more than case.

Further, .NET enums broadly follow the same conventions as other .NET types, meaning the gross majority of (normal) enums in the wild are defined in PascalCase. This means that any use of an enum in a RDG-generated handler will suffer from the unexpected case-sensitivity demonstrated below in Argument 3.

(I anticipate and respect the quibble that this shouldn't be unexpected, since its well-documented, but if minimal APIs are about making .NET easy to work with out-of-the-box, then it's still a problem worth solving.)

Argument 2: RDG has enough context to decide when case-sensitivity makes sense

The web is case-insensitive.
The web should be case-insensitive.
The web was, is, and always shall be a place of chaos, but we still deserve sensible happy-path defaults.

It makes sense to use case-sensitive enum parsing by default for things like query strings, request bodies, etc. However, it makes much less sense to do the same in places like path segments, headers, etc.

RDG should have enough context available to generate case-insensitive parsing for path parameters without having to do so for non-path parameters.

Argument 3: the rest of the routing stack treats path segments as case-insensitive

var builder = WebApplication.CreateSlimBuilder(args);

var app = builder.Build();

app.MapGet("hello/{enum}/world", static (SomeEnum @enum) => Results.Ok());

app.Run();

enum SomeEnum
{
    None,
    Some
}

$ curl -i http://localhost:5000/hello/None/world
HTTP/1.1 200 OK

$ curl -i http://localhost:5000/HELLO/None/world
HTTP/1.1 200 OK

$ curl -i http://localhost:5000/HELLO/None/WORLD
HTTP/1.1 200 OK

$ curl -i http://localhost:5000/hello/none/world
HTTP/1.1 400 Bad Request

The real-world question I received today was: "why is only part of this URL case-sensitive?"

The answer was immediately obvious, because I already knew better than to use raw enums in route templates, but I still managed to walk right into it for the umpteenth time. (...and again, I have a sneaking suspicion that I can't be the only one repeatedly stubbing their toes on this.)

Culprit

aspnetcore/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs

Lines 323 to 335 in b28c8a3

	// If we are parsable we need to emit code based on the enumeration ParsabilityMethod which has a bunch of members
	// which spell out the preferred TryParse usage. This switch statement makes slight variations to them based on
	// which method was encountered.
	Func<string, string, string>? preferredTryParseInvocation = parsabilityMethod switch
	{
	ParsabilityMethod.IParsable => (string inputArgument, string outputArgument) => $$"""GeneratedRouteBuilderExtensionsCore.TryParseExplicit<{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}>({{inputArgument}}!, CultureInfo.InvariantCulture, out var {{outputArgument}})""",
	ParsabilityMethod.TryParseWithFormatProvider => (string inputArgument, string outputArgument) => $$"""{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}.TryParse({{inputArgument}}!, CultureInfo.InvariantCulture, out var {{outputArgument}})""",
	ParsabilityMethod.TryParse => (string inputArgument, string outputArgument) => $$"""{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}.TryParse({{inputArgument}}!, out var {{outputArgument}})""",
	ParsabilityMethod.Enum => (string inputArgument, string outputArgument) => $$"""Enum.TryParse<{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}>({{inputArgument}}!, out var {{outputArgument}})""",
	ParsabilityMethod.Uri => (string inputArgument, string outputArgument) => $$"""Uri.TryCreate({{inputArgument}}!, UriKind.RelativeOrAbsolute, out var {{outputArgument}})""",
	ParsabilityMethod.String => null, // string parameters don't require parsing
	_ => throw new NotImplementedException($"Unreachable! Unexpected {nameof(ParsabilityMethod)}: {parsabilityMethod}"),
	};

Related

• Case insensitivity in enum route binding #45590
• Extend the ability to customize parameter binding for Minimal APIs #35489
• Introduce a more ergonomic way of enabling case-insensitive enum parsing in Minimal API endpoints #48346

Expected Behavior

var builder = WebApplication.CreateSlimBuilder(args);

var app = builder.Build();

app.MapGet("hello/{enum}/world", static (SomeEnum @enum) => Results.Ok());

app.Run();

enum SomeEnum
{
    None,
    Some
}

$ curl -i http://localhost:5000/hello/none/world
HTTP/1.1 200 OK

$ curl -i http://localhost:5000/HELLO/NONE/WORLD
HTTP/1.1 200 OK

Steps To Reproduce

• https://github.com/austindrenski/request-delegate-generator-enum-parsing-case-sensitive

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

No response

[captainsafia]

@austindrenski Thanks for this write-up! It was a good read.

In particular, the argument you've laid out have caused me to reframe this as an issue with the implicit parsing of enums in particular, as opposed to a problem related to parameter binding customization in general.

I think we can safely modify the codegen for enum parsing to assume case-insensitivity by default. It'll be a breaking behavioral change in the (I think, rare) case that people are relying on the current behavior to prevent invalid inputs.

I'll try to see how difficult this is to do...