From a890df472415f8a6c9f2ecdf6c414539ad47687a Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Wed, 8 Aug 2018 14:42:15 +0300 Subject: [PATCH 1/6] general idea of fix for https://github.com/aspnet/AspNetWebStack/issues/147 --- src/System.Web.Cors/CorsPolicy.cs | 5 +++++ src/System.Web.Http.Cors/CorsMessageHandler.cs | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/System.Web.Cors/CorsPolicy.cs b/src/System.Web.Cors/CorsPolicy.cs index fa532a55c..efdfd8605 100644 --- a/src/System.Web.Cors/CorsPolicy.cs +++ b/src/System.Web.Cors/CorsPolicy.cs @@ -85,6 +85,11 @@ public long? PreflightMaxAge /// public bool SupportsCredentials { get; set; } + /// + /// Gets or sets a value indicating whether upstream exceptions should be rethrown. + /// + public bool RethrowExceptions { get; set; } + /// /// Returns a that represents this instance. /// diff --git a/src/System.Web.Http.Cors/CorsMessageHandler.cs b/src/System.Web.Http.Cors/CorsMessageHandler.cs index 5d683ca26..ececbd0cf 100644 --- a/src/System.Web.Http.Cors/CorsMessageHandler.cs +++ b/src/System.Web.Http.Cors/CorsMessageHandler.cs @@ -47,6 +47,7 @@ protected async override Task SendAsync(HttpRequestMessage CorsRequestContext corsRequestContext = request.GetCorsRequestContext(); if (corsRequestContext != null) { + CorsPolicy corsPolicy = await GetCorsPolicyAsync(request, cancellationToken); try { if (corsRequestContext.IsPreflight) @@ -60,6 +61,9 @@ protected async override Task SendAsync(HttpRequestMessage } catch (Exception exception) { + if (corsPolicy.RethrowExceptions) + throw; + return HandleException(request, exception); } } From 1b5e6a49d22b76aa06437d62c6d473770d204ede Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Wed, 8 Aug 2018 15:50:57 +0300 Subject: [PATCH 2/6] build fix --- src/System.Web.Http.Cors/CorsMessageHandler.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/System.Web.Http.Cors/CorsMessageHandler.cs b/src/System.Web.Http.Cors/CorsMessageHandler.cs index ececbd0cf..55916cc36 100644 --- a/src/System.Web.Http.Cors/CorsMessageHandler.cs +++ b/src/System.Web.Http.Cors/CorsMessageHandler.cs @@ -47,21 +47,25 @@ protected async override Task SendAsync(HttpRequestMessage CorsRequestContext corsRequestContext = request.GetCorsRequestContext(); if (corsRequestContext != null) { - CorsPolicy corsPolicy = await GetCorsPolicyAsync(request, cancellationToken); + CorsPolicy corsPolicy = null; try { + HttpResponseMessage responseMessage; if (corsRequestContext.IsPreflight) { - return await HandleCorsPreflightRequestAsync(request, corsRequestContext, cancellationToken); + responseMessage = await HandleCorsPreflightRequestAsync(request, corsRequestContext, cancellationToken); } else { - return await HandleCorsRequestAsync(request, corsRequestContext, cancellationToken); + responseMessage = await HandleCorsRequestAsync(request, corsRequestContext, cancellationToken); } + + corsPolicy = await GetCorsPolicyAsync(request, cancellationToken); + return responseMessage; } catch (Exception exception) { - if (corsPolicy.RethrowExceptions) + if (corsPolicy != null && corsPolicy.RethrowExceptions) throw; return HandleException(request, exception); From 83d5629c13769ff7cd0f22be91b9f5d141a4c046 Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Wed, 8 Aug 2018 16:22:49 +0300 Subject: [PATCH 3/6] passing rethrow flag through ctor, not throught CorsPolicy --- src/System.Web.Cors/CorsPolicy.cs | 5 ----- .../CorsHttpConfigurationExtensions.cs | 15 +++++++++------ src/System.Web.Http.Cors/CorsMessageHandler.cs | 16 +++++++--------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/System.Web.Cors/CorsPolicy.cs b/src/System.Web.Cors/CorsPolicy.cs index efdfd8605..fa532a55c 100644 --- a/src/System.Web.Cors/CorsPolicy.cs +++ b/src/System.Web.Cors/CorsPolicy.cs @@ -85,11 +85,6 @@ public long? PreflightMaxAge /// public bool SupportsCredentials { get; set; } - /// - /// Gets or sets a value indicating whether upstream exceptions should be rethrown. - /// - public bool RethrowExceptions { get; set; } - /// /// Returns a that represents this instance. /// diff --git a/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs b/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs index 3bf6631b5..cdd7fd7c8 100644 --- a/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs +++ b/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs @@ -24,9 +24,10 @@ public static class CorsHttpConfigurationExtensions /// Enables the support for CORS. /// /// The . - public static void EnableCors(this HttpConfiguration httpConfiguration) + /// Indicates whether upstream exceptions should be rethrown + public static void EnableCors(this HttpConfiguration httpConfiguration, bool rethrowExceptions = false) { - EnableCors(httpConfiguration, null); + EnableCors(httpConfiguration, null, rethrowExceptions); } /// @@ -34,8 +35,10 @@ public static void EnableCors(this HttpConfiguration httpConfiguration) /// /// The . /// The default . + /// Indicates whether upstream exceptions should be rethrown /// httpConfiguration - public static void EnableCors(this HttpConfiguration httpConfiguration, ICorsPolicyProvider defaultPolicyProvider) + public static void EnableCors(this HttpConfiguration httpConfiguration, ICorsPolicyProvider defaultPolicyProvider, + bool rethrowExceptions = false) { if (httpConfiguration == null) { @@ -49,11 +52,11 @@ public static void EnableCors(this HttpConfiguration httpConfiguration, ICorsPol httpConfiguration.SetCorsPolicyProviderFactory(policyProviderFactory); } - AddCorsMessageHandler(httpConfiguration); + AddCorsMessageHandler(httpConfiguration, rethrowExceptions); } [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Caller owns the disposable object")] - private static void AddCorsMessageHandler(this HttpConfiguration httpConfiguration) + private static void AddCorsMessageHandler(this HttpConfiguration httpConfiguration, bool rethrowExceptions) { object corsEnabled; if (!httpConfiguration.Properties.TryGetValue(CorsEnabledKey, out corsEnabled)) @@ -64,7 +67,7 @@ private static void AddCorsMessageHandler(this HttpConfiguration httpConfigurati if (!config.Properties.TryGetValue(CorsEnabledKey, out corsEnabled)) { // Execute this in the Initializer to ensure that the CorsMessageHandler is added last. - config.MessageHandlers.Add(new CorsMessageHandler(config)); + config.MessageHandlers.Add(new CorsMessageHandler(config, rethrowExceptions)); ITraceWriter traceWriter = config.Services.GetTraceWriter(); diff --git a/src/System.Web.Http.Cors/CorsMessageHandler.cs b/src/System.Web.Http.Cors/CorsMessageHandler.cs index 55916cc36..23ba094a9 100644 --- a/src/System.Web.Http.Cors/CorsMessageHandler.cs +++ b/src/System.Web.Http.Cors/CorsMessageHandler.cs @@ -18,13 +18,15 @@ namespace System.Web.Http.Cors public class CorsMessageHandler : DelegatingHandler { private HttpConfiguration _httpConfiguration; + private bool _rethrowExceptions; /// /// Initializes a new instance of the class. /// /// The . + /// Indicates whether upstream exceptions should be rethrown /// httpConfiguration - public CorsMessageHandler(HttpConfiguration httpConfiguration) + public CorsMessageHandler(HttpConfiguration httpConfiguration, bool rethrowExceptions = false) { if (httpConfiguration == null) { @@ -32,6 +34,7 @@ public CorsMessageHandler(HttpConfiguration httpConfiguration) } _httpConfiguration = httpConfiguration; + _rethrowExceptions = rethrowExceptions; } /// @@ -47,25 +50,20 @@ protected async override Task SendAsync(HttpRequestMessage CorsRequestContext corsRequestContext = request.GetCorsRequestContext(); if (corsRequestContext != null) { - CorsPolicy corsPolicy = null; try { - HttpResponseMessage responseMessage; if (corsRequestContext.IsPreflight) { - responseMessage = await HandleCorsPreflightRequestAsync(request, corsRequestContext, cancellationToken); + return await HandleCorsPreflightRequestAsync(request, corsRequestContext, cancellationToken); } else { - responseMessage = await HandleCorsRequestAsync(request, corsRequestContext, cancellationToken); + return await HandleCorsRequestAsync(request, corsRequestContext, cancellationToken); } - - corsPolicy = await GetCorsPolicyAsync(request, cancellationToken); - return responseMessage; } catch (Exception exception) { - if (corsPolicy != null && corsPolicy.RethrowExceptions) + if (_rethrowExceptions) throw; return HandleException(request, exception); From cc39a81d01cbd25a8f239c15602b5093ebe93d33 Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Wed, 8 Aug 2018 16:42:44 +0300 Subject: [PATCH 4/6] StyleCop fixes --- .../CorsHttpConfigurationExtensions.cs | 23 +++++++++++++++++-- .../CorsMessageHandler.cs | 11 ++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs b/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs index cdd7fd7c8..42190f97b 100644 --- a/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs +++ b/src/System.Web.Http.Cors/CorsHttpConfigurationExtensions.cs @@ -20,16 +20,35 @@ public static class CorsHttpConfigurationExtensions private const string CorsPolicyProviderFactoryKey = "MS_CorsPolicyProviderFactoryKey"; private const string CorsEnabledKey = "MS_CorsEnabledKey"; + /// + /// Enables the support for CORS. + /// + /// The . + public static void EnableCors(this HttpConfiguration httpConfiguration) + { + EnableCors(httpConfiguration, null, false); + } + /// /// Enables the support for CORS. /// /// The . /// Indicates whether upstream exceptions should be rethrown - public static void EnableCors(this HttpConfiguration httpConfiguration, bool rethrowExceptions = false) + public static void EnableCors(this HttpConfiguration httpConfiguration, bool rethrowExceptions) { EnableCors(httpConfiguration, null, rethrowExceptions); } + /// + /// Enables the support for CORS. + /// + /// The . + /// The default . + public static void EnableCors(this HttpConfiguration httpConfiguration, ICorsPolicyProvider defaultPolicyProvider) + { + EnableCors(httpConfiguration, defaultPolicyProvider, false); + } + /// /// Enables the support for CORS. /// @@ -38,7 +57,7 @@ public static void EnableCors(this HttpConfiguration httpConfiguration, bool ret /// Indicates whether upstream exceptions should be rethrown /// httpConfiguration public static void EnableCors(this HttpConfiguration httpConfiguration, ICorsPolicyProvider defaultPolicyProvider, - bool rethrowExceptions = false) + bool rethrowExceptions) { if (httpConfiguration == null) { diff --git a/src/System.Web.Http.Cors/CorsMessageHandler.cs b/src/System.Web.Http.Cors/CorsMessageHandler.cs index 23ba094a9..1cbcd7eff 100644 --- a/src/System.Web.Http.Cors/CorsMessageHandler.cs +++ b/src/System.Web.Http.Cors/CorsMessageHandler.cs @@ -20,13 +20,22 @@ public class CorsMessageHandler : DelegatingHandler private HttpConfiguration _httpConfiguration; private bool _rethrowExceptions; + /// + /// Initializes a new instance of the class. + /// + /// The . + /// httpConfiguration + public CorsMessageHandler(HttpConfiguration httpConfiguration) : this(httpConfiguration, false) + { + } + /// /// Initializes a new instance of the class. /// /// The . /// Indicates whether upstream exceptions should be rethrown /// httpConfiguration - public CorsMessageHandler(HttpConfiguration httpConfiguration, bool rethrowExceptions = false) + public CorsMessageHandler(HttpConfiguration httpConfiguration, bool rethrowExceptions) { if (httpConfiguration == null) { From 08a202b95fa941c0f1e1bf7ddd70c79ca5422b40 Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Thu, 16 Aug 2018 22:37:47 +0300 Subject: [PATCH 5/6] review fixes; added tests --- .../CorsMessageHandler.cs | 2 + .../Controllers/ThrowingController.cs | 11 +++++ .../CorsMessageHandlerTest.cs | 43 +++++++++++++++++++ .../System.Web.Http.Cors.Test.csproj | 1 + 4 files changed, 57 insertions(+) create mode 100644 test/System.Web.Http.Cors.Test/Controllers/ThrowingController.cs diff --git a/src/System.Web.Http.Cors/CorsMessageHandler.cs b/src/System.Web.Http.Cors/CorsMessageHandler.cs index 1cbcd7eff..5308ee340 100644 --- a/src/System.Web.Http.Cors/CorsMessageHandler.cs +++ b/src/System.Web.Http.Cors/CorsMessageHandler.cs @@ -73,7 +73,9 @@ protected async override Task SendAsync(HttpRequestMessage catch (Exception exception) { if (_rethrowExceptions) + { throw; + } return HandleException(request, exception); } diff --git a/test/System.Web.Http.Cors.Test/Controllers/ThrowingController.cs b/test/System.Web.Http.Cors.Test/Controllers/ThrowingController.cs new file mode 100644 index 000000000..6879816c2 --- /dev/null +++ b/test/System.Web.Http.Cors.Test/Controllers/ThrowingController.cs @@ -0,0 +1,11 @@ +namespace System.Web.Http.Cors +{ + [EnableCors("*", "*", "*")] + public class ThrowingController : ApiController + { + public string Get() + { + throw new Exception(); + } + } +} diff --git a/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs b/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs index e3fe7cf49..842a809d6 100644 --- a/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs +++ b/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using System.Web.Cors; +using System.Web.Http.ExceptionHandling; using System.Web.Http.Hosting; using Microsoft.TestCommon; @@ -14,6 +15,14 @@ namespace System.Web.Http.Cors { public class CorsMessageHandlerTest { + private class PassthroughExceptionHandler : IExceptionHandler + { + public Task HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken) + { + throw context.Exception; + } + } + [Fact] public void Constructor_NullConfig_Throws() { @@ -180,6 +189,40 @@ public async Task SendAsync_HandlesExceptions_ThrownDuringPreflight() Assert.Equal(HttpStatusCode.MethodNotAllowed, response.StatusCode); } + [Fact] + public async Task SendAsync_Preflight_RethrowsExceptions_WhenRethrowFlagIsTrue() + { + HttpConfiguration config = new HttpConfiguration(); + config.Routes.MapHttpRoute("default", "{controller}"); + HttpServer server = new HttpServer(config); + CorsMessageHandler corsHandler = new CorsMessageHandler(config, true); + corsHandler.InnerHandler = server; + HttpMessageInvoker invoker = new HttpMessageInvoker(corsHandler); + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Options, "http://localhost/sample"); + request.SetConfiguration(config); + request.Headers.Add(CorsConstants.Origin, "http://localhost"); + request.Headers.Add(CorsConstants.AccessControlRequestMethod, "RandomMethod"); + + await Assert.ThrowsAsync(() => invoker.SendAsync(request, CancellationToken.None)); + } + + [Fact] + public async Task SendAsync_RethrowsExceptions_WhenRethrowFlagIsTrue() + { + HttpConfiguration config = new HttpConfiguration(); + config.Routes.MapHttpRoute("default", "{controller}"); + config.Services.Replace(typeof(IExceptionHandler), new PassthroughExceptionHandler()); + HttpServer server = new HttpServer(config); + CorsMessageHandler corsHandler = new CorsMessageHandler(config, true); + corsHandler.InnerHandler = server; + HttpMessageInvoker invoker = new HttpMessageInvoker(corsHandler); + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/throwing"); + request.SetConfiguration(config); + request.Headers.Add(CorsConstants.Origin, "http://localhost"); + + await Assert.ThrowsAsync(() => invoker.SendAsync(request, CancellationToken.None)); + } + [Fact] public Task HandleCorsRequestAsync_NullConfig_Throws() { diff --git a/test/System.Web.Http.Cors.Test/System.Web.Http.Cors.Test.csproj b/test/System.Web.Http.Cors.Test/System.Web.Http.Cors.Test.csproj index ac5963fcd..9454da007 100644 --- a/test/System.Web.Http.Cors.Test/System.Web.Http.Cors.Test.csproj +++ b/test/System.Web.Http.Cors.Test/System.Web.Http.Cors.Test.csproj @@ -82,6 +82,7 @@ + From e0257cd61fd84a2043736b3cc2f0096207523830 Mon Sep 17 00:00:00 2001 From: Trofimov Ivan Andreevich Date: Thu, 16 Aug 2018 22:52:24 +0300 Subject: [PATCH 6/6] review fixes --- .../CorsMessageHandlerTest.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs b/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs index 842a809d6..c6ecfdf25 100644 --- a/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs +++ b/test/System.Web.Http.Cors.Test/CorsMessageHandlerTest.cs @@ -15,14 +15,6 @@ namespace System.Web.Http.Cors { public class CorsMessageHandlerTest { - private class PassthroughExceptionHandler : IExceptionHandler - { - public Task HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken) - { - throw context.Exception; - } - } - [Fact] public void Constructor_NullConfig_Throws() { @@ -281,5 +273,13 @@ public Task HandleCorsPreflightRequestAsync_NullContext_Throws() () => corsHandler.HandleCorsPreflightRequestAsync(new HttpRequestMessage(), null, CancellationToken.None), "corsRequestContext"); } + + private class PassthroughExceptionHandler : IExceptionHandler + { + public Task HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken) + { + throw context.Exception; + } + } } } \ No newline at end of file