Reputation: 119856
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?
Upvotes: 17
Views: 4023
Reputation: 105091
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)
{
...
}
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.
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.
Upvotes: 6
Reputation: 102468
I managed to get this working by adding this route at the end of all routes:
routes.MapRoute("CatchAllErrors", "{*url}",
new { controller = "Error", action = "NotFound" }
);
Note: First I followed this: How can I properly handle 404 in ASP.NET MVC?
Upvotes: 0
Reputation: 39501
Disclaimer: this does not cover all the cases
For urls in your examples, returning 404 can be done in single line. Just add route constraint for id
parameter.
routes.MapRoute(
"Default", // Route name
"{controller}/{action}/{id}", // URL with parameters
new { controller = "Home", action = "Index" }, // Parameter defaults
new { id = @"\d+" } // restrict id to be required and numeric
);
And that's all. Now any matching url that has no id
or id
is not numeric, autimatically triggers not found error (for which there are plenty of ways to handle, one in your example, another by using custom HandleErrorAttribute
, etc). And you can use non-nullable int
parameters on your actions.
Upvotes: 1