From 307e020703a8bf697bf4b8ed463c53dedd8391b1 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 18 Nov 2015 16:32:22 -0800 Subject: [PATCH] Don't crash the server if a connection filter throws synchronously. --- .../Http/Connection.cs | 42 +++++++++++-------- .../ConnectionFilterTests.cs | 40 +++++++++++++++++- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Connection.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Connection.cs index c2ff59be01..4af0676761 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Connection.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Connection.cs @@ -78,25 +78,33 @@ namespace Microsoft.AspNet.Server.Kestrel.Http Address = ServerAddress }; - ConnectionFilter.OnConnection(_filterContext).ContinueWith((task, state) => + try { - var connection = (Connection)state; + ConnectionFilter.OnConnection(_filterContext).ContinueWith((task, state) => + { + var connection = (Connection)state; - if (task.IsFaulted) - { - connection.Log.LogError("ConnectionFilter.OnConnection", task.Exception); - connection.ConnectionControl.End(ProduceEndType.SocketDisconnect); - } - else if (task.IsCanceled) - { - connection.Log.LogError("ConnectionFilter.OnConnection Canceled"); - connection.ConnectionControl.End(ProduceEndType.SocketDisconnect); - } - else - { - connection.ApplyConnectionFilter(); - } - }, this); + if (task.IsFaulted) + { + connection.Log.LogError("ConnectionFilter.OnConnection", task.Exception); + connection.ConnectionControl.End(ProduceEndType.SocketDisconnect); + } + else if (task.IsCanceled) + { + connection.Log.LogError("ConnectionFilter.OnConnection Canceled"); + connection.ConnectionControl.End(ProduceEndType.SocketDisconnect); + } + else + { + connection.ApplyConnectionFilter(); + } + }, this); + } + catch (Exception ex) + { + Log.LogError("ConnectionFilter.OnConnection", ex); + ConnectionControl.End(ProduceEndType.SocketDisconnect); + } } } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/ConnectionFilterTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/ConnectionFilterTests.cs index 5842e1d7c0..5dbe530f14 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/ConnectionFilterTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/ConnectionFilterTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.Threading.Tasks; using Microsoft.AspNet.Http; @@ -80,7 +81,36 @@ namespace Microsoft.AspNet.Server.KestrelTests "Hello World!"); } } - } + } + + [ConditionalFact] + [FrameworkSkipCondition(RuntimeFrameworks.Mono, SkipReason = "Test hangs after execution on Mono.")] + public async Task ThrowingSynchronousConnectionFilterDoesNotCrashServer() + { + var serviceContext = new TestServiceContext() + { + ConnectionFilter = new ThrowingConnectionFilter() + }; + + using (var server = new TestServer(App, serviceContext)) + { + using (var connection = new TestConnection()) + { + try + { + await connection.SendEnd( + "POST / HTTP/1.0", + "", + "Hello World?"); + } + catch (IOException) + { + // Will throw because the exception in the connection filter will close the connection. + Assert.True(true); + } + } + } + } private class RewritingConnectionFilter : IConnectionFilter { @@ -112,6 +142,14 @@ namespace Microsoft.AspNet.Server.KestrelTests } } + private class ThrowingConnectionFilter : IConnectionFilter + { + public Task OnConnection(ConnectionFilterContext context) + { + throw new Exception(); + } + } + private class RewritingStream : Stream { private readonly Stream _innerStream;