From 09fda749b07e1ea5f9b9922483a0ed6553867346 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 28 Sep 2016 10:38:15 -0700 Subject: [PATCH] Don't reset frame state when connection is aborted (#1103). --- .../Internal/Http/FrameOfT.cs | 7 +- .../HttpsTests.cs | 67 ++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs index fd8306baf7..203e97152c 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs @@ -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(); } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsTests.cs index 1fd4889a4e..716c71e7b0 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsTests.cs @@ -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(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func 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 state) { - throw new NotImplementedException(); + return NullScope.Instance; } } }