6

I have an internet-facing website that's built using MVC4 and I occasionally get error reports from bots or curious users who send requests for incomplete URLs.

For example:

public class ProductController : Controller
{
    [HttpGet]
    public void View(int id)
    {
        // ...
  • A GET request to /product/view/1 is valid.
  • A GET request to /product/view is invalid as the argument is not specified.

Such invalid requests raise exceptions resembling:

System.ArgumentException: The parameters dictionary contains a null entry
for parameter 'id' of non-nullable type 'System.Int32' for method
'System.Web.Mvc.ActionResult View(Int32)' in 'Foo.ProductController'. An
optional parameter must be a reference type, a nullable type, or be declared
as an optional parameter.

Parameter name: parameters
   at System.Web.Mvc.ActionDescriptor.ExtractParameterFromDictionary(ParameterInfo parameterInfo, IDictionary`2 parameters, MethodInfo methodInfo)
   at System.Web.Mvc.ReflectedActionDescriptor.<>c__DisplayClass1.<Execute>b__0(ParameterInfo parameterInfo)
   ...

As the exception message states, I could make the id argument nullable and check within the action method, but I have many controllers with many actions.

I'd like to return a BadRequest/NotFound response to any request that fails to bind arguments to action parameters, and specify this in a single place in code to apply across all controllers.

How can this be done?

Drew Noakes
  • 300,895
  • 165
  • 679
  • 742
  • In route config where u define the controller and action and parameter definition there u can specify the parameters as nullable. – Anirudh Agarwal Oct 15 '13 at 10:45
  • @AnirudhAgarwal that will probably throw when trying to convert the null to an int – Panagiotis Kanavos Oct 15 '13 at 10:50
  • @AnirudhAgarwal, the only way that'd work would be if I included a default value for `id` which I don't want to do. If `id` is defined on the action and it's not provided, then the client made a mistake and I want to send them a 404. Also, I have various different controllers, each of which would require entries in my route config. The whole point here is that I don't want to make custom changes across my actions and config just in case some user/bot sends invalid requests -- they should be handled centrally and not raise exceptions that cause me to receive error emails which I just delete. – Drew Noakes Oct 15 '13 at 10:50
  • 1
    I think the problems comes from having an optional id in the Route and non-matching Actions. With a consistent design you wouldn't have to do anything. – H H Oct 15 '13 at 11:07
  • possible duplicate of [How to make custom error pages work in ASP.NET MVC 4](http://stackoverflow.com/questions/13905164/how-to-make-custom-error-pages-work-in-asp-net-mvc-4) – Panagiotis Kanavos Oct 15 '13 at 11:07

2 Answers2

4

One approach that seems to work is to override OnActionExecuted in the controller (I use a base controller, so would put it there.)

protected override void OnActionExecuted(ActionExecutedContext filterContext)
{
    if (filterContext.Exception == null)
        return;

    // Avoid 'action parameter missing' exceptions by simply returning an error response
    if (filterContext.Exception.TargetSite.DeclaringType == typeof(ActionDescriptor) &&
        filterContext.Exception.TargetSite.Name == "ExtractParameterFromDictionary")
    {
        filterContext.ExceptionHandled = true;
        filterContext.Result = new HttpStatusCodeResult((int)HttpStatusCode.BadRequest);
    }
}

It feels a little uncomfortable to do this as it may break in future versions of the framework. However if it does break, then the site will revert to returning 500's instead of 400's.

Drew Noakes
  • 300,895
  • 165
  • 679
  • 742
  • 2
    You could also override the OnException method for the same effect. Don't know if it matters but I guess OnActionExecuted is called for every action method called whereas OnException is only called in case of an exception. Just a thought. – Doktorn Oct 16 '13 at 11:50
3

You can use the HandleError attribute to handle errors in your application. The HandleError attribute can be specified both at controller level and method level. I have used something like this before:

[HandleError(ExceptionType = typeof(ArgumentException), View = "MissingArgument")]
public ActionResult Test(int id)
{
    return View();
}

If you don't want to catch exceptions on a per method basis you can put the attribute on class level instead:

[HandleError(ExceptionType = typeof(ArgumentException), View = "MissingArgument")]
public class HomeController : Controller
{
}

If you want to handle this in a central place you can add it to the FilterConfig class in the App-Start folder:

public static void RegisterGlobalFilters(GlobalFilterCollection filters)
{
    var error = new HandleErrorAttribute();
    error.ExceptionType = typeof (ArgumentException);
    error.View = "MissingArgument";
    filters.Add(error);
}

The MissingArgument view should be located in the Shared view folder. If you want to send a specific HTTP error code back to the client you can put that in the view:

@{
    ViewBag.Title = "Error";
    Context.Response.StatusCode = (int) HttpStatusCode.BadRequest;
 }

<h2>Not found</h2>
Doktorn
  • 777
  • 5
  • 7
  • Interesting, though this will handle all `ArgumentException`s, whereas I only want those related to the inability to bind action parameters. Also, I'd want to put this filter in a central location rather than on each controller (which I think is possible with some small code changes.) – Drew Noakes Oct 16 '13 at 10:22
  • If you want this to be centralized you put it in FilterConfig. I've updated my answer to show this. – Doktorn Oct 16 '13 at 10:52
  • Thanks for the update. In the end I went with overriding `OnActionExecuted` in my base controller and targeting the exact error, rather than all `ArgumentException` instances. My concern is that I'll suppress error logging/emailing of a legitimate argument exception thrown under different circumstances. – Drew Noakes Oct 16 '13 at 11:03