Aspnetcore: Razor Pages: Urls generated by built-in taghelpers do not honor PageRouteModelConvention

Created on 10 Nov 2019  路  11Comments  路  Source: dotnet/aspnetcore

Describe the bug

In a project that utilizes Razor Pages and request localization in conjunction with an IPageRouteModelConvention, Urls generated through standard TagHelpers for anchors or forms do not honor routing conventions and route values.

To Reproduce

Current route values (page url is /de)

  • Page: /Index
  • culture: de

Markup:
<a asp-page="Search">Search</a>

Expected Result:
<a href="/de/search">Search</a>

Actual Result:
<a href="/search">Search</a>

There's a workaround but it is quite ugly:

<a asp-page="Search" asp-route-culture="@Request.RouteValues["culture"]">Search</a>

Startup.cs:

```c#
public class Startup
{
public Startup(IConfiguration configuration, IWebHostEnvironment env)
{
Configuration = configuration;
Env = env;

    var builder = new ConfigurationBuilder()
        .SetBasePath(Environment.CurrentDirectory)
        .AddJsonFile("appsettings.json", optional: true)
        .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true);

    if (env.IsDevelopment())
        builder.AddUserSecrets<Startup>();

    Configuration = builder.Build();
}

public IConfiguration Configuration { get; }
public IWebHostEnvironment Env { get; }
public static RequestCulture DefaultRequestCulture = new RequestCulture("en-US", "en-US");

public void ConfigureServices(IServiceCollection services)
{
    services.AddOptions();
    services.AddSingleton(Configuration);

    services.AddRazorPages(options =>
    {
        options.Conventions.Add(new CultureTemplateRouteModelConvention());
    }).SetCompatibilityVersion(CompatibilityVersion.Version_3_0);

    services.AddLocalization(options => options.ResourcesPath = "Resources");

    services.Configure<RequestLocalizationOptions>(options =>
    {
        options.DefaultRequestCulture = DefaultRequestCulture;
        options.SupportedCultures = AppConstants.SupportedCultures;
        options.SupportedUICultures = AppConstants.SupportedCultures;

        options.RequestCultureProviders.Insert(0, new RouteDataRequestCultureProvider { Options = options });
    });

    services.AddRouting(options =>
    {
        options.LowercaseUrls = true;
    });

    services.AddOptions();
    services.AddLogging();
    services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();
    services.AddSingleton(Configuration);
}

public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    Container = app.ApplicationServices.GetAutofacRoot();

    app.UseStaticFiles();
    app.UseRouting();
    app.UseRequestLocalization();
    app.UseAuthorization();

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

}


**CultureTemplateRouteModelConvention.cs:**

```c#
public class CultureTemplateRouteModelConvention : IPageRouteModelConvention
{
    public void Apply(PageRouteModel model)
    {
        var selectorCount = model.Selectors.Count;

        for (var i = 0; i < selectorCount; i++)
        {
            var selector = model.Selectors[i];

            model.Selectors.Add(new SelectorModel
            {
                AttributeRouteModel = new AttributeRouteModel
                {
                    Order = -1,
                    Template = AttributeRouteModel.CombineTemplates(
                          "{culture?}", selector.AttributeRouteModel.Template),
                }
            });
        }
    }
}

Further technical details

  • ASP.NET Core 3.0
  • VS 2019
affected-medium area-mvc bug feature-routing investigate severity-major

Most helpful comment

@gerardfoley: Yes it is definitely the ambient values behavior change after the switch to endpoint routing.

@oliverw: I've worked around needing to inject the culture in each caller that is associated with a HttpContext by adding a decorator class around the default LinkGenerator that's added to the container.

public class LinkGeneratorDecorator : LinkGenerator
{
  private LinkGenerator _innerLinkGenerator;
  public LinkGeneratorDecorator(LinkGenerator inner) =>
    _innerLinkGenerator = inner ?? throw new ArgumentNullException(nameof(inner));

