Elmah not logging exceptions for http post requests in MVC app - if the request contains XML

Question

I've come across a weird problem in my MVC4 (RC) application. (running on .NET 4.0)

I have just setup Elmah for logging exceptions / errors.

I basically installed the Elmah.MVC and elmah.sqlserver NuGet packages. (versions 2.0.0 and 1.2 respectively)

It seemed to work fine out of the box - I can go to the elmah page and view errors:

http://myserver/elmah

for example, if I create some 404 errors, they appear in this log.

What is not working is this: I have a standard MVC controller with a [HttpPost] action. I've set it up so it will always throw an exception:

public class TestController : Controller
{
    [HttpPost]
    [ValidateInput(false)]
    public void Testing()
    {
        throw new Exception("uh oh");
    }
}

I then try to post data to this controller via jQuery:

$.post('/Test/Testing', {test_data: 'This is some test data'});

Ok, this works. The response returns the typical yellow screen of death, and the error is caught and logged in Elmah.

However, if I try to post something like XML/HTML the error is not logged in Elmah. I still get the same response from the server back (yellow screen of death), but nothing in Elmah.

$.post('/Test/Testing', {test_data: '<test><test1>This is some test data</test1></test>'});

Why? It doesn't make sense.

Notice I have already turned off the request validation on the action. If I didn't do that, then posting XML/HTML data would cause this exception:

A potentially dangerous Request.Form value was detected from the client

NuGet would also refuse to log that exception too - which I believe is a bug:

http://code.google.com/p/elmah/issues/detail?id=217

So what is the cause of this problem that I'm experiencing? It it a bug related to the issue I found above?

It just seems quite an unfortunate situation that I can't log exceptions just because the request contained XML/HTML.

Surely there is a way around this?

Answer 1

ELMAH does not catch HttpRequestValidationException by default and if a user sends an invalid request it will be missed in ELMAH's report. so it's necessary to define and use this global filter as well:

public class ElmahRequestValidationErrorFilter : IExceptionFilter
{
    public void OnException(ExceptionContext context)
    {
        if (context.Exception is HttpRequestValidationException)
           ErrorLog.GetDefault(HttpContext.Current).Log(new Error(context.Exception));
    }
}

Answer 2

I have a work around for now, which someone suggested on http://code.google.com/p/elmah/issues/detail?id=217

You can force ASP to use the older request validation logic by adding this into the <system.web> section of your Web.config:

<httpRuntime requestValidationMode="2.0" />

This will effect the app globally which is not really that great of a situation.

If anyone has something better, please let me know.

Answer 3

Looks like this was fixed after the current release (as of this writing) 1.2.2. I ended up explicitly logging the error in the log. This won't go through the normal processing as unhandled exceptions (email notifications, etc) but it will log it to the error log.

catch (Exception ex)
{
    var newEx = new HttpException(500, "Kaboom!!!", ex);

    // Adding this explicit ELMAH logging because of ELMAH bug:
    // https://code.google.com/p/elmah/issues/detail?id=217
    // Waiting for a new ELMAH release with the fix
    var elmahError = new Elmah.Error(newEx);
    var elmahLog = Elmah.ErrorLog.GetDefault(HttpContext.ApplicationInstance.Context);
    elmahLog.Log(elmahError);

    throw newEx;
}

Answer 4

There was a bug in Elmah that was fixed a while back, but I don't believe any of the builds on the google code site are recent enough to include it.

As I mentioned in my comment, the issue is that if you access data through the model (MVC) or control (webforms), no problem; but if you access request.Form["inputId"] (which Elmah does when building one object), it throws an exception every time.

What I did:

• Export the google code project to my github account
• Clone the new repo to my workstation using VS2013
• Select the NETFX 4.5 build configuration
• Build the solution, and run the demo
• Test my case in the demo by adding the following to default.aspx:

• Run some tests

After I was happy with my tests, I added the new dlls to my solution by:

• Removing the current Elmah.dll reference
• Copy/paste Elmah.dll, Elmah.AspNet.dll, and AntiXssLibrary.dll to a 'lib' folder in my solution
• Add references to the three new dlls
• Update web.config so that it looks like the demo's web.config (things are very similar, but different (Elmah = Elmah.AspNet))
• Remove the Elmah.mvc reference (it caused problems for me)
• Remove filters.Add(new HandleErrorAttribute()); from FilterConfig (this allows custom errors to continue to work)

In the Elmah source, the crucial piece is the use of request.Unvalidated in HttpRequestValidation.cs

Answer 5

You can get Elmah to fire by throwing an HttpException instead of a normal exception.

Our solution was to wrap our controller action code with a try/catch block, and in the catch block wrap the exception that is thrown by your code in an HttpException and throw that instead. Like this:

[HttpPost]
[ValidateInput(false)]
public ActionResult MyAction(FormCollection riskyData)
{
    try
    {
        //... your code here ...
    }
    catch (Exception ex)
    {
        throw new HttpException(500, "Internal Server Error", ex);
    }
}

Answer 6

Add the following classes to your project:

public class ElmahErrorLogModuleFix : ErrorLogModule
{
    protected override void LogException(Exception e, HttpContext context)
    {
        if (e == null)
            throw new ArgumentNullException("e");
        ExceptionFilterEventArgs args = new ExceptionFilterEventArgs(e, (object)context);
        this.OnFiltering(args);
        if (args.Dismissed)
            return;
        ErrorLogEntry entry = (ErrorLogEntry)null;
        try
        {
            //FIX STARTS
            //Error error = new Error(e, context);
            Error error = CreateErrorSafe(e, context);
            //FIX ENDS
            ErrorLog errorLog = this.GetErrorLog(context);
            error.ApplicationName = errorLog.ApplicationName;
            string id = errorLog.Log(error);
            entry = new ErrorLogEntry(errorLog, id, error);
        }
        catch (Exception ex)
        {
            Trace.WriteLine((object)ex);
        }
        if (entry == null)
            return;
        this.OnLogged(new ErrorLoggedEventArgs(entry));
    }

    public static Error CreateErrorSafe(Exception e, HttpContext context)
    {
        try
        {
            var safeFormCollection = new NameValueCollection();
            var form = context.Request.Form;
            var additionalMessage = string.Empty;
            foreach (var key in form.AllKeys)
            {
                try
                {
                    safeFormCollection.Add(key, form[key]);
                }
                catch (Exception)
                {
                    safeFormCollection.Add(key, "_invalid input data_");
                    additionalMessage += "Form parameter with name=" + key + " has dangerous value. " + Environment.NewLine;
                }
            }

            //if no invalid values in form then do as elmah does
            if (string.IsNullOrEmpty(additionalMessage))
            {
                return new Error(e, context);
            }

            var exception = new Exception(additionalMessage, e);
            var error = new Error(exception);
            error.HostName = TryGetMachineName(context, null);

            IPrincipal user = context.User;
            if (user != null && NullString(user.Identity.Name).Length > 0)
                error.User = user.Identity.Name;
            HttpRequest request = context.Request;
            //this._serverVariables = Error.CopyCollection(request.ServerVariables);
            error.ServerVariables.Add(CopyCollection(request.ServerVariables));
            if (error.ServerVariables != null && error.ServerVariables["AUTH_PASSWORD"] != null)
                error.ServerVariables["AUTH_PASSWORD"] = "*****";
            error.QueryString.Add(CopyCollection(request.QueryString));
            error.Form.Add(CopyCollection(safeFormCollection));
            error.Cookies.Add(CopyCollection(request.Cookies));
            return error;
        }
        catch (Exception logEx)
        {
            return new Error(new Exception("Error when trying to process error catched by elmah", logEx));
        }
    }

    /// <summary>
    /// Elmah dll method in Environment.cs
    /// </summary>
    /// <param name="context"></param>
    /// <param name="unknownName"></param>
    /// <returns></returns>
    public static string TryGetMachineName(HttpContext context, string unknownName)
    {
        if (context != null)
        {
            try
            {
                return context.Server.MachineName;
            }
            catch (HttpException ex)
            {
            }
            catch (SecurityException ex)
            {
            }
        }
        try
        {
            return System.Environment.MachineName;
        }
        catch (SecurityException ex)
        {
        }
        return NullString(unknownName);
    }

    /// <summary>
    /// Elmah method in Mask.cs
    /// </summary>
    /// <param name="s"></param>
    /// <returns></returns>
    public static string NullString(string s)
    {
        if (s != null)
            return s;
        else
            return string.Empty;
    }

    /// <summary>
    /// Elmah method in Error.cs
    /// </summary>
    /// <param name="collection"></param>
    /// <returns></returns>
    private static NameValueCollection CopyCollection(NameValueCollection collection)
    {
        if (collection == null || collection.Count == 0)
            //FIX HERE: cannot allow reutrn null collection as elmah does, because of exception. fix as below
            //return (NameValueCollection)null;
            return new NameValueCollection();
        //FIX ENDS
        else
            return new NameValueCollection(collection);
    }

    /// <summary>
    /// Elmah method in Error.cs
    /// </summary>
    /// <param name="cookies"></param>
    /// <returns></returns>
    private static NameValueCollection CopyCollection(HttpCookieCollection cookies)
    {
        if (cookies == null || cookies.Count == 0)
            //FIX HERE: cannot allow reutrn null collection as elmah does, because of exception. fix as below
            //return (NameValueCollection)null;
            return new NameValueCollection();
        //FIX ENDS
        NameValueCollection nameValueCollection = new NameValueCollection(cookies.Count);
        for (int index = 0; index < cookies.Count; ++index)
        {
            HttpCookie httpCookie = cookies[index];
            nameValueCollection.Add(httpCookie.Name, httpCookie.Value);
        }
        return nameValueCollection;
    }
}

and

public class ElmahErrorMailModuleFix : ErrorMailModule
{
    private bool _reportAsynchronously2;

    protected override void OnInit(HttpApplication application)
    {
        base.OnInit(application);
        IDictionary config = (IDictionary)this.GetConfig();
        if (config == null)
            return;
        _reportAsynchronously2 = Convert.ToBoolean(GetSetting(config, "async", bool.TrueString));
    }

    protected override void OnError(Exception e, HttpContext context)
    {
        if (e == null)
            throw new ArgumentNullException("e");
        ExceptionFilterEventArgs args = new ExceptionFilterEventArgs(e, (object)context);
        this.OnFiltering(args);
        if (args.Dismissed)
            return;
        //FIX STARTS
        //Error error = new Error(e, context);
        Error error = ElmahErrorLogModuleFix.CreateErrorSafe(e, context);
        //FIX ENDS
        if (this._reportAsynchronously2)
            this.ReportErrorAsync(error);
        else
            this.ReportError(error);
    }

    /// <summary>
    /// Elmah method in ErrorMailModule.cs
    /// </summary>
    /// <param name="config"></param>
    /// <param name="name"></param>
    /// <param name="defaultValue"></param>
    /// <returns></returns>
    private static string GetSetting(IDictionary config, string name, string defaultValue)
    {
        string str = ElmahErrorLogModuleFix.NullString((string)config[(object)name]);
        if (str.Length == 0)
        {
            if (defaultValue == null)
                throw new global::Elmah.ApplicationException(string.Format("The required configuration setting '{0}' is missing for the error mailing module.", (object)name));
            str = defaultValue;
        }
        return str;
    }
}

These classes inherit from ErrorLogModule and ErrorMailModule and rewrite methods where the Error class is created, so that the HttpRequestValidationException exception will not raise.

Then add these to your Web.config:

<add name="ErrorLog" type="YourProject.SomeFolder.ElmahErrorLogModuleFix, YourProject" preCondition="managedHandler" />
<!--and for email module-->

to use these classes instead of the original ones. A bit of a dirty hack, but it works.

Credit goes to the poster of message #17 found here.