Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Conventional routing with custom templates not working when you have area attributes #7959

Closed
davidliang2008 opened this issue Jun 21, 2018 · 9 comments
Assignees
Labels
3 - Done bug cost: XS Will take up to half a day to complete servicing-approved Shiproom has approved the issue
Milestone

Comments

@davidliang2008
Copy link

davidliang2008 commented Jun 21, 2018

Is this a Bug or Feature request?:

Bug

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

https://github.com/davidliang2008/DL.Issues.AspNetCore.Mvc.CustomRouting

Description of the problem:

When you want a more user/SEO friendly URLs on your site, you would set up custom routings to use slugs instead of ids.

public void Configure(IApplicationBuilder app, IHostingEnvironmentt env)
{
    ...

    app.UseMvc(routes =>
    {
        routes.MapRoute(
                name: "productListByTypeRoute",
                template: "products/{type}",
                defaults: new { area = "", controller = "products", action = "listByType" }
        );

        routes.MapRoute(
                name: "productListByCategoryRoute",
                template: "products/{type}/{category}",
                defaults: new { area = "", controller = "products", action = "listByCategory" }
        );

        routes.MapRoute(
                name: "productDetailsRoute",
                template: "products/{type}/{category}/{product}",
                defaults: new { area = "", controller = "products", action = "details" }
        );

        routes.MapRoute(
                name: "areaRoute",
                template: "{area:exists}/{controller=dashboard}/{action=index}/{id?}"
        );

        routes.MapRoute(
                name: "default",
                template: "{controller=home}/{action=index}/{id?}");
        );
    });

    ...
}

And then I have the controller and action setup to handle those requests.

public class ProductsController : Controller
{
     ...

    public async Task<IActionResult> ListByType(string type)
    {
        ...
        return View(...);
    }

    public async Task<IActionResult> ListByCategory(string type, string category)
    {
        ...
        return View(...);
    }

    public async Task<IActionResult> Details(string type, string category, string product)
    {
        ...
        return View(...);
    }

    ...
}

So the following links are expected to call their corresponding actions:

Link Action Parameters
/products/countertop ListByType type=countertop
/products/countertop/2cm-granite ListByCategory type=countertop, category=2cm-granite
/products/countertop/2cm-granite/imperial-splendor Details type=countertop, category=2cm-granite, product=imperial-splendor

Everything works fine until you add an area! All custom URLs would return 404!

[Area("admin")]
public abstract class AdminControllerBase : Controller {}
public class DashboardController : AdminControllerBase
{
    public IActionResult Index()
    {
        return View();
    }
}

Notes:

  1. With the area routing setup in Startup, as long as the [Area("admin")] is commented out, all custom links would work with the setting new { area = "", ... }.
  2. If you have [Area("admin")] on, the only way to get those custom links to work is to remove area = "" from custom routing setup.
  3. This setup worked flawlessly in previous Microsoft.AspNetCore.Mvc 2.0.7!

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

V2.1.0

@davidliang2008 davidliang2008 changed the title Conventional routing with custom templates not working Conventional routing with custom templates not working when you have area attributes Jun 21, 2018
@kichalla
Copy link
Member

@davidliang2008 From the repro app its not clear where you are trying to generate the links from. Are you doing it from the DashboardController to Products? if so, the ambient value of 'admin' is probably being used.

@davidliang2008
Copy link
Author

davidliang2008 commented Jun 29, 2018

No, all product links are public. They're not under Admin area. I have the ProductController defined just as a regular Controller and 3 more actions within it: ListByType, ListByCategory and Details. For testing purpose, All 3 actions return Index view with different string so that we know which action is getting called.

The issue is:

  • The links in the table in the OP work as long as [Area()] is not used.
  • If you use any area in the app, admin area being just an example, the links in the table won't work and all return 404.
  • The links in the table in the OP work with area being used in the app when you remove area="" in those 3 custom route mapping defaults in Startup.cs, which I don't understand.

Hope I explained it clearly.

@kichalla
Copy link
Member

kichalla commented Jul 2, 2018

Ah yes, I am able to reproduce the issue. Investigating the cause...

@kichalla
Copy link
Member

kichalla commented Jul 2, 2018

So seems like the root cause of the issue is change in behavior of the GetHashCode api on StringComparer.

In netcoreapp2.0, the following prints 0:

Console.WriteLine(StringComparer.OrdinalIgnoreCase.GetHashCode(""));   // here "" can be thought as area=""

Since this used to give 0, we have logic of a string being null also as 0

In netcoreapp2.1, the above code prints a non-zero value which causes a look up miss for action descriptors where they are built with key like area=null and the look value is area=""

cc @rynowak

@davidliang2008 as a workaround, you can change your route configuration to be like

defaults: new { area = (string)null, controller = "products", action = "listByType" }

@kichalla kichalla added 2 - Working cost: XS Will take up to half a day to complete labels Jul 3, 2018
@pranavkm
Copy link
Contributor

pranavkm commented Jul 3, 2018

FYI @stephentoub in case the behavior change was unintentional.

@davidliang2008
Copy link
Author

You guys are awesome!!

@stephentoub
Copy link

FYI @stephentoub in case the behavior change was unintentional.

It's intentional. Randomized string hashing was enabled for IgnoreCase in 2.1.

@pranavkm pranavkm added bug servicing-consider Shiproom approval is required for the issue and removed investigate labels Jul 3, 2018
@pranavkm pranavkm added servicing-approved Shiproom has approved the issue and removed servicing-consider Shiproom approval is required for the issue labels Jul 3, 2018
@mkArtakMSFT mkArtakMSFT added this to the 2.1.3 milestone Jul 6, 2018
kichalla added a commit that referenced this issue Jul 9, 2018
kichalla added a commit that referenced this issue Jul 9, 2018
@kichalla
Copy link
Member

kichalla commented Jul 9, 2018

bd995d4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done bug cost: XS Will take up to half a day to complete servicing-approved Shiproom has approved the issue
Projects
None yet
Development

No branches or pull requests

5 participants