dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.25k stars 9.95k forks source link

Warning on use of regex constraints in route parameters. #46546

Open mitchdenny opened 1 year ago

mitchdenny commented 1 year ago

Background and Motivation

As part of #46227 we are removing the default regex constraint from the set of constraints available when CreateSlimBuilder(...) is used. The reason for this is that including regular expression support on constraints pulls in about 800K of extra file size. We still include the regex constraint resolver when CreateBuilder(...) is used.

However ... there is an opportunity for people to continue to use regular expressions for router parameters if they implement their own route constraints and provided they know what the regular expression is in advance they can take advantage of source generated regular expressions. We want an analyzer which detects the use of regex constraints and adds a warning which links to documentation on how to create a custom route constraint that uses source generated regex.

Proposed Analyzer

Analyzer Behavior and Message

When a route pattern is used that includes a regex constraint, we would output the following info message:

INFO: Recommend use of custom reoute constraint with source generated regular expressions. See https://aka.ms/aspet/tba for more details.

Note, if we detect that we are using CreateSlimBuilder (won't always be possible) then we can increase this to a warning?

Category

Severity Level

Usage Scenarios

app.MapGet("/products/{productId:regex(...)}", (string productId) => {});

Risks

To discuss.

ghost commented 1 year ago

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:

captainsafia commented 1 year ago

Triage: This analyzer would be most helpful in scenarios where the user has updated their code to use the SlimBuilder and is still using regular expressions in their route. In this case, it would fallback to a runtime warning that describes the issue. Users who are upgrading from .NET 7 to .NET 8 won't run into any issues since they don't use the SlimBuilder.

For a full-fidelity solution, we can consider this analyzer and a fixer that generates code the Regex constraint referenced in the route.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.