Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

CustomErrors vs HttpErrors - A significant design flaw?

As we might know, from e.g. What is the difference between customErrors and httpErrors?, CustomErrors is a older way to define error pages in a web application but this approach have some issues - for example if you care about correct http response codes since the approach of CustomErrors is to redirect to an error page instead of replacing the current response, which destroys most of the semantic integrity of the communication by http status codes.

HttpErrors is a newer feature available since IIS 7.0 which is running on the server level instead of the application level, and is much better suited to handle error responses in a valid way, for example by using the current response instead of redirecting.

However, if seems to me that the infrastructure for these artifacts in ASP.NET as it looks today gives us some problem.

A simple configuration as example

<httpErrors existingResponse="Auto" errorMode="Custom">
    <remove statusCode="404"/>
    <error statusCode="404" path="/Error/E404" responseMode="ExecuteURL" />
</httpErrors>

We set errorMode to Custom because we want to test the error handling itself, and we set existingResponse to Auto, which will introduce a branch dependent on Response.TrySkipIisCustomErrors:

  • True: the existing erroneous response will pass through this module unhandled, which makes total sense considering it's semantic meaning.
  • False: the existing erroneous response will be replaced by the HttpErrors module if it has a matching rule, which makes just as much sense.

This would ideally allow us to handle some errors ourselves, like when the product in ../product/id does not exist, where we can manually can return a specific 404 page with information about missing products and still let the HttpErrors module handle all the rest like ../products/namebutshouldbeid or just ../misspelledandunmatchableurl.

However, as far as I can see, this is not working. The reason is in the internal method System.Web.HttpResponse.ReportRuntimeError, which will be called on runtime errors (e.g. controllers/actions not found) and where we have a section that looks like this:

// always try to disable IIS custom errors when we send an error
if (_wr != null) {
    _wr.TrySkipIisCustomErrors = true;
}

if (!localExecute) {
code = HttpException.GetHttpCodeForException(e);

// Don't raise event for 404.  See VSWhidbey 124147.
if (code != 404) {
    WebBaseEvent.RaiseRuntimeError(e, this);
}

// This cannot use the HttpContext.IsCustomErrorEnabled property, since it must call
// GetSettings() with the canThrow parameter.
customErrorsSetting = CustomErrorsSection.GetSettings(_context, canThrow);
if (customErrorsSetting != null)
    useCustomErrors = customErrorsSetting.CustomErrorsEnabled(Request);
else
    useCustomErrors = true;
}

While first debugging this I saw that useCustomErrors was set to false, and I could not understand why since I knew that I had a working HttpError configuration, since it works where I return a HttpNotFoundResult from a controller.

Then I realized that this is not HttpErrors, but the older CustomErrors. And the CustomErrors has obviously no knowledge of HttpErrors.

The "bug"

So what is happening is that Response.TrySkipIisCustomErrors is set to true and since no CustomErrors are defined, it returns the detailed 404 response. At this point we would like HttpErrors to kick in, but it wont because of TrySkipIisCustomErrors now is set to true.

And we can't use CustomErrors either, since this would bring us back to the issues with repugnant error redirections.

And the reason that returning HttpNotFoundResult works is because it will not trigger a runtime error, only return a 404-result which HttpErrors will intercept as expected, as long as we avoid to set Response.TrySkipIisCustomErrors to true.

How should/could this be handled/resolved?

I'm thinking that System.Web.HttpResponse.ReportRuntimeError should not be allowed to set Response.TrySkipIisCustomErrors to true by default, since we have another errorhandling module dependent on this. Thus, this method needs to be aware of any HttpErrors configuration as well, either by avoiding setting TrySkipIisCustomErrors to true if we have any CustomErrors configuration, or that it handles the HttpErrors configuration together with the CustomErrors configuration.

Or have I missed some secret magic to resolve this?

like image 536
Alex Avatar asked Jun 28 '14 09:06

Alex


1 Answers

I've been struggling for several days to solve this problem and I think the only valid solution is the one posted here (An answer by Starain chen to the same question posted by @Alex on forums.asp.net):

(I've slightly modified the code)

The code

Create a custom handle error attribute

public class CustomHandleErrorAttribute : HandleErrorAttribute {
    public override void OnException (ExceptionContext filterContext) {
        if (filterContext.ExceptionHandled) {
            return;
        }

        var httpException = new HttpException(null, filterContext.Exception);
        var httpStatusCode = httpException.GetHttpCode();

        switch ((HttpStatusCode) httpStatusCode) {
            case HttpStatusCode.Forbidden:
            case HttpStatusCode.NotFound:
            case HttpStatusCode.InternalServerError:
                break;

            default:
                return;
        }

        if (!ExceptionType.IsInstanceOfType(filterContext.Exception)) {
            return;
        }

        // if the request is AJAX return JSON else view.
        if (filterContext.HttpContext.Request.Headers["X-Requested-With"] == "XMLHttpRequest") {
            filterContext.Result = new JsonResult {
                JsonRequestBehavior = JsonRequestBehavior.AllowGet,
                Data = new {
                    error = true,
                    message = filterContext.Exception.Message
                }
            };
        }
        else {
            var controllerName = (String) filterContext.RouteData.Values["controller"];
            var actionName = (String) filterContext.RouteData.Values["action"];
            var model = new HandleErrorInfo(filterContext.Exception, controllerName, actionName);

            filterContext.Result = new ViewResult {
                ViewName = String.Format("~/Views/Hata/{0}.cshtml", httpStatusCode),
                ViewData = new ViewDataDictionary(model),
                TempData = filterContext.Controller.TempData
            };
        }

        // TODO: Log the error by using your own method

        filterContext.ExceptionHandled = true;
        filterContext.HttpContext.Response.Clear();
        filterContext.HttpContext.Response.StatusCode = httpStatusCode;
        filterContext.HttpContext.Response.TrySkipIisCustomErrors = true;
    }
}

Use this custom handle error attribute in App_Start/FilterConfig.cs

public class FilterConfig {
    public static void RegisterGlobalFilters (GlobalFilterCollection filters) {
        filters.Add(new CustomHandleErrorAttribute());
    }
}

Handle the remaining exceptions in Global.asax

protected void Application_Error () {
    var exception = Server.GetLastError();
    var httpException = exception as HttpException ?? new HttpException((Int32) HttpStatusCode.InternalServerError, "Internal Server Error", exception);
    var httpStatusCode = httpException.GetHttpCode();

    Response.Clear();

    var routeData = new RouteData();

    routeData.Values.Add("Controller", "Error");
    routeData.Values.Add("fromAppErrorEvent", true);
    routeData.Values.Add("ErrorMessage", httpException.Message);
    routeData.Values.Add("HttpStatusCode", httpStatusCode);

    switch ((HttpStatusCode) httpStatusCode) {
            case HttpStatusCode.Forbidden:
            case HttpStatusCode.NotFound:
            case HttpStatusCode.InternalServerError:
                routeData.Values.Add("action", httpStatusCode.ToString());
                break;

            default:
                routeData.Values.Add("action", "General");
                break;
        }

    Server.ClearError();

    IController controller = new Controllers.ErrorController();

    // TODO: Log the error if you like

    controller.Execute(new RequestContext(new HttpContextWrapper(Context), routeData));
}

Create an ErrorController

[AllowAnonymous]
public class ErrorController : Controller {
    protected override void OnActionExecuting (ActionExecutingContext filterContext) {
        base.OnActionExecuting(filterContext);

        var errorMessage = RouteData.Values["ErrorMessage"];
        var httpStatusCode = RouteData.Values["HttpStatusCode"];

        if (errorMessage != null) {
            ViewBag.ErrorMessage = (String) errorMessage;
        }

        if (httpStatusCode != null) {
            ViewBag.HttpStatusCode = Response.StatusCode = (Int32) httpStatusCode;
        }

        Response.TrySkipIisCustomErrors = true;
    }

    [ActionName("403")]
    public ActionResult Error403 () {
        return View();
    }

    [ActionName("404")]
    public ActionResult Error404 () {
        return View();
    }

    [ActionName("500")]
    public ActionResult Error500 () {
        return View();
    }

    public ActionResult General () {
        return View();
    }
}

Create views

Create views for the actions in ErrorController. (403.cshtml, 404.cshtml, 500.cshtml and General.cshtml)

Why do I think this is the only valid solution?

  1. It handles both ASP.NET MVC and IIS-level errors (Assuming IIS7+ and integrated pipeline)
  2. It returns valid http status codes. (Not 302 or 200)
  3. It returns 200 OK if I navigate to the error page directly: I'd like to get 200 OK if I navigate to /error/404.
  4. I can adjust the error page content. (Using ViewBag.ErrorMessage)
  5. If the client makes an erroneous AJAX request and expects json data (within the actions of controllers) he/she will be served with a json data with the appropriate status code.
like image 141
mono blaine Avatar answered Oct 07 '22 09:10

mono blaine