Add Route Short Circuit middleware

[Tratcher]

Background and Motivation

Browsers and bots often probe servers for well known paths like favicon.ico. Normally the middleware pipeline runs to completion before rejecting a request as 404.

Services can reduce resource usage and log noise by filtering out known requests early in the pipeline. We can provide a middleware to do this efficiently.

Proposed API

Project/Assembly: Microsoft.AspNetCore.Routing

namespace Microsoft.Extensions.DependencyInjection;

+ public static class RouteShortCircuitIServiceCollectionExtensions
{
+     public static IServiceCollection AddRouteShortCircuit(this IServiceCollection services, Action<RouteShortCircuitOptions> configure) { }
}

namespace Microsoft.AspNetCore.Builder;

+ public static class RouteShortCircuitIApplicationBuilderExtensions
+ {
+     public static IApplicationBuilder UseRouteShortCircuit(this IApplicationBuilder builder) { }
+ }

namespace Microsoft.AspNetCore.Routing;

+ public class RouteShortCircuitOptions
+ {
+     // Exact path matches, empty by default
+     public ISet<string> Paths { get; } = new HashSet<string>(StringComparer.OridinalIgnoreCase);
+     // Any path that starts with these, empty by default
+     public ISet<string> PathsStartWith { get; } = new HashSet<string>(StringComparer.OridinalIgnoreCase);
+     public RequestDelegate OnRejected { get; set; }
+ }

Usage Examples

services.AddRouteShortCircuit(options =>
{
    options.Paths.Add("/favicon.ico");
    options.OnRejected(context => { /* metrics */ });
});
...
app.UseRouteShortCircuit();

Alternative:

