Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undocumented route value invalidation when route values are not specified in route pattern #31602

Closed
ajbeaven opened this issue Apr 7, 2021 · 11 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing investigate
Milestone

Comments

@ajbeaven
Copy link

ajbeaven commented Apr 7, 2021

Describe the bug

I believe the changes made in .NET Core 2.2(?) with ambient route value invalidation do not rightly consider how routes are generated when a route pattern is specified that does not contain controller or action route values.

Consider the following routes:

app.UseEndpoints(endpoints =>
{
	endpoints.MapControllerRoute(
		name: "fancyRouteWithCulture",
		pattern: "{culture}/test-a-with-fancy-route",
		defaults: new { controller = "TestA", action = "Index" });

	endpoints.MapControllerRoute(
		name: "fancyRoute",
		pattern: "test-a-with-fancy-route",
		defaults: new { controller = "TestA", action = "Index" });

	endpoints.MapControllerRoute(
		name: "defaultWithCulture",
		pattern: "{culture}/{controller=Home}/{action=Index}/{id?}");

	endpoints.MapControllerRoute(
		name: "default",
		pattern: "{controller=Home}/{action=Index}/{id?}");
});

If the current page is /ru, the URL generated with Url.Action("Index", "TestA") will differ between versions:

  • .NET Core 2.1 will be /ru/test-a-with-fancy-route
  • .NET Core 3.1 will be /test-a-with-fancy-route

From what I gather from the URL generation process documentation, a route value is invalidated if in the pattern it appears to the right of a route value that is changed. The pattern above does not specify controller or action, so I would expect that the culture ambient route value would not be invalidated as it is in the latest versions of .NET.

To Reproduce

Here's a repro showing the behaviour change from .NET Core 2.1 to .NET Core 3.1.

Clone the repro above, run the two projects side-by-side and follow the instructions below on each:

  1. Click on "Russian culture" link to change the URL to /ru (thereby making culture an ambient route value)
  2. Review the URLs generated for each of the links at the bottom of the page

Workaround

I have found a hacky(?) work around for my particular case, but if there are better options, I'm all ears:

endpoints.MapControllerRoute(
	name: "fancyRouteWithCulture",
	pattern: "{culture}/test-a-with-fancy-route/{controller=TestA}/{action=Index}",
	constraints: new { controller = "TestA", action = "Index" });

By replacing the route at the top with something like that, I continue to restrict that route to the one specific controller action, but also force the route invalidation rules to work as they are described in the documentation above.

Further technical details

See here
  • ASP.NET Core version: .NET Core 3.1
  • Include the output of dotnet --info:
.NET SDK (reflecting any global.json):
 Version:   5.0.201
 Commit:    a09bd5c86c

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19042
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.201\

Host (useful for support):
  Version: 5.0.4
  Commit:  f27d337295

.NET SDKs installed:
  1.1.10 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.400 [C:\Program Files\dotnet\sdk]
  2.1.401 [C:\Program Files\dotnet\sdk]
  2.1.402 [C:\Program Files\dotnet\sdk]
  2.1.403 [C:\Program Files\dotnet\sdk]
  2.1.500 [C:\Program Files\dotnet\sdk]
  2.1.502 [C:\Program Files\dotnet\sdk]
  2.1.503 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.1.700 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.2.103 [C:\Program Files\dotnet\sdk]
  3.1.100 [C:\Program Files\dotnet\sdk]
  3.1.400 [C:\Program Files\dotnet\sdk]
  5.0.201 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version: Visual studio 16.9.2
@javiercn javiercn added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing labels Apr 8, 2021
@pranavkm pranavkm added this to the Next sprint planning milestone Apr 8, 2021
@ghost
Copy link

ghost commented Apr 8, 2021

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. 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.

@ajbeaven
Copy link
Author

ajbeaven commented Apr 8, 2021

Sure thing, thanks for that.

Note I am having difficulties coming up with a workaround when the current URL is in an area eg. if the current URL matches the following route:

endpoints.MapControllerRoute(
	name: "areasWithCulture",
	pattern: "{culture}/{area:exists}/{controller=Home}/{action=Index}/{id?}";

Any URLs generated for routes outside of the current area don't pass through the culture ambient route value and I can't figure out how to use a similar hack as above to avoid this.

Also pinging #16960 as initially I thought it was related to that bug. Looks like others have thought the same.

@tdamir
Copy link

tdamir commented May 1, 2021

I've had a similar problem and the most simple solution which worked for me was to keep the old routing (app.UseMvc) and use endpoint routing for other stuff like endpoints.MapReverseProxy().

            app.UseRouting();
            app.UseAuthentication();
            app.UseAuthorization();

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

            app.UseMvc(routes =>
            {
                routes.MapRoute(
                    name: "default",
                    template: "{controller=Home}/{action=Index}/{id?}");
            });

and the service registration:

            services.AddMvc(options =>
                {
                    options.EnableEndpointRouting = false;
                })

I'm not sure if there are any drawbacks regarding this solution but for now it looks like everything works without rewriting of every action URL generation call...

@Taita-Yam
Copy link

@tdamir Thanks for this suggestion; it seems to work for a netcore2.1 project that I've inherited and am updating to Net5.0. My symptom was empty href strings everywhere when using "asp-page" anchor tag, whereby the original site was utilising ambient values for various userId, documentId etc to build the URLs. Your solution seems like the cleanest one I've found; I do not wish to implement hacky helper classes or manually change hundreds of anchor tags - I've got real work to do.

@valeriob
Copy link

Hi, is this related ? #31476

@fschmied
Copy link

fschmied commented Aug 8, 2021

@ajbeaven We've also run into this when moving from ASP.NET Core 2.1 to 5.0. You're saying this:

From what I gather from the URL generation process documentation, a route value is invalidated if in the pattern it appears to the right of a route value that is changed. The pattern above does not specify controller or action, so I would expect that the culture ambient route value would not be invalidated as it is in the latest versions of .NET.

However, I think you're running into the concept of "required values". If I understand correctly, if the required values "controller" or "action" do not appear inside the pattern/template, they are assumed to be "to the left" of everything else. I.e., when either controller or action changes, all ambient values are invalidated (as they are viewed to be to the right).

I think the docs even highlight your specific issue in the section titled Problems with route value invalidation:

In the preceding code, the culture route parameter is used for localization. The desire is to have the culture parameter always accepted as an ambient value. However, the culture parameter is not accepted as an ambient value because of the way required values work [...]

Altogether, I find the concept of "required values" hard to understand, not very intuitive.

@fschmied
Copy link

fschmied commented Aug 8, 2021

@pranavkm Until now, there hasn't been any feedback from the team on this issue (only by means of labels). Is there any feedback from the ASP.NET Core team on the topic of these "global" ambient values, which could be used up to 2.1 for multinenancy, active culture, etc. See also #31476 (comment), where @valeriob is also not getting any feedback on a similar problem (despite having originally reported this in #20489 in April 2020!).

I'm moving my question over to #31476.

@fschmied
Copy link

fschmied commented Aug 8, 2021

Related/similar older issues:
#19927 (ambient value for tenant not working)
#11635 (by design: userId not transferred)
#12794 (by design: slug not transferred)

@valeriob
Copy link

valeriob commented Aug 8, 2021

Indeed @fschmied ! This kind of feature is the very reason to use a framework like aspnet.
Seeing the team not tackling this issue, not even answering with at least a work around to focus so much on blazor is very scary for the future to be honest :( The same appened for Msmq, there was silence for years on the issue, than one day it was dropped.
AspNet lost many ppl that used to build websites from aspnet to aspnetcore and it shows on features to be honest, the engineering is great, but i think the team lost some contact with real world applications.
For example the ever missing features folders for static assests or bundling and minifying tools.

@ajbeaven
Copy link
Author

ajbeaven commented Aug 8, 2021

However, I think you're running into the concept of "required values". If I understand correctly, if the required values "controller" or "action" do not appear inside the pattern/template, they are assumed to be "to the left" of everything else. I.e., when either controller or action changes, all ambient values are invalidated (as they are viewed to be to the right).

@fschmied looks like you're right. Looking forward to global ambient values being a thing.

Edit: oops, sorry for the close/reopen - misclick!

@ajbeaven ajbeaven closed this as completed Aug 8, 2021
@ajbeaven ajbeaven reopened this Aug 8, 2021
@fschmied
Copy link

fschmied commented Aug 9, 2021

@ajbeaven Maybe this issue can indeed be closed since the route value invalidation you (and we) are seeing is documented after all. I'll move my question over to #31476.

@ajbeaven ajbeaven closed this as completed Aug 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing investigate
Projects
None yet
Development

No branches or pull requests

7 participants