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.55k stars 10.05k forks source link

RewriteMiddleware sets endpoint in pipeline before routing middleware #44639

Open bgulrich opened 2 years ago

bgulrich commented 2 years ago

Is there an existing issue for this?

Describe the bug

I'm attempting to use the RewriteMiddleware to (in part) select alternate paths for static files. If I use the UseRewriter extension to register the middleware in the pipeline, endpoint evaluation/selection occurs sooner than expected on rewritten requests, which causes StaticFilesMiddleware to be bypassed when an alternate endpoint is found. I'm using YARP to route all unserviceable requests to another application, so the YARP endpoint will always be selected in my case.

Expected Behavior

I would expect the rewrite middleware to simply rewrite the request path and let the pipeline continue as configured.

Steps To Reproduce

Example code here: https://github.com/bgulrich/rewrite-bug

I took the template ASP.NET Core 6 WebApplication and moved wwwroot/favicon.ico -> wwwroot/test/favicon.ico and configured the rewrite middlware to point /favicon.ico request to this new location. I also added the TestController with an endpoint on the /test/favicon.ico route. My expectation is that this endpoint should not be reachable because it should be serviced by the static files middleware that appears earlier in the pipeline, but issuing a request to /favicon.ico hits this endpoint instead because the RewriteMiddleware is setting the endpoint (checked with breakpoint in dummy middleware between rewrite and static files), causing StaticFilesMiddleware to bypass.

Exceptions (if any)

No response

.NET Version

6.0.400

Anything else?

My current workaround is to register my RewriteOptions with the service provider and to register the RewriteMiddleware manually with app.Use<RewriteMiddleware>() to avoid the UseRewrite extension that tweaks the pipline.

Tratcher commented 2 years ago

@BrennanConroy

adityamandaleeka commented 2 years ago

Triage: We want to keep the behavior of UseRewriter but we should fix this so that the case that @bgulrich shared (where UseRouting() is explicitly called) just works.

@bgulrich Glad you found a workaround for it; that seems reasonable (though it's unfortunate that it's necessary).

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.

bgulrich commented 2 years ago

Triage: We want to keep the behavior of UseRewriter but we should fix this so that the case that @bgulrich shared (where UseRouting() is explicitly called) just works.

Maybe just a flag in the options to opt-out of this branching.