Build this into UseRouting (#43642) and have it terminate immediately with a 404 if such an endpoint is matched.

We could also allow short circuiting to specify an endpoint/delegate to execute to produce a custom response.

Project/Assembly: Microsoft.AspNetCore.Routing

namespace Microsoft.Extensions.DependencyInjection;

+ public static class RouteShortCircuitEndpointConventionBuilderExtensions
{
+     public static IEndpointConventionBuilder ShortCircuit(this IEndpointConventionBuilder builder) { }
}

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[martincostello]

Could this be extended to support configuring file extensions too?

I'm thinking of a use case where you get bots probing for PHP stuff and WordPress sites etc., and you could just blanket short-circuit all requests for *.php in addition to paths that start with /wp/(?).

[davidfowl]

I wonder if this should be built on top of routing instead. This middleware would the short circuit if the appropriate endpoint matched with specific metadata. Another ref count for #43642.

Strawman would look like;

app.MapMetadata("/{path}.php").ShortCircuit(); // Add the short circuit metadata to this route pattern

// This middleware short circuits the request requests
app.UseRouteShortCircuit();

[Tratcher]

Why even have UseRouteShortCircuit? Why not have UseRouting do it?

The original intent was to avoid the costs of static files lookups, route tables, logging & telemetry, etc.. Building into the route table limits the effectiveness of the feature, though it does make match definitions easier.

[davidfowl]

Agreed @Tratcher! Something we can measure though, right?

[tekian]

@davidfowl Routing comes later in the pipeline, right? After authentication, throttling, telemetry, etc. Ideally, we'd want to zero the costs for these requests. We don't need to attempt to authenticate them, we don't need to consider throttling them, we are not interested in capturing telemetry for them. Short circuiting as soon as the requests hits the middleware pipeline does that.

[Tratcher]

@tekian these days we're putting routing almost first, second only to exception handlers. Auth and throttling come later because they want to consider which endpoint the request was routed to.

Telemetry is custom middleware in this case? That goes wherever you put it.

[tekian]

@Tratcher I'm not aware of a change that would put routing almost first. Can you share more information on this? Usually what I have seen is that services assemble route-agnostic middlewares in-between exception handling and routing. That way, they can apply shared policies such as validate certificate issuer of a remote party validate a request has a token or apply per-IP throttling policy. For that, we don't need to know an endpoint or a user associated with the request. And then, per-endpoint or per-user, we may choose to apply additional authorization or throttling as filters. Short circuit was intended to stop even shared policies from applying.

[Tratcher]

When you use the new WebApplicationBuilder class it automatically adds UseDeveloperExceptionPage and UseRouting at the start of the pipeline if you did not specify them.

aspnetcore/src/DefaultBuilder/src/WebApplicationBuilder.cs

Lines 148 to 165 in 83d6c56

	app.UseDeveloperExceptionPage();
	}
	// Wrap the entire destination pipeline in UseRouting() and UseEndpoints(), essentially:
	// destination.UseRouting()
	// destination.Run(source)
	// destination.UseEndpoints()
	// Set the route builder so that UseRouting will use the WebApplication as the IEndpointRouteBuilder for route matching
	app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication);
	// Only call UseRouting() if there are endpoints configured and UseRouting() wasn't called on the global route builder already
	if (_builtApplication.DataSources.Count > 0)
	{
	// If this is set, someone called UseRouting() when a global route builder was already set
	if (!_builtApplication.Properties.TryGetValue(EndpointRouteBuilderKey, out var localRouteBuilder))
	{
	app.UseRouting();

I understand wanting to minimize the costs. We should see what the cost of moving routing up front would be for apps like yours to see if this approach makes sense.

[davidfowl]

@tekian That makes sense, and it sounds like avoiding those things would still happen.

[Tratcher]

We could also do both:

• Add a middleware with simple options for filtering. E.g. fixed paths, that's what customers have asked for so far.
• Add a feature to routing that can short circuit based on its advanced pattern matching.

[davidfowl]

When do you choose one over the other?

[Tratcher]

Middleware: Use when you have expensive things before routing and only need to do simple matches.
Routing: When you can put the expensive things after routing, and might need to do complex matching. This can also be used when doing simple matching.

The permutation that doesn't work is needing to do complex matching before routing.

[davidfowl]

I need to see some examples and benchmarks to be convinced we need both.

[tekian]

@Tratcher Thanks! Looking into the code, even if you use WebApplicationBuilder and don't customize your middleware pipeline and as a result get a default middleware pipeline from the builder, there are still features like authN/Z for which you may provide handlers that may take time to execute. Benchmarking a handler that does return success will give you negligable results, of course. Regardless, I don't think it's safe to assume that nobody will want to customize middleware pipeline.

[tekian]

Here is another way how to look at it: It may not be .NET's code that is slow. It may be a 3P library that provides a middleware that isn't particularly efficient for all requests. 😁

Having two ways to configure the same thing doesn't seem right.

How about:

• Metadata are stored in routing.
• Routing does short circuiting by default.
• For cases where this is not enough, we can provide additional extension method UseShortCircuit that would re-use metadata/logic from the routing and perform it pre-emptively.

@davidfowl Is that the way you were thinking about it?

[Tratcher]

• For cases where this is not enough, we can provide additional extension method UseShortCircuit that would re-use metadata/logic from the routing and perform it pre-emptively.

How is this different from moving UseRouting earlier? It seems like it would have to do the same work.

[tekian]

Oh I see what you mean. We'd advise to add routing as one of the first middlewares, claiming and assuming that the overhead to compute a route is negligent even for cases where we want to short circuit. And we'd sequence the rest of the middlewares after it, even those that are agnostic of the path.

[halter73]

API Review Notes:

• Do we like new middleware or integrating with routing? It feels like we're more interested in routing integration. It's less new API for defining patterns and custom handlers. We can have an IEndpointConventionBuilder extension method which runs the Endpoint.RequestDelegate immediately in UseRouting() instead of later in the pipeline in UseEndpoints().
• Can we add a helper to set up a bunch of routing short circuits from config? Something that takes a list of route patterns that should immediate 404 and run an optional specific handler?
• Can we come up with a better name for the IEndpointConventionBuilder extension method other than ShortCircuit()? SkipMiddleware()? ExecuteImmidiately()?
• For defining many endpoints at once with a params array of route prefixes, what "Map" method name do we like? MapRejects, MapShortCircuit, MapImmediate404, MapQuickReject, MapEarlyNotFound? Let's go MapShortCircuit().
• For the many endpoints API, do we need a way to define a handler? No.
• Should MapShortCircuit return an IEndpointConventionBuilder? Yes. For additional constraints like on a host.

API Approved!

Example Usage:

app.MapGet("/favicon.ico", httpContext =>
{
    // httpContext.Response.StatusCode will default to 404 here.
    httpContext.RequestServices.GetService<ILogger<MyThing>>().LogTrace("Skipped favicon.ico");
}).ShortCircuit();

app.MapGet("/short-circuit-prefix/**", httpContext => { .. }).ShortCircuit();

// So /favicon.ico/test/extra/path would also be rejected
// MapRejects 404s and implies ShortCircuit()
app.MapShortCircuit("/favicon.ico", "/short-circuit-prefix", "/another-prefix");

Project/Assembly: Microsoft.AspNetCore.Routing

namespace Microsoft.AspNetCore.Builder;

+ public static class RouteShortCircuitEndpointConventionBuilderExtensions
+ {
+     public static IEndpointConventionBuilder ShortCircuit(this IEndpointConventionBuilder builder) { }
+ }

namespace Microsoft.AspNetCore.Routing;

+ public static class RouteShortCircuitEndpointRouteBuilderExtensions
+ {
+    public static IEndpointConventionBuilder MapShortCircuit(this IEndpointConventionBuilder builder, params string[] routePrefixes);
+ }

[Tratcher]

I still need to work out what metrics this will produce, and how that would be configurable.

[mitchdenny]

Shouldn't the MapShortcircuit(...) signature be MapShortCircuit(IEndpointRouteBuilder this builder, params [string] routePrefixes). Also should it return an IEndpointConventionBuilder. If we want this thing to shortcircuit any further processing then the API shouldn't act like additional options are going to do anything. Just return void (perhaps controversial).

[Tratcher]

Shouldn't the MapShortcircuit(...) signature be MapShortCircuit(IEndpointRouteBuilder this builder, params [string] routePrefixes).

You're right, the IEndpointRouteBuilder this builder, is missing. That's just at typo. Fixed

Also should it return an IEndpointConventionBuilder. If we want this thing to shortcircuit any further processing then the API shouldn't act like additional options are going to do anything. Just return void (perhaps controversial).

This was intentional so that you could add additional route constraints like host filtering.

[Kahbazi]

@Tratcher I'm interested in doing this one. Would you accept a PR?

[Tratcher]

Sure, but hold off a little longer, I still need to work out the requirements for logging/metrics.

[Tratcher]

I've made a separate proposal for metrics (#46361) that's generalized to all of routing. I'll see if that's acceptable to people.

[Tratcher]

As for this design, I'm still a little leary about ShortCircuit and MapShortCircuit being hardcoded to 404 responses. Executing a route immediately does not necessarily imply 404, especially with a RequestDelegate and ShortCircuit, that could generate any kind of response.

@halter73 I think we should amend both of these to take a status code:

app.MapGet("/favicon.ico", httpContext =>
{
    httpContext.RequestServices.GetService<ILogger<MyThing>>().LogTrace("Skipped favicon.ico");
}).ShortCircuit(404);

app.MapGet("/short-circuit-prefix/**", httpContext => { .. }).ShortCircuit(404);

// So /favicon.ico/test/extra/path would also be rejected
// MapShortCircuit implies ShortCircuit(status)
app.MapShortCircuit(404, "/favicon.ico", "/short-circuit-prefix", "/another-prefix");

Project/Assembly: Microsoft.AspNetCore.Routing

namespace Microsoft.AspNetCore.Builder;

+ public static class RouteShortCircuitEndpointConventionBuilderExtensions
+ {
+     public static IEndpointConventionBuilder ShortCircuit(this IEndpointConventionBuilder builder, int statusCode) { }
+ }

namespace Microsoft.AspNetCore.Routing;

+ public static class RouteShortCircuitEndpointRouteBuilderExtensions
+ {
+    public static IEndpointConventionBuilder MapShortCircuit(this IEndpointRouteBuilder builder, int statusCode, params string[] routePrefixes);
+ }

[Kahbazi]

@Tratcher Did you mean to write IEndpointRouteBuilder as argument for MapShortCircuit?

[Tratcher]

Uh, sure, that one 😆

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review notes:

• Should the IEndpointConventionBuilder ShortCircuit method take an optional statuscode in case the endpoint already takes care of setting it?
• Could the route prefixes come before the status code? Maybe, but we like the params list.

app.MapShortCircuit(404, "prefix1", "prefix2");
// vs.
app.MapShortCircuit(new[] { "prefix1", "prefix2" }, 404);
// vs.
// public static IEndpointConventionBuilder MapShortCircuit(this IEndpointRouteBuilder builder, params (int StatusCode, string Prefix)[] tuples)
app.MapShortCircuit(("prefix1", 404), ("prefix2", 400));
// vs. Require one at a time. public static IEndpointConventionBuilder MapShortCircuit(this IEndpointRouteBuilder builder, string prefix, int statusCode = 404)
app.MapShortCircuit("prefix1");
app.MapShortCircuit("prefix2", 400);

• We like the original proposal with the status code first then the params list.

Example Usage:

app.MapGet("/favicon.ico", httpContext =>
{
    // httpContext.Response.StatusCode will not be set by default before running this unless a parameter is passed
    httpContext.Response.StatusCode = 404;
    httpContext.RequestServices.GetService<ILogger<MyThing>>().LogTrace("Skipped favicon.ico");
}).ShortCircuit();

app.MapGet("/favicon2.ico", httpContext =>
{
    // httpContext.Response.StatusCode will be set to 404 here.
    httpContext.RequestServices.GetService<ILogger<MyThing>>().LogTrace("Skipped favicon2.ico");
}).ShortCircuit(404);

app.MapGet("/favicon3.ico", httpContext =>
{
    // This blows up because of RequireAuthorization()
}).RequireAuthorization().ShortCircuit(404);

app.MapGet("/short-circuit-prefix/**", httpContext => { .. }).ShortCircuit();

// So /favicon.ico/test/extra/path would also be rejected
// MapShortCircuit sets the specified status code and implies ShortCircuit()
app.MapShortCircuit(404, "/favicon.ico", "/short-circuit-prefix", "/another-prefix");

// May not work, but you can get route prefixes from config
app.MapShortCircuit(404, app.Config["MyShortCircuits"].GetChildren()...);

• Disallow short circuiting for endpoints that have [Authorize] and [RequireCors] metadata. Don't worry about other things like output caching and rate limiting and other middleware.
• Are we okay with not having public metadata for now? Yes, because we could add it later if needed.
• Should MapShortCircuit take a RequestDelegate instead of a status code for greater flexibility? No. You can use IEndpointConventionBuilder extension method for that.

API Approved!

Project/Assembly: Microsoft.AspNetCore.Routing

namespace Microsoft.AspNetCore.Builder;

+ public static class RouteShortCircuitEndpointConventionBuilderExtensions
+ {
+     public static IEndpointConventionBuilder ShortCircuit(this IEndpointConventionBuilder builder, int? statusCode = null) { }
+ }

namespace Microsoft.AspNetCore.Routing;

+ public static class RouteShortCircuitEndpointRouteBuilderExtensions
+ {
+    public static IEndpointConventionBuilder MapShortCircuit(this IEndpointRouteBuilder builder, int statusCode, params string[] routePrefixes);
+ }

[Kahbazi]

@Tratcher @halter73 Do you think that there would be a scenario that we could only wants to have short circuit with certain type of methods?

It's possible to achieve it with ShortCircuit method

builder.MapGet(...).ShortCircuit();

but not with MapShortCircuit directly.

[Tratcher]

Do you think that there would be a scenario that we could only wants to have short circuit with certain type of methods?

It's possible to achieve it with ShortCircuit method

builder.MapGet(...).ShortCircuit();

but not with MapShortCircuit directly.

Maybe, but MapGet(...).ShortCircuit() seems adequate. Isn't there also a way to add a method constraint using the IEndpointConventionBuilder returned from MapShortCircuit?

[Tratcher]

@javiercn proposed an additional API where the implementation is moved out of the route middleware into its own.

app.UseShortCircuitRoutes();

Pros:

• Separation of concerns
• Pay for play
• More flexible about placement

Cons:

• Easy to forget
• Harder to place right after routing when using the default WebApplication builder that bundles routing, you also have to add routing to ensure ordering.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• It would be easy not to notice if you forgot to call app.UseShortCircuitRoutes().
• If you call app.UseShortCircuitRoutes() too early, we cannot error out early unless UseRouting adds something to non-routed HttpContext to make it clear it has run. And the cost of adding something to the HttpContext counteracts the perf benefits of this proposal. We could consider analyzers to make sure you call this.
• Or we put it in UseRouting. And non-short circuited requests take 658.7 nanoseconds compared to 639.7 nanoseconds to match which seems insignificant.
• Could we just add a global option to enable/disable short circuiting? Maybe later.

API update rejected. Original API approved with no updated!