  public override string GetPathByAddress<TAddress>(
    HttpContext httpContext,
    TAddress address,
    RouteValueDictionary values,
    RouteValueDictionary ambientValues = null,
    PathString? pathBase = null,
    FragmentString fragment = null,
    LinkOptions options = null)
  {
    // clone the explicit route values
    var newValues = new RouteValueDictionary(values);

    // get culture from ambient route values
    if (ambientValues?.TryGetValue("culture", out var value) == true)
    {
      // add to the cloned route values to make it explicit
      // respects existing explicit value if specified
      newValues.TryAdd("culture", value);
    }

    return _innerLinkGenerator.GetPathByAddress(httpContext, address, newValues,
      ambientValues, pathBase, fragment, options);
  }

  // do the same with GetUriByAddress<TAddress> overload with ambient values,
  // straight pass-through for overloads without ambient values to use
}

If you're using UseRequestLocalization and detecting locales using other methods besides route values, there will be instances where the ambient route values don't have the locale. You could instead switch this code to use the IRequestCultureFeature of the HttpContext to get the resolved locale.

var requestCulture = httpContext.Features.Get<IRequestCultureFeature>().RequestCulture;
newValues.TryAdd("culture", requestCulture.Culture.Name); // or UiCulture.Name if preferred

If you're using a IPageRouteModelConvention to take every page route and adding a new route with a culture prefix, you could make the culture required on that route and disallow the original non-prefix route from being used in any link generation to be defensive. This is how I originally stumbled onto the problem; after I disabled non-prefix routes for link generation, I was getting blank results for any link where the ambient values were not used (e.g. going from Page = "/Index" to Page = "/Search").

public class CultureTemplateRouteModelConvention : IPageRouteModelConvention
{
    public void Apply(PageRouteModel model)
    {
        var selectorCount = model.Selectors.Count;

        for (var i = 0; i < selectorCount; i++)
        {
            var selector = model.Selectors[i];

            model.Selectors.Add(new SelectorModel
            {
                AttributeRouteModel = new AttributeRouteModel
                {
                    Order = -1,
                    Template = AttributeRouteModel.CombineTemplates(
                          "{culture}", selector.AttributeRouteModel.Template), // changed to required
                }
            });

            // suppress original route so URLs can't be made using its non-prefixed template
            selector.AttributeRouteModel.SuppressLinkGenerator = true;
        }
    }
}

@rynowak and @javiercn: It seems like the bigger theme here is that we need a seam to be able to describe/override policy behavior on the treatment of specific ambient values to allow them to continue to be route value candidates in link generation.

The DefaultLinkGenerator is too complex to re-implement, so a custom LinkGenerator implementation registered in DI doesn't seem like the right approach. The decorator I added to wrap the initially registered DefaultLinkGenerator with my own seems hacky.

