views:

7976

answers:

4

Using ASP.NET MVC Preview 5 (though this has also been tried with the Beta), it appears that querystring defaults in a route override the value that is passed in on the query string. A repro is to write a controller like this:

public class TestController : Controller
{
    public ActionResult Foo(int x)
    {
        Trace.WriteLine(x);
        Trace.WriteLine(this.HttpContext.Request.QueryString["x"]);
        return new EmptyResult();
    }
}

With route mapped as follows:

routes.MapRoute(
    "test",
    "Test/Foo",
    new { controller = "Test", action = "Foo", x = 1 });

And then invoke it with this relative URI:

/Test/Foo?x=5

The trace output I see is:

1
5

So in other words the default value that was set up for the route is always passed into the method, irrespective of whether it was actually supplied on the querystring. Note that if the default for the querystring is removed, i.e. the route is mapped as follows:

routes.MapRoute(
    "test",
    "Test/Foo",
    new { controller = "Test", action = "Foo" });

Then the controller behaves as expected and the value is passed in as the parameter value, giving the trace output:

5
5

This looks to me like a bug, but I would find it very surprising that a bug like this could still be in the beta release of the ASP.NET MVC framework, as querystrings with defaults aren't exactly an esoteric or edge-case feature, so it's almost certainly my fault. Any ideas what I'm doing wrong?

A: 

I thought the point with Routing in MVC is to get rid of querystrings. Like this:

routes.MapRoute(
    "test",
    "Test/Foo/{x}",
    new { controller = "Test", action = "Foo", x = 1 });
Spoike
Querystrings are appropriate when it is a query, e.g. you may have optional params like sort, page, and so on. Irrespective, this doesn't answer the question in any way.
Greg Beech
@Greg Beech: The question should at least provide an example that illustrates this.
Spoike
+8  A: 

One of my colleagues found a link which indicates that this is by design and it appears the author of that article raised an issue with the MVC team saying this was a change from earlier releases. The response from them was below (for "page" you can read "x" to have it relate to the question above):

This is by design. Routing does not concern itself with query string values; it concerns itself only with values from RouteData. You should instead remove the entry for "page" from the Defaults dictionary, and in either the action method itself or in a filter set the default value for "page" if it has not already been set.

We hope to in the future have an easier way to mark a parameter as explicitly coming from RouteData, the query string, or a form. Until that is implemented the above solution should work. Please let us know if it doesn't!

So it appears that this behaviour is 'correct', however it is so orthogonal to the principle of least astonishment that I still can't quite believe it.


Edit #1: Note that the post details a method of how to provide default values, however this no longer works as the ActionMethod property he uses to access the MethodInfo has been removed in the latest version of ASP.NET MVC. I'm currently working on an alternative and will post it when done.


Edit #2: I've updated the idea in the linked post to work with the Preview 5 release of ASP.NET MVC and I believe it should also work with the Beta release though I can't guarantee it as we haven't moved to that release yet. It's so simple that I've just posted it inline here.

First there's the default attribute (we can't use the existing .NET DefaultValueAttribute as it needs to inherit from CustomModelBinderAttribute):

[AttributeUsage(AttributeTargets.Parameter)]
public sealed class DefaultAttribute : CustomModelBinderAttribute
{
    private readonly object value;

    public DefaultAttribute(object value)
    {
        this.value = value;
    }

    public DefaultAttribute(string value, Type conversionType)
    {
        this.value = Convert.ChangeType(value, conversionType);
    }

    public override IModelBinder GetBinder()
    {
        return new DefaultValueModelBinder(this.value);
    }
}

The the custom binder:

public sealed class DefaultValueModelBinder : IModelBinder
{
    private readonly object value;

    public DefaultValueModelBinder(object value)
    {
        this.value = value;
    }

    public ModelBinderResult BindModel(ModelBindingContext bindingContext)
    {
        var request = bindingContext.HttpContext.Request;
        var queryValue = request .QueryString[bindingContext.ModelName];
        return string.IsNullOrEmpty(queryValue) 
            ? new ModelBinderResult(this.value) 
            : new DefaultModelBinder().BindModel(bindingContext);
    }
}

And then you can simply apply it to the method parameters that come in on the querystring, e.g.

public ActionResult Foo([Default(1)] int x)
{
    // implementation
}

Works like a charm!

Greg Beech
grr thanks for the solution, this was bugging me!
Andrew Bullock
+18  A: 

The best way to look at ASP.NET MVC with QueryStrings is to think of them as values that the route does not know about. As you found out, the QueryString is not part of the RouteData, therefore, you should keep what you are passing as a query string separate from the route values.

A way to work around them is to create default values yourself in the action if the values passed from the QueryString are null.

In your example, the route knows about x, therefore your url should really look like this:

/Test/Foo or /Test/Foo/5

and the route should look like this:

routes.MapRoute("test", "Test/Foo/{x}", new {controller = "Test", action = "Foo", x = 1});

To get the behavior you were looking for.

If you want to pass a QueryString value, say like a page number then you would do this:

/Test/Foo/5?page=1

And your action should change like this:

public ActionResult Foo(int x, int? page)
{
    Trace.WriteLine(x);
    Trace.WriteLine(page.HasValue ? page.Value : 1);
    return new EmptyResult();
}

Now the test:

Url:  /Test/Foo
Trace:
1
1

Url:  /Test/Foo/5
Trace:
5
1

Url:  /Test/Foo/5?page=2
Trace:
5
2

Url:  /Test/Foo?page=2
Trace:
1
2

Hope this helps clarify some things.

Dale Ragan
Yes, thanks, that makes sense. I think what confused the issue is that the route does put the defaults into the parameter values, even though it shouldn't really know about them. But I'll keep things properly separated now I understand the semantic difference to the routing engine.
Greg Beech
A: 

I think the reason querystring parameters do not override the defaults is to stop people hacking the url.

Someone could use a url whose querystring included controller, action or other defaults you didn't want them to change.

I've dealt with this problem by doing what @Dale-Ragan suggested and dealing with it in the action method. Works for me.

Richard