dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.5k stars 10.04k forks source link

ASP.NET Core MVC : UrlHelper.Action fail to resolve when mixing nullable route value and query string parameters #36352

Open Dunge opened 3 years ago

Dunge commented 3 years ago

I have a ASP.NET Core MVC .NET5 application with a simple route. It is the default controller/action pattern with an optional integer parameter. It is configured as:

endpoints.MapControllerRoute("default", "{controller=Account}/{action=Login}/{enterpriseId:int?}");

And a test Controller with two actions, one without query string parameters, one with one:

public class TestController : Controller
{
    public ActionResult Test()
    {
        return View();
    }

    public ActionResult TestQuery(int test)
    {
        return View();
    }
}

I can access the site via those URL successfully:

https://localhost/Test/Test https://localhost/Test/Test/1 https://localhost/Test/TestQuery?test=2 https://localhost/Test/TestQuery/1?test=2

The versions without the /1 doesn't contains a RouteData entry for enterpriseId value, the others do, which is fine.

When using UrlHelper.Action(), the parameter "values" allow to pass both Route data entries and query strings. But somehow it fails to resolve when setting the RouteData entry to null only in the specific case that it also contains query parameters.

urlHelper.Action("Test", "Test", new RouteValueDictionary { { "enterpriseId", 1 } });                           // Returns "/Test/Test/1"
urlHelper.Action("Test", "Test", new RouteValueDictionary { });                                                 // Returns "/Test/Test"
urlHelper.Action("Test", "Test", new RouteValueDictionary { { "enterpriseId", null } });                        // Returns "/Test/Test"

urlHelper.Action("TestQuery", "Test", new RouteValueDictionary { { "enterpriseId", 1 }, { "test", 2 } });       // Returns "/Test/TestQuery/1?test=2"
urlHelper.Action("TestQuery", "Test", new RouteValueDictionary { { "test", 2 } });                              // Returns "/Test/TestQuery?test=2"
urlHelper.Action("TestQuery", "Test", new RouteValueDictionary { { "enterpriseId", null }, { "test", 2 } });    // Returns null

I would expect the last line to also returns "/Test/TestQuery?test=2" instead of null.

Why does the sixth line behave differently than the third one (both with enterpriseId set to null)? But it works when it is omitted? Why can't it resolve to the URL I expect?

Is this a bug in the framework or something I misunderstand?

Worth mentioning that in the previous ASP.NET version (non-Core), using UrlHelper.GenerateUrl() was working fine with all cases.

sackri10 commented 3 years ago

@Dunge Can you confirm how you are instantiating an urlHelper Instance?

Dunge commented 3 years ago

Hello @sackri10 thanks for the reply.

In my original post I was using in inside an IHtmlHelper extension so I did it this way:

    var urlHelperFactory = helper.ViewContext.HttpContext.RequestServices.GetRequiredService<IUrlHelperFactory>();
    var urlHelper = urlHelperFactory.GetUrlHelper(helper.ViewContext);

But since then I tested by creating a new empty project doing just that to be sure there was nothing else was interfering, and used it directly using @Url inside a .cshtml view. It produced the exact same results.

sackri10 commented 3 years ago

@Dunge

If you have the GitHub URL for the test project, can you please add it here?

Dunge commented 3 years ago

Here you go:

https://github.com/Dunge/UrlHelperFailure

This is the Visual Studio 2019 MVC template with just the default route changed and the Test Controller/View as above.

Access via https://localhost:5001/Test/Test to see the result.

mkArtakMSFT commented 3 years ago

@javiercn can you please take a look at this? Thanks!

ghost commented 3 years ago

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.