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?

Improve this question
5
  • In route config where u define the controller and action and parameter definition there u can specify the parameters as nullable. Commented Oct 15, 2013 at 10:45
  • @AnirudhAgarwal that will probably throw when trying to convert the null to an int Commented Oct 15, 2013 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. Commented Oct 15, 2013 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. Commented Oct 15, 2013 at 11:07
  • possible duplicate of How to make custom error pages work in ASP.NET MVC 4 Commented Oct 15, 2013 at 11:07

2 Answers 2

Reset to default
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.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

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.
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>
Improve this answer

3 Comments

Interesting, though this will handle all ArgumentExceptions, 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.)
If you want this to be centralized you put it in FilterConfig. I've updated my answer to show this.
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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.