Custom IRouteConstraint or IOutboundRouteValuesTransformer is not a candidate because constraints are only checked after filtering out candidate routes that have all the required route values set (the ambient value for culture is dropped so the routes requiring that route value are not candidates, and routes with the value set as optional don't run the constraint check because the value was not specified at all).

In conclusion, it seems that a new extensibility point is needed. Here's some options I've thought about.

Option: New Constraint

Constraint to mark a route value in a template that is always allowed to be taken from ambient values: "{culture:alwaysallowambient}/Search".

Pros:

  • Easy configuration
  • Existing convention
  • Works for callers without HttpContext

Cons:

  • No ServiceProvider to resolve services, so it can't take any dependencies (similar to #4579)
  • Requires changes to the candidate route value computation during route selection

Option: Better LinkGenerator abstraction

A simpler LinkGenerator interface to be able to implement from scratch.

Pros:

  • Allows more extensibility for all link generation behavior
  • Works for callers without HttpContext

Cons:

  • Doesn't provide a declarative convention
  • Requires endpoint routing to be enabled (although this is now the default behavior and the option on UseMvc is probably going to be taken out)

Alternative: DynamicRouteValueTransformer?

There are no docs about DymamicRouteValueTransformer. I've tried setting up a generic route for Razor Pages like endpoints.MapRazorPages(); endpoints.MapDynamicPageRoute("{page}"); with a transformer that would add the detected locale as a route value back into the RouteValuesDictionary, but I kept getting an AmbiguousMatchException.

All 11 comments

@oliverw Thanks for contacting us.

@rynowak any idea of what's going on here?

Further investigation revealed that the problem also applies to link to controller actions, not just pages. The reason is that AnchorTagHelper (as well as others) only pass Route Values explicitly set using the asp-route-x attribute to its HtmlGenerator instance which in turn invokes urlHelper.Page or urlHelper.Action etc. I wouldn't be surprised if this is by design to avoid unwanted side effects. But as a matter of fact, manually forwarding route values using asp-route-culture on each and every anchor or form was not necessary in earlier versions of Net Core.

I've come up with the following workaround for the time being. One that I'm not entirely happy with:

_ViewImports.cshtml:

@*Remove built-in TagHelpers that interfere with our overrides*@
@removeTagHelper Microsoft.AspNetCore.Mvc.TagHelpers.AnchorTagHelper, Microsoft.AspNetCore.Mvc.TagHelpers
@removeTagHelper Microsoft.AspNetCore.Mvc.TagHelpers.FormActionTagHelper, Microsoft.AspNetCore.Mvc.TagHelpers

@addTagHelper *, WebApp

CultureAwareAnchorTagHelper:

```c#
[HtmlTargetElement("a", Attributes = ActionAttributeName)]
[HtmlTargetElement("a", Attributes = ControllerAttributeName)]
[HtmlTargetElement("a", Attributes = AreaAttributeName)]
[HtmlTargetElement("a", Attributes = PageAttributeName)]
[HtmlTargetElement("a", Attributes = PageHandlerAttributeName)]
[HtmlTargetElement("a", Attributes = FragmentAttributeName)]
[HtmlTargetElement("a", Attributes = HostAttributeName)]
[HtmlTargetElement("a", Attributes = ProtocolAttributeName)]
[HtmlTargetElement("a", Attributes = RouteAttributeName)]
[HtmlTargetElement("a", Attributes = RouteValuesDictionaryName)]
[HtmlTargetElement("a", Attributes = RouteValuesPrefix + "*")]
public class CultureAwareAnchorTagHelper : AnchorTagHelper
{
public CultureAwareAnchorTagHelper(IHttpContextAccessor contextAccessor, IHtmlGenerator generator) :
base(generator)
{
this.contextAccessor = contextAccessor;
}

#region Verbatim Copy: https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.TagHelpers/src/AnchorTagHelper.cs

private const string ActionAttributeName = "asp-action";
private const string ControllerAttributeName = "asp-controller";
private const string AreaAttributeName = "asp-area";
private const string PageAttributeName = "asp-page";
private const string PageHandlerAttributeName = "asp-page-handler";
private const string FragmentAttributeName = "asp-fragment";
private const string HostAttributeName = "asp-host";
private const string ProtocolAttributeName = "asp-protocol";
private const string RouteAttributeName = "asp-route";
private const string RouteValuesDictionaryName = "asp-all-route-data";
private const string RouteValuesPrefix = "asp-route-";
private const string Href = "href";

#endregion

private readonly IHttpContextAccessor contextAccessor;
private readonly string defaultRequestCulture = Startup.DefaultRequestCulture.Culture.TwoLetterISOLanguageName;

public override void Process(TagHelperContext context, TagHelperOutput output)
{
    var culture = (string) contextAccessor.HttpContext.Request.RouteValues["culture"];

    if(culture != null && culture != defaultRequestCulture)
        RouteValues["culture"] = culture;

    base.Process(context, output);
}

}
```

Remarks:

The above solution kind of sucks because ...

  • Implementing a custom TagHelper that inherits from the built-in one is only necessary because AnchorTagHelper throws when encountering a tag which has its href attribute already set by another TagHelper with lower priority. Because of this, it is also necessary to completely remove it in _ViewImports.cshtml
  • TagHelperContext.AllAttributes is read-only. Otherwise it would be possible for a TagHelper to inject attributes (like asp-route-culture) into the context to be processed by a TagHelper lower down the chain which would be a nice extension point. Again I also understand the motivation for this currently not being the possible
  • The built-in TagHelpers are not sealed, yet all the constants for the HtmlTargetElement attributes are private and require copying in a derived implemenation.

Perhaps I've missed an extension point that executes before tag helpers that would allow for a cleaner implementing of this functionality?

I'm running into the same problem myself.

It looks like this behavior is by design as oliverw suspected.
From the page https://docs.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-3.0
Section: Endpoint routing differences from earlier versions of routing

In endpoint routing with ASP.NET Core 2.2 or later, the result is /Login. Ambient values aren't reused when the linked destination is a different action or page.

This might also explain why this was not necessary in earlier versions of Net Core.

I really hope for a work around for this. Having to manually pass the culture in every link and redirect throughout the entire site is a real pain.

Following

@gerardfoley: Yes it is definitely the ambient values behavior change after the switch to endpoint routing.

@oliverw: I've worked around needing to inject the culture in each caller that is associated with a HttpContext by adding a decorator class around the default LinkGenerator that's added to the container.

public class LinkGeneratorDecorator : LinkGenerator
{
  private LinkGenerator _innerLinkGenerator;
  public LinkGeneratorDecorator(LinkGenerator inner) =>
    _innerLinkGenerator = inner ?? throw new ArgumentNullException(nameof(inner));

  public override string GetPathByAddress<TAddress>(
    HttpContext httpContext,
    TAddress address,
    RouteValueDictionary values,
    RouteValueDictionary ambientValues = null,
    PathString? pathBase = null,
    FragmentString fragment = null,
    LinkOptions options = null)
  {
    // clone the explicit route values
    var newValues = new RouteValueDictionary(values);

    // get culture from ambient route values
    if (ambientValues?.TryGetValue("culture", out var value) == true)
    {
      // add to the cloned route values to make it explicit
      // respects existing explicit value if specified
      newValues.TryAdd("culture", value);
    }

    return _innerLinkGenerator.GetPathByAddress(httpContext, address, newValues,
      ambientValues, pathBase, fragment, options);
  }

  // do the same with GetUriByAddress<TAddress> overload with ambient values,
  // straight pass-through for overloads without ambient values to use
}

If you're using UseRequestLocalization and detecting locales using other methods besides route values, there will be instances where the ambient route values don't have the locale. You could instead switch this code to use the IRequestCultureFeature of the HttpContext to get the resolved locale.

var requestCulture = httpContext.Features.Get<IRequestCultureFeature>().RequestCulture;
newValues.TryAdd("culture", requestCulture.Culture.Name); // or UiCulture.Name if preferred

If you're using a IPageRouteModelConvention to take every page route and adding a new route with a culture prefix, you could make the culture required on that route and disallow the original non-prefix route from being used in any link generation to be defensive. This is how I originally stumbled onto the problem; after I disabled non-prefix routes for link generation, I was getting blank results for any link where the ambient values were not used (e.g. going from Page = "/Index" to Page = "/Search").

public class CultureTemplateRouteModelConvention : IPageRouteModelConvention
{
    public void Apply(PageRouteModel model)
    {
        var selectorCount = model.Selectors.Count;

        for (var i = 0; i < selectorCount; i++)
        {
            var selector = model.Selectors[i];

            model.Selectors.Add(new SelectorModel
            {
                AttributeRouteModel = new AttributeRouteModel
                {
                    Order = -1,
                    Template = AttributeRouteModel.CombineTemplates(
                          "{culture}", selector.AttributeRouteModel.Template), // changed to required
                }
            });

            // suppress original route so URLs can't be made using its non-prefixed template
            selector.AttributeRouteModel.SuppressLinkGenerator = true;
        }
    }
}

@rynowak and @javiercn: It seems like the bigger theme here is that we need a seam to be able to describe/override policy behavior on the treatment of specific ambient values to allow them to continue to be route value candidates in link generation.

The DefaultLinkGenerator is too complex to re-implement, so a custom LinkGenerator implementation registered in DI doesn't seem like the right approach. The decorator I added to wrap the initially registered DefaultLinkGenerator with my own seems hacky.

Custom IRouteConstraint or IOutboundRouteValuesTransformer is not a candidate because constraints are only checked after filtering out candidate routes that have all the required route values set (the ambient value for culture is dropped so the routes requiring that route value are not candidates, and routes with the value set as optional don't run the constraint check because the value was not specified at all).

In conclusion, it seems that a new extensibility point is needed. Here's some options I've thought about.

Option: New Constraint

Constraint to mark a route value in a template that is always allowed to be taken from ambient values: "{culture:alwaysallowambient}/Search".

Pros:

  • Easy configuration
  • Existing convention
  • Works for callers without HttpContext

Cons:

  • No ServiceProvider to resolve services, so it can't take any dependencies (similar to #4579)
  • Requires changes to the candidate route value computation during route selection

Option: Better LinkGenerator abstraction

A simpler LinkGenerator interface to be able to implement from scratch.

Pros:

  • Allows more extensibility for all link generation behavior
  • Works for callers without HttpContext

Cons:

  • Doesn't provide a declarative convention
  • Requires endpoint routing to be enabled (although this is now the default behavior and the option on UseMvc is probably going to be taken out)

Alternative: DynamicRouteValueTransformer?

There are no docs about DymamicRouteValueTransformer. I've tried setting up a generic route for Razor Pages like endpoints.MapRazorPages(); endpoints.MapDynamicPageRoute("{page}"); with a transformer that would add the detected locale as a route value back into the RouteValuesDictionary, but I kept getting an AmbiguousMatchException.

I've just run into the same issue. Thanks for all the work on this. The workaround mostly works for me, but one thing I've noticed is that for routes with the culture present, it generates links that include "Index" in them when it shouldn't. For example, I have a site with default culture "en" and alternate supported culture "ja." Links for "en" all work fine, for example I have a link generated with asp-page="/Puzzles/Index" and that generates a link like this: "/puzzles/" which is what I want (no "index" part as expected). However, when I am on Japanese pages, all the links look good, except those that are default "Index" pages... and the puzzle link above ends up as "/ja/puzzles/index" when what I want and expect is "/ja/puzzles/." Can you confirm this is not as expected? Any workaround for this issue?

@fuzl-llc - does this work for removing your index suffix?

services.AddRazorPages()
    .AddRazorPagesOptions(options =>
    {
        // Remove routes ending with /Index
        options.Conventions.AddFolderRouteModelConvention("/", model =>
        {
            var selectorCount = model.Selectors.Count;
            for (var i = selectorCount - 1; i >= 0; i--)
            {
                var selectorTemplate = model.Selectors[i].AttributeRouteModel.Template;
                if (selectorTemplate.EndsWith("Index"))
                {
                    model.Selectors.RemoveAt(i);
                }
            }
        });
    });

@jeffreyrivor I have the same issue where if you suppress link generation for the non-locale route/selector then it will generate a blank string for non localized urls. There doesn't seem to be a great way to solve this other than overriding the anchor tag helper and the url generator or by passing in the culture every time, both are extremely ugly.

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.

I am also hit by this. I wanted to add multi tenant support to an existing app by modifying all links from /{area}/{page} to /{tenant}/{area}/{page}/. I thought it is going to be trivial as adding custom IPageRouteModelConvention but unfortunately all links are broken between pages.

I can't believe this didn't make the cut for .NET 5. I accept this was a consequence of ambient route values no longer being passed with endpoint routing as that causes other problems. However we still should have a way of configuring which route values we would like to add automatically without having to manually add them.

What annoys me most is this is a regression which I'm sure affects far more people than the number of people using Blazor and in terms of web development that seems to be the only focus in the latest release.

Was this page helpful?
0 / 5 - 0 ratings