Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Commit

Permalink
Set StatusCode before disposing HttpContext (#876)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikeharder authored and Cesar Blum Silveira committed Oct 11, 2016
1 parent 8c103f0 commit a784f1e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 13 deletions.
27 changes: 15 additions & 12 deletions src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,24 @@ public override async Task RequestProcessingAsync()
await FireOnCompleted();
}

_application.DisposeContext(context, _applicationException);
}

// If _requestAbort is set, the connection has already been closed.
if (Volatile.Read(ref _requestAborted) == 0)
{
ResumeStreams();

if (_keepAlive)
// If _requestAbort is set, the connection has already been closed.
if (Volatile.Read(ref _requestAborted) == 0)
{
// Finish reading the request body in case the app did not.
await messageBody.Consume();
ResumeStreams();

if (_keepAlive)
{
// Finish reading the request body in case the app did not.
await messageBody.Consume();
}

// ProduceEnd() must be called before _application.DisposeContext(), to ensure
// HttpContext.Response.StatusCode is correct set when
// IHttpContextFactory.Dispose(HttpContext) is called.
await ProduceEnd();
}

await ProduceEnd();
_application.DisposeContext(context, _applicationException);
}

StopStreams();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.ObjectPool;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;
using Moq;
using Xunit;
Expand Down Expand Up @@ -175,13 +179,50 @@ public async Task OnStartingThrowsWhenSetAfterResponseHasAlreadyStarted()
using (var client = new HttpClient())
{
var response = await client.GetAsync($"http://localhost:{host.GetPort()}/");

// Despite the error, the response had already started
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.NotNull(ex);
}
}
}

[Fact]
public async Task ResponseStatusCodeSetBeforeHttpContextDispose()
{
var mockHttpContextFactory = new Mock<IHttpContextFactory>();
mockHttpContextFactory.Setup(f => f.Create(It.IsAny<IFeatureCollection>()))
.Returns<IFeatureCollection>(fc => new DefaultHttpContext(fc));

int disposedStatusCode = -1;
mockHttpContextFactory.Setup(f => f.Dispose(It.IsAny<HttpContext>()))
.Callback<HttpContext>(c => disposedStatusCode = c.Response.StatusCode);

var hostBuilder = new WebHostBuilder()
.UseKestrel()
.UseUrls("http://127.0.0.1:0")
.ConfigureServices(services => services.AddSingleton<IHttpContextFactory>(mockHttpContextFactory.Object))
.Configure(app =>
{
app.Run(context =>
{
throw new Exception();
});
});

using (var host = hostBuilder.Build())
{
host.Start();

using (var client = new HttpClient())
{
var response = await client.GetAsync($"http://localhost:{host.GetPort()}/");
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
}
}

Assert.Equal(HttpStatusCode.InternalServerError, (HttpStatusCode)disposedStatusCode);
}

// https://github.com/aspnet/KestrelHttpServer/pull/1111/files#r80584475 explains the reason for this test.
[Fact]
Expand Down

0 comments on commit a784f1e

Please sign in to comment.