Mvc: Internal routing prevents using route parameter names “page”, “action”, and “controller”

[poke]

We just talked about this on Slack and found out that MVC’s internal routing prevents one from using various route parameter names. These names are in particular page, action, controller, and handler. It probably also applies sometimes to area.

For example, this code basically prevents us from using a page route value when using Razor pages:

if (string.IsNullOrEmpty(pageName))
{
    if (!routeValues.ContainsKey("page") &&
        ambientValues.TryGetValue("page", out var value))
    {
        routeValues["page"] = value;
    }
}
else
{
    routeValues["page"] = CalculatePageName(urlHelper.ActionContext, pageName);
}

So basically, when there’s no explicit pageName set (e.g. using asp-page in the tag helper), then it will pick up the value from the page route value (e.g. using asp-route-page). That’s fine on its own but ultimately, the page route value will be overwritten with the resulting page name.

Similar logic also exists for the MVC side of it, e.g. for the action and controller route value.

To make it clear what this means, the following action in a controller will always have the controller and action value set to a constant value:

public IActionResult Test(string controller, string action)
{ … }

At some point, there was some effort to make sure that the addition of Razor pages wouldn’t break the usage of a page parameter in MVC. This was done in 9f5e4eb483e19be069defa6b091aebef166d50be.

I understand that this was done to make the whole system more flexible, so instead of transferring the data separately (which would have made adding Razor pages more difficult), it is stored within the route values. But what I am wondering is why those names were chosen for this internal routing data.

Especially the names action and page are super common in routes, and are also incidentally the easiest to break here. Couldn’t some more internal names have been used here instead? E.g. asp-action and asp-page, or something else? Is there any chance we could still change this now? (I assume setting actions using asp-route-action is a supported thing to do? 😕)

cc @DamianEdwards, @pranavkm, @bartmax

[mkArtakMSFT]

/cc @rynowak

[rynowak]

I sympathize with this, but unfortunately I don't think there's anything we can do.

This would break lots of applications that use these route values directly. I've seen lots of user-written code that reads these route values, the impact would be quite big.

[Bartmax]

I've seen lots of user-written code that reads these route values, the impact would be quite big.

Not for page

also if you knew that from mvc, picking page on razor pages (which is new) sounds like double dumb.

I'm sorry if my comment sounds like I'm upset (i am), but I don't think you really sympathize with any of this if you let this design slip thru on razor pages.

[mkArtakMSFT]

Closing this per @rynowak's response.
/cc @danroth27, @glennc

[poke]

While I understand that we cannot do anything about this anymore, I would be interested to learn a bit about the initial decision for this. Why was it decided to put these properties as common names into the route data? Was this just an accident and not thought through enough? Or was there an actual reasoning that made this useful?

[rynowak]

This decision was made in 2007 during the initial release of MVC. Route data is something that user code can see, inspect and modify. The 'keys' for route values like are frequently typed in user-provided routes and calls to apis like Url.Route and Url.Action. It's important that any 'key' also be a valid C# identifier to make it easy to use with the APIs that accept an anonymously typed object.

We made a decision not to mangle page because it would be esoteric and weird compared to the other route values that are straightforward.

[poke]

@rynowak Thank you for the explanation! That makes sense.

It’s a shame though that the choice of key names is a legacy carry-over from MVC. I wish this would have used less ambiguous names (e.g. asp-action or something) so that these common names wouldn’t basically end up being invisible.

Luckily, we can still use stuff like [FromQuery] string action to access actual query arguments, although it feels certainly a bit odd that we need to do this (this issue might serve as a good documentation of this behavior).

[mqudsi]

[FromQuery] still breaks routing, as <a asp-page="Foo" asp-route-page="foo">foo</a> generates /Foo as the link instead of /Foo?page=foo.

As discussed in #8010 I believe this should be a hard error. It's not hard to detect this, and the problem isn't that it's a reserved word, it's that _things break silently_ and you bang your head against the wall and go crazy before it occurs to you to try renaming the variable.

[poke]

Hmm, the tag helper / html helper not being able to generate the query arguments might be a bug – or at least something we could fix: If the tag helper contains both asp-page and asp-route-page, make the latter append a query argument (same with asp-action and asp-route-action and those other ones).

And I don’t know, is changing the internal keys of these items something we could do as a breaking change for 3.0?

[mqudsi]

I honestly don't care if the keywords are reserved or not. I am however extremely bothered that I wasted an hour+ of my life trying to figure it out, as currently they are not "reserved" names but rather "go ahead and use them but the output will make no sense" names.

To summarize my experience: it was one of my first times binding to a GET parameter in a razor page, and it didn't work the way I thought it would from my MVC experience. The documentation on razor model binding is _extremely_ misleading with its talk about how for security reasons no one in their right mind would bind a model to a GET parameter without an explicit request from the user to do so, but completely neglects to mention that this only applies if the model in question is a property in the razor page model class and does not hold if it's a function-local argument instead, in which case there's no need to decorate anything. But since it doesn't say that, I wasted time trying to figure out how to use [BindProperty(SupportsGet = true)] on a parameter, since a) I could see the parameter was not binding to the query string value, b) the documentation says it wouldn't automatically bind in case of GET unless I used this. I searched the documentation and on Google, but something about the keywords I was using kept returning the same misleading/tangential docs on razor page model binding.

At this point, I wasted another chunk of time moving the model from a function parameter to a class property so that I _could_ use [BindProperty(SupportsGet = true)] only to discover that lo and behold, it _still_ didn't bind.

At this point, I felt it was a "it's not me, it's you" kind of situation so I decided to play around with things and it occurred to me that Page might be reserved, so I renamed the property to Page2 and it magically worked. Except, of course, no one would ever name a public-facing variable Page2 unless they have no shame, so I was about to rename it to p and internally bind it to the Page property. But then I thought to myself "there's no way Page is _that_ reserved, there should be a workaround since this would mean certain legacy apps just can't be ported to ASP.NET Core without URL breakage" and figured I'd already wasted what felt like the entire day on this so I could afford to dig around in the source and see what I can find.

Looking at the .NET source (thank God you guys open sourced it, though in this case simply looking at the assembly exports would have sufficed) I came across [FromGet] which is not documented in this context (I suppose it really means "Ignore POST parameters with this name and only consider GET query string parameters" and not "for this parameter, enable binding from GET"). I undid my changes from property back to function parameter and decorated it with [FromGet] and was happy to see it finally worked, though I didn't understand why it was so hard to get it going and why this would work since [FromGet] really only disabled non-GET methods and not explicitly enabled GET methods.

Glad to finally be getting somewhere, I added the link to the search results page (having only earlier embraced the asp tag helpers over manually crafting the URLs) only to find to my dismay not even two minutes later that it didn't, in fact, generate the URL it promised to generate. Change things back to a property and use both [FromGet] and [PropertyBinding(SupportsGet = true)] to see if that could coax the asp tag helpers into behaving, but that didn't work - time to switch things back and give up on using the word Page altogether.