How do I return a 404 status where invalid parameters are passed to my ASP.NET MVC controller?

Question

I want to return a HTTP status 404 if invalid arguments are passed to my controller. For example if I have a controller that looks like:

public ActionResult GetAccount(int id)
{
   ...
}

Then I want to return a 404 if say urls such as these are encountered:

/GetAccount
/GetAccount/notanumber

i.e. I want to trap the ArgumentException that is thrown.

I know I could use a nullable type:

public ActionResult GetAccount(int? id)
{
  if(id == null) throw new HttpException(404, "Not found");
}

But that's pretty icky and repetitious.

I was hoping I could add this to my controllers where necessary:

[HandleError(View="Error404", ExceptionType = typeof(ArgumentException))]
public class AccountsController : Controller
{
  public ActionResult GetAccount(int id)
  {
    ...
  }
}

But that doesn't appear to work well.

I saw this post and this answer which nearly solves my problem:

In that answer an abstract BaseController is created from which you derive all your other controllers from:

public abstract class MyController : Controller
{
    #region Http404 handling

    protected override void HandleUnknownAction(string actionName)
    {
        // If controller is ErrorController dont 'nest' exceptions
        if (this.GetType() != typeof(ErrorController))
            this.InvokeHttp404(HttpContext);
    }

    public ActionResult InvokeHttp404(HttpContextBase httpContext)
    {
        IController errorController = ObjectFactory.GetInstance<ErrorController>();
        var errorRoute = new RouteData();
        errorRoute.Values.Add("controller", "Error");
        errorRoute.Values.Add("action", "Http404");
        errorRoute.Values.Add("url", httpContext.Request.Url.OriginalString);
        errorController.Execute(new RequestContext(
             httpContext, errorRoute));

        return new EmptyResult();
    }

    #endregion
}

This works great at handling unknown actions with a 404 but doesn't allow me to handle invalid data as a 404.

Can I safely override Controller.OnException(ExceptionContext filterContext) like this:

protected override void OnException(ExceptionContext filterContext)
{
  if(filterContext.Exception.GetType() == typeof(ArgumentException))
  {
    filterContext.ExceptionHandled = true;
    this.InvokeHttp404(filterContext.HttpContext);
  }
  else
  {
    base.OnException(filterContext);
  }
}

On the surface it seems to work, but am I storing up any problems by doing this?

Is this semantically correct thing to do?

Answer 1

Best way? Action method selector attribute!

To actually avoid nullable method arguments I suggest that you write an Action Method Selector attribute that will actually only match your action method when id is supplied. It won't say that argument wasn't supplied but that it couldn't match any action methods for the given request.

I would call this action selector RequireRouteValuesAttribute and would work this way:

[RequireRouteValues("id")]
public ActionResult GetAccount(int id)
{
    ...
}

Why is this the best solution for your problem?

If you look at your code you'd like to return a 404 on actions that match name but parameter binding failed (either because it wasn't supplied or any other reason). Your action so to speak requires particular action parameter otherwise a 404 is returned.

So when adding action selector attribute adds the requirement on the action so it has to match name (this is given by MVC) and also require particular action parameters. Whenever id is not supplied this action is not matched. If there's another action that does match is not the issue here because that particular action will get executed. The main thing is accomplished. Action doesn't match for invalid route request and a 404 is returned instead.

There's an app code for that!

Check my blog post that implements this kind of attribute that you can use out of the box. It does exactly what you're after: it won't match your action method if route data provided doesn't have all required values.