Routing Regression With Two Consecutive Optional Url Parameters

0 comments suggest edit

It pains me to say it, but ASP.NET MVC 3 introduces a minor regression in routing from ASP.NET MVC 2. The good news is that there’s an easy workaround.

The bug manifests when you have a route with two consecutive optional URL parameters and you attempt to use the route to generate an URL. The incoming request matching behavior is unchanged and continues to work fine.

For example, suppose you have the following route defined:

routes.MapRoute("by-day", 
        "archive/{month}/{day}",
        new { controller = "Home", action = "Index", 
            month = UrlParameter.Optional, day = UrlParameter.Optional }
);

Notice that the month and day parameters are both optional.

routes.MapRoute("by-day", 
        "archive/{month}/{day}",
        new { controller = "Home", action = "Index", 
            month = UrlParameter.Optional, day = UrlParameter.Optional }
);

Now suppose you have the following view code to generate URLs using this route.

@Url.RouteUrl("by-day", new { month = 1, day = 23 })
@Url.RouteUrl("by-day", new { month = 1 })
@Url.RouteUrl("by-day", null)

In ASP.NET MVC 2 the above code (well actually, the equivalent to the above code since Razor didn’t exist in ASP.NET MVC 2) would result in the following URLs as you would expect:

  • /archive/1/23
  • /archive/1
  • /archive

But in ASP.NET MVC 3, you get:

  • /archive/1/23
  • /archive/1

In the last case, the value returned is nullbecause of this bug. The bug occurs when two or more consecutive optional URL parameters don’t have values specified for URL generation.

Let’s look at the workaround first, then we’ll dive deeper into why this bug occurs.

The Workaround

The workaround is simple. To fix this issue, change the existing route to not have any optional parameters by removing the default values for month and day. This route now handles the first URL where month and day was specified.

We then add a new route for the other two cases, but this route only has one optional month parameter.

Here are the two routes after we’re done with these changes.

routes.MapRoute("by-day", 
        "archive/{month}/{day}",
        new { controller = "Home", action = "Index"}
);

routes.MapRoute("by-month", 
        "archive/{month}",
        new { controller = "Home", action = "Index", 
            month = UrlParameter.Optional}
);

And now, we need to change the last two calls to generate URLs to use the by-month route.

@Url.RouteUrl("by-day", new { month = 1, day = 23 })
@Url.RouteUrl("by-month", new { month = 1 })
@Url.RouteUrl("by-month", null)

Just to be clear, this bug affects all the URL generation methods in ASP.NET MVC. So if you were generating action links like so:

@Html.ActionLink("sample", "Index", "Home", new { month = 1, day = 23 }, null)
@Html.ActionLink("sample", "Index", "Home", new { month = 1}, null)
@Html.ActionLink("sample", "Index", "Home")

The last one would be broken without the workaround just provided.

The workaround is not too bad if you happen to follow the practice of centralizing your URL generation. For example, the developers building http://forums.asp.net/ ran into this problem as well during the upgrade to ASP.NET MVC 3. But rather than having calls to ActionLink all over their views, they have calls to methods that are specific to their application domain such as ForumDetailUrl. This allowed them to workaround this issue by updating a single method.

The Root Cause

For the insanely curious, let’s look at the root cause of this bug. Going back to the original route defined at the top of this post, we never tried generating an URL where only the second optional parameter was specified.

@Url.RouteUrl("by-day", new { day = 23 })

This call really should fail because we didn’t specify a value for the first optional parameter, month. If it’s not clear why it should fail, suppose we allowed this to succeed, what URL would it generate? /archive/23?  Well that’s obviously not correct because when a request is made for that URL, 23 will be interpreted to be the month, not the date.

In ASP.NET MVC 2, if you made this call, you ended up with /archive/System.Web.Mvc.UrlParameter/23. UrlParameter.Optional is a class introduced by ASP.NET MVC 2 which ships on its own schedule outside of the core ASP.NET Framework. What that means is we added this new class which provided this new behavior in ASP.NET MVC, but core routing didn’t know about it.

The way we fixed this in ASP.NET MVC 3 was to make the ToString method of UrlParameter.Optional return an empty string. That solved this bug, but uncovered a bug in core routing where a route with optional parameters having default values behaves incorrectly when two of them don’t have values specified during URL generation. Sound familiar?

In hindsight, I think it was a mistake to take this fix because it caused a regression for many applications that had worked around the bug. The bug was found very late in our ship cycle and this is just one of the many challenging decisions we make when building software that sometimes don’t work out the way you hoped or expected. All we can do is learn from it and let the experience factor into the next time we are faced with such a dilemma.

The good news is we have bugs logged against this behavior in core ASP.NET Routing so hopefully this will all get resolved in the next core .NET framework release.

Found a typo or error? Suggest an edit! If accepted, your contribution is listed automatically here.

Comments

avatar

26 responses

  1. Avatar for Linkgoron
    Linkgoron February 20th, 2011

    Nice to have an official "workaround", but I'd expect (hope?) you guys would leverage NuGet and use it for these kinds of fixes, instead of waiting for the next .Net core release.

  2. Avatar for Søren Gregersen
    Søren Gregersen February 20th, 2011

    Hi Phil, You've been hacked!
    Well, technicaly you've just inserted an empty H2-element before the "The Workaround"-heading, so IE9 makes the rest of the page look really akward.
    Other browser render the "closed" H2 (<h2 />) as an empty header, that is 0 (zero) pixel tall.

  3. Avatar for Paul Linton
    Paul Linton February 20th, 2011

    Hi Phil,
    Can you remove the self-closing h2 tag just before 'The Workaround' it makes the rest of the page render as h2 in my browser (IE8/9)
    thanks

  4. Avatar for Johannes Hansen
    Johannes Hansen February 20th, 2011

    Hi Phil, thanks for writing this post. I've asked about this "change" between MVC2 and MVC3 on Connect and StackOverflow (http://stackoverflow.com/questions/4846022/), and the responses have been few and far apart. So its nice to hear whats actually going on.
    I was wondering what the fix will be? Will it be like MVC2 but throw an exception for the "@Url.RouteUrl("by-day", new { day = 23 })" case, or will there be another kind of resolution to the problem?

  5. Avatar for Gorazd Švajger
    Gorazd Švajger February 20th, 2011

    Ha! I got to experience this first-hand during the making of my blog, with nearly the exact same route as the example. I couldn't find a reason for it and didn't want to waste time tracking it down, so in frustration I made separate routes, which finally worked. I'm glad I took the correct turn with the workaround (and that it was a bug in the first place, not just my inability to make valid routes!). :)

  6. Avatar for prokofyev
    prokofyev February 20th, 2011

    Last Friday I was also investigating the reason of this regression. Reflector shows what .NET 3.5 and .NET 4 versions of Bind method of System.Web.Routing.ParsedRoute class has a subtle difference. Because of these extra lines of code
    if ((builder2.Length > 0) && (builder2[builder2.Length - 1] == '/'))
    {
    return null;
    }
    two UrlParameter.Optional or even String.Empty parameters in row inside a route in .NET 4 give wrong virtual path.
    So I believe the reason not in ASP.NET MVC 3 itself.

  7. Avatar for haacked
    haacked February 20th, 2011

    @LinkGoron unfortunately, patches to the core .NET framework is not something we can release via NuGet. It's beyond scope and really not possible to have out-of-band bin deployable releases of System.Web.
    @prokofyev The change was to the ToString method of the UrlParameter class. But as I point out, that change just exposed a bug that's been in System.Web's Routing implementation. So you are right that the core bug is not within ASP.NET MVC.

  8. Avatar for Linkgoron
    Linkgoron February 20th, 2011

    Thanks for the answer, I thought it might be simpler with a small fix like this one.
    I hope I didn't come out offensive, I've re-read my earlier comment and thought it might seem offensive, if so, I apologize.
    Anyway, thanks!

  9. Avatar for prokofyev
    prokofyev February 20th, 2011

    @haacked: Now we know were are two changes: first in MVC 3 and second in ASP.NET 4 Routing, but I believe the reason of the regression is only second change, because not using in a route definition of UrlParameter.Optional at all (using String.Empty instead) gives the same bug.

  10. Avatar for Imran BAloch
    Imran BAloch February 20th, 2011

    I discussed this bug with a workaround 2 months before at,
    weblogs.asp.net/...

  11. Avatar for Kim Tranjan
    Kim Tranjan February 22nd, 2011

    thank you Haack!

  12. Avatar for Fujiy
    Fujiy February 22nd, 2011

    Phil, at the case that ASP.NET MVC 2 shows "/archive/System.Web.Mvc.UrlParameter/23", what ASP.NET MVC 3 generates? "/archive//23"?

  13. Avatar for haacked
    haacked February 22nd, 2011

    @Fujiy it returns null as the route is not a match for those specified parameters.

  14. Avatar for Jigga
    Jigga February 25th, 2011

    This is pathetic to say the least. How could it pass any tests?
    And I never read that using Html.ActionLink on my View is bad. Now You say it is.

  15. Avatar for Simon
    Simon February 28th, 2011

    Thanks for the update on this. Was quite frustrated when I discovered this in December when trying to switch to MVC3 - and quite disgruntled that you weren't able to fix it.
    It messed up so many of my links that it made me feel like the whole MVC3 was broken - but fortunately after figuring out a similar fix to yours I've seen no other problems with the final release and loving Razor!
    But feel much better now knowing that it's the core System.Web team's fault :-) Hopefully it'll be taken care of!

  16. Avatar for Winter
    Winter March 4th, 2011

    Thnax for the workaround tip haacked! Although of course it would be great if the bug itself could be fixed (with an update for instance), cause (imho) when the workarounds start to sum up, at some point you will lose the overview of wht's what and what/how exactly you worked around a bug (if it hast to be adjusted at some point for example).

  17. Avatar for Chris
    Chris April 20th, 2011

    I have found another bug which may or may not be caused by this.
    When you access an MVC app on a virutal folder in IIS so that the route ends up as /Virtual/Home/Test rather than just /Home/Test with the /Home/Test action taking multiple nullable parameters only the first nullable variable binds properly and all others do not.
    See stackoverflow.com/... for more details.

  18. Avatar for Damian
    Damian May 7th, 2011

    Hi Phil,
    What's your opinion about this issue: http://forums.asp.net/t/1328683.aspx ? The solution proposed by tgmdbm didnt work and I had to "hardcode" the routing. Meaning that, instead of using "Url.Action(blabla)", I have to do "Url.RouteUrl(blabla)".
    Thanks
    Damian

  19. Avatar for valamas
    valamas July 4th, 2011

    After much hair pulling, I opted to go the form tag + @Url.Action.

  20. Avatar for Weihnachtskarten drucken
    Weihnachtskarten drucken December 2nd, 2011

    Thanks for the workaround - but the article is from February - has there been any Update that would fix the Bug entirely?!

  21. Avatar for mitja.gti
    mitja.gti December 5th, 2011

    OMG! Thank you for this tip Haack, you saved my day :)
    Btw is, or should i say will, this be fixed in MVC 4?
    Thank you again.

  22. Avatar for Johan
    Johan December 21st, 2011

    Just ran into this beauty. Appreciate the post on it, though I wonder who decided to ship with this.
    The MVC 2 method worked correctly, sure it could have failed more gracefully on invalid data, but it wasn't a bug per se.
    MVC 3 routing is actually defective, sure the real reason is the core routing, but you had the option of not exposing the bug.
    I understand that you might have been to late in the cycle to do any changes (though this is an ugly one), but you could release and MVC patch that goes back to the MVC 2 behavior.

  23. Avatar for nalini
    nalini April 18th, 2012

    It is a nice example and clear explanation thanks a lot

  24. Avatar for Nik
    Nik May 1st, 2012

    This appears to still be a problem in MVC4.
    PS.
    blogs.msdn.com/...

  25. Avatar for bambams
    bambams October 30th, 2013

    I am having a lot of trouble figuring out how to get routing working as desired... It would sure be nice if there was centralized documentation that detailed the routing in depth...

  26. Avatar for Matt Titchener
    Matt Titchener February 16th, 2017

    Believe it or not, this is *still* an issue with .NET Core. It's probably fair to say this issue either never got fixed, or regressed once again for Core. To be fair, there are similar (though different) workarounds using default values in the routing string templates.