3

Closed

HttpControllerHandler.EndProcessRequest fails when a message handler sets a new response + terminates the request and the other tries to reach out to response.RequestMessage

description

If I were to have two message handlers registered and one of them sets a new response message + terminates the pipeline and the other tries to reach out to response.RequestMessage object, System.Web.Http.WebHost.HttpControllerHandler.EndProcessRequest throws "Object reference not set to an instance of an object" NullReferenceException. Repro steps:

First message handler which modified MS_HttpContext:

public class RemoveServerHeaderHandler : DelegatingHandler {
protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, 
    CancellationToken cancellationToken) {

    return base.SendAsync(request, cancellationToken).ContinueWith(task => {

        var response = task.Result;

        var httpContext = response.RequestMessage.Properties["MS_HttpContext"] as HttpContextWrapper;
        if (httpContext != null)
            httpContext.Response.Headers.Remove("Server");

        return response;
    });
}
}

Second message handler which terminates the request:

public class TerminatingMessageHandler : DelegatingHandler {
protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, 
    CancellationToken cancellationToken) {

    var response = new HttpResponseMessage(HttpStatusCode.OK);
    return Task.FromResult(response);
}
}

Registering them:

protected void Application_Start(object sender, EventArgs e) {
var config = GlobalConfiguration.Configuration;
config.Routes.MapHttpRoute(
    "DefaultApiRoute",
    "api/{controller}/{id}",
    new { id = RouteParameter.Optional }
);

config.MessageHandlers.Add(new RemoveServerHeaderHandler());
config.MessageHandlers.Add(new TerminatingMessageHandler());
}

When we run the application and get into the Web API pipeline, message handlers are invoked and following error message is thrown:

Object reference not set to an instance of an object.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.NullReferenceException: Object reference not set to an instance of an object.

Source Error:

An unhandled exception was generated during the execution of the current web request. Information regarding the origin and location of the exception can be identified using the exception stack trace below.

Stack Trace:


[NullReferenceException: Object reference not set to an instance of an object.]
System.Web.Http.WebHost.HttpControllerHandler.EndProcessRequest(IAsyncResult result) +112
System.Web.Http.WebHost.HttpControllerHandler.System.Web.IHttpAsyncHandler.EndProcessRequest(IAsyncResult result) +10
System.Web.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +9629708
System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +155

I also attached the sample project. Just send a request to /api/cars.

Note: I have a comment below which explains the cause (I think).

file attachments

Closed Sep 16, 2013 at 10:33 PM by yishaigalatzer
This is easy to workaround (just set the request, as Kiran mentions above), and considering this is a minor breaking change, we decided not to fix this issue.

comments

tugberk wrote Sep 12, 2012 at 7:35 AM

here is the attachment.

tugberk wrote Sep 12, 2012 at 7:49 AM

I think I found out what is causing this. The below one also fails on his own, no need for another handler:

public class TerminatingMessageHandler : DelegatingHandler {
protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, 
    CancellationToken cancellationToken) {

    var response = new HttpResponseMessage(HttpStatusCode.OK);
    return Task.FromResult(response).ContinueWith(task => {

        var responseMsg = task.Result;

        var httpContext = responseMsg.RequestMessage.Properties["MS_HttpContext"] as HttpContextWrapper;
        if (httpContext != null)
            httpContext.Response.Headers.Remove("Server");

        return responseMsg;
    });
}
}

The problem is that as we create the response from scratch, it doesn't carry the RequestMessage. So, it fails. The blow code, which sets the RequestMessage property of the HttpResponseMessage, solves the problem but I believe this should be done by the framework:

public class TerminatingMessageHandler : DelegatingHandler {
protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, 
    CancellationToken cancellationToken) {

    var response = new HttpResponseMessage(HttpStatusCode.OK);
    response.RequestMessage = request;
    return Task.FromResult(response).ContinueWith(task => {

        var responseMsg = task.Result;

        var httpContext = responseMsg.RequestMessage.Properties["MS_HttpContext"] as HttpContextWrapper;
        if (httpContext != null)
            httpContext.Response.Headers.Remove("Server");

        return responseMsg;
    });
}
}

kichalla wrote Sep 12, 2012 at 4:03 PM

hi tugberk,

In this case you can use "Request.CreateResponse" extension which sets the Request message property for the response.

tugberk wrote Sep 12, 2012 at 4:42 PM

yes I know but this is not the case. Framework should force to set the current request to RequestMessage property of the response message that a message handler might generate.

This is really error prone and I believe that this should be fixed.

riles01 wrote Sep 18, 2012 at 3:37 AM

I tend to agree with Kiran. The request message object is available for capture in the message handler. Also, a request message should not be required in a return message. If the framework is expecting a request somewhere, then that's a different matter.

tugberk wrote Sep 18, 2012 at 7:19 AM

This is completely a bug IMO. The Framework should ensure that Response message returned from each message handler carries a RequestMessage object inside the message handler pipeline as it is already done inside the ApiController pipeline.

Have a look at the line 13: https://gist.github.com/3722483#L13

tugberk wrote Sep 18, 2012 at 7:47 AM

This will make incorporating code from others a pain as well because I can see that there are lots of message handlers out there which new up a new HttpResponseMesage and those Reponse objects don't carry the RequestMessage property.

Of course, I can just use the request parameter which is passed into the SendAsync method instead of RequestMessage property of the HttpResponseMessage as HttpRequestMessage is a reference type and both request parameter and RequestMessage property will point to the same object inside the heap but still, this looks like a bug to me.

HongmeiG wrote Sep 26, 2012 at 5:51 AM

We should consider auto wire up the RequestMessage if we can.