Don't reset frame state when connection is aborted (#1103).

This commit is contained in:
Cesar Blum Silveira 2016-09-28 10:38:15 -07:00
parent 73656f6503
commit 09fda749b0
2 changed files with 71 additions and 3 deletions

View File

@ -141,8 +141,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
}
}
// Don't lose request rejection state
if (!_requestRejected)
// Don't reset frame state if we're exiting the loop. This avoids losing request rejection
// information (for 4xx response), and prevents ObjectDisposedException on HTTPS (ODEs
// will be thrown if PrepareRequest is not null and references objects disposed on connection
// close - see https://github.com/aspnet/KestrelHttpServer/issues/1103#issuecomment-250237677).
if (!_requestProcessingStopping)
{
Reset();
}

View File

@ -2,12 +2,19 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Net.Security;
using System.Net.Sockets;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions.Internal;
using Xunit;
namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
@ -78,6 +85,57 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
Assert.Equal(0, loggerFactory.ErrorLogger.TotalErrorsLogged);
}
// Regression test for https://github.com/aspnet/KestrelHttpServer/issues/1103#issuecomment-246971172
[Fact]
public async Task DoesNotThrowObjectDisposedExceptionOnConnectionAbort()
{
var x509Certificate2 = new X509Certificate2(@"TestResources/testCert.pfx", "testPassword");
var loggerFactory = new HandshakeErrorLoggerFactory();
var hostBuilder = new WebHostBuilder()
.UseKestrel(options =>
{
options.UseHttps(@"TestResources/testCert.pfx", "testPassword");
})
.UseUrls("https://127.0.0.1:0/")
.UseLoggerFactory(loggerFactory)
.Configure(app => app.Run(async httpContext =>
{
var ct = httpContext.RequestAborted;
while (!ct.IsCancellationRequested)
{
try
{
await httpContext.Response.WriteAsync($"hello, world\r\r", ct);
await Task.Delay(1000, ct);
}
catch (TaskCanceledException)
{
// Don't regard connection abort as an error
}
}
}));
using (var host = hostBuilder.Build())
{
host.Start();
using (var socket = await HttpClientSlim.GetSocket(new Uri($"https://127.0.0.1:{host.GetPort()}/")))
using (var stream = new NetworkStream(socket, ownsSocket: false))
using (var sslStream = new SslStream(stream, true, (sender, certificate, chain, errors) => true))
{
await sslStream.AuthenticateAsClientAsync("127.0.0.1", clientCertificates: null,
enabledSslProtocols: SslProtocols.Tls11 | SslProtocols.Tls12,
checkCertificateRevocation: false);
var request = Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n\r\n");
await sslStream.WriteAsync(request, 0, request.Length);
await sslStream.ReadAsync(new byte[32], 0, 32);
}
}
Assert.False(loggerFactory.ErrorLogger.ObjectDisposedExceptionLogged);
}
private class HandshakeErrorLoggerFactory : ILoggerFactory
{
public HttpsConnectionFilterLogger FilterLogger { get; } = new HttpsConnectionFilterLogger();
@ -133,12 +191,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
{
public int TotalErrorsLogged { get; set; }
public bool ObjectDisposedExceptionLogged { get; set; }
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
{
if (logLevel == LogLevel.Error)
{
TotalErrorsLogged++;
}
if (exception is ObjectDisposedException)
{
ObjectDisposedExceptionLogged = true;
}
}
public bool IsEnabled(LogLevel logLevel)
@ -148,7 +213,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
public IDisposable BeginScope<TState>(TState state)
{
throw new NotImplementedException();
return NullScope.Instance;
}
}
}