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.42k stars 10.01k forks source link

[Analyzer] Detect middleware in pipeline after terminal middleware #44244

Open JamesNK opened 2 years ago

JamesNK commented 2 years ago

Is there an existing issue for this?

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

Some middleware is terminal. That means it doesn't call the next middleware (if present) and starts the request returning back down the pipeline. For example, UseSpa is terminal.

Developers could place middleware after UseSpa, not realizing it isn't executed. For example, UseEndpoints after UseSpa.

This problem also happens with WebApplication, which implicitly adds UseEndpoints to the end of the pipeline. Any terminal middleware prevents endpoints from being executed.

Describe the solution you'd like

Additional context

No response

ghost commented 2 years ago

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.

juliushardt commented 1 year ago

Possibly related: When terminal middleware is used with WebApplication, it is required to use UseEndpoints(), as described above. However, the analyzer for ASP0014 unconditionally suggests top level route registrations even in this case, which may be confusing.

Scenario: I created a new ASP.NET Core web application using the "ASP.NET Core Web App" template in Visual Studio and added the Spa middleware from Microsoft.AspNetCore.SpaServices.Extensions:

// Program.cs from the template
// ...
app.MapRazorPages();

// I added the following lines:
if (app.Environment.IsDevelopment())
{
    app.UseSpa(spa =>
    {
        spa.UseProxyToSpaDevelopmentServer("http://127.0.0.1:5173");
    });
}

app.Run();

This preceding code does not work as expected, since UseSpa is terminal and, hence, the ASP.NET routes (e.g. Razor Pages) are never executed. To fix this, I changed the code such that UseEndpoints() is called explicitly:

//app.MapRazorPages();

app.UseEndpoints(endpoints =>
{
    endpoints.MapRazorPages();
});

if (app.Environment.IsDevelopment())
{
    app.UseSpa(spa =>
    {
        spa.UseProxyToSpaDevelopmentServer("http://127.0.0.1:5173");
    });
}

The modified code works, but results in code style warning ASP0014. As far as I can tell, there is no way to use UseSpa() with the new hosting model without calling UseEndpoints() explicitly. Therefore, when this issue is addressed, I suggest updating the analyzer for ASP0014 as well, such that is does not issue a warning if UseEndpoints() is called before the registration of terminal middleware.

jornhd commented 1 year ago

I have a similar problem with Swagger. If I use app.MapGet(), I get "No operations defined in spec!". When I wrap it in app.UseEndpoints() like before, Swagger works as expected, but then I get the AP0014 warning.

EDIT: After further investigation, I found that that the problem is that I have a custom filter reading api description: app.Services.GetService<IApiDescriptionGroupCollectionProvider>().ApiDescriptionGroups.Items.ToList(); If I remove this it works, but as soon as I access the ApiDescriptionGroupCollectionProvider, I get "No operations defined in spec!". Again, no problem when wrapping the registrations in .UseEndpoints().