From c8635b74c6007176df8529f6f8d5d14e809d8af0 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 17 Aug 2021 14:43:20 -0700 Subject: [PATCH 01/14] Update middleware that assumes UseRouting is called after them, for minimal hosting --- .../ExceptionHandlerExtensions.cs | 54 ++++++++++++++++- .../test/UnitTests/ExceptionHandlerTest.cs | 60 +++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 996cd4703ef9..6e54a8873c4b 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -1,9 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; +using System.Diagnostics; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Builder @@ -26,7 +28,7 @@ public static IApplicationBuilder UseExceptionHandler(this IApplicationBuilder a throw new ArgumentNullException(nameof(app)); } - return app.UseMiddleware(); + return SetExceptionHandlerMiddleware(app, options: null); } /// @@ -95,7 +97,53 @@ public static IApplicationBuilder UseExceptionHandler(this IApplicationBuilder a throw new ArgumentNullException(nameof(options)); } - return app.UseMiddleware(Options.Create(options)); + var iOptions = Options.Create(options); + return SetExceptionHandlerMiddleware(app, iOptions); + } + + private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBuilder app, IOptions? options) + { + // Check if UseRouting() has been called so we know if it's safe to call UseRouting() + // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + { + return app.Use(next => + { + var loggerFactory = app.ApplicationServices.GetRequiredService(); + var diagnosticListener = app.ApplicationServices.GetRequiredService(); + + if (options is null) + { + options = app.ApplicationServices.GetRequiredService>(); + } + + if (!string.IsNullOrEmpty(options.Value.ExceptionHandlingPath) && options.Value.ExceptionHandler is null) + { + app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + // start a new middleware pipeline + var builder = app.New(); + if (routeBuilder is not null) + { + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + } + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + // store the pipeline for the error case + options.Value.ExceptionHandler = builder.Build(); + } + + return new ExceptionHandlerMiddleware(next, loggerFactory, options, diagnosticListener).Invoke; + }); + } + + if (options is null) + { + return app.UseMiddleware(); + } + + return app.UseMiddleware(options); } } } diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index 3e65cf7a4a39..226a59677e52 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -657,5 +657,65 @@ public async Task ExceptionHandler_CanReturn404Responses_WhenAllowed() && w.EventId == 4 && w.Message == "No exception handler was found, rethrowing original exception."); } + + [Fact] + public async Task ExceptionHandlerWorksAfterUseRoutingIfGlobalRouteBuilderUsed() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(services => + { + services.AddRouting(); + }) + .UseTestServer() + .Configure(app => + { + app.Use(async (httpContext, next) => + { + Exception exception = null; + try + { + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } + + Assert.Null(exception); + }); + + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + + app.UseExceptionHandler("/handle-errors"); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); + + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync(string.Empty); + response.EnsureSuccessStatusCode(); + Assert.Equal("Handled", await response.Content.ReadAsStringAsync()); + } + } } } From 6c50147b20a5c32c08791c5569f87faf98b68067 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 17 Aug 2021 16:05:44 -0700 Subject: [PATCH 02/14] rewrite --- .../test/UnitTests/ExceptionHandlerTest.cs | 193 +++++++++++++++++- .../src/Microsoft.AspNetCore.Rewrite.csproj | 1 + .../Rewrite/src/RewriteBuilderExtensions.cs | 48 ++++- .../Rewrite/src/RewriteMiddleware.cs | 12 ++ .../Rewrite/test/MiddlewareTests.cs | 92 +++++++++ 5 files changed, 342 insertions(+), 4 deletions(-) diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index 226a59677e52..05dc11b1ba9f 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -659,7 +659,7 @@ public async Task ExceptionHandler_CanReturn404Responses_WhenAllowed() } [Fact] - public async Task ExceptionHandlerWorksAfterUseRoutingIfGlobalRouteBuilderUsed() + public async Task ExceptionHandlerWithPathWorksAfterUseRoutingIfGlobalRouteBuilderUsed() { using var host = new HostBuilder() .ConfigureWebHost(webHostBuilder => @@ -717,5 +717,196 @@ public async Task ExceptionHandlerWorksAfterUseRoutingIfGlobalRouteBuilderUsed() Assert.Equal("Handled", await response.Content.ReadAsStringAsync()); } } + + [Fact] + public async Task ExceptionHandlerWithOptionsWorksAfterUseRoutingIfGlobalRouteBuilderUsed() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(services => + { + services.AddRouting(); + }) + .UseTestServer() + .Configure(app => + { + app.Use(async (httpContext, next) => + { + Exception exception = null; + try + { + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } + + Assert.Null(exception); + }); + + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + + app.UseExceptionHandler(new ExceptionHandlerOptions() + { + ExceptionHandlingPath = "/handle-errors" + }); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); + + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync(string.Empty); + response.EnsureSuccessStatusCode(); + Assert.Equal("Handled", await response.Content.ReadAsStringAsync()); + } + } + + [Fact] + public async Task ExceptionHandlerWithAddWorksAfterUseRoutingIfGlobalRouteBuilderUsed() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(services => + { + services.AddRouting(); + services.AddExceptionHandler(o => o.ExceptionHandlingPath = "/handle-errors"); + }) + .UseTestServer() + .Configure(app => + { + app.Use(async (httpContext, next) => + { + Exception exception = null; + try + { + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } + + Assert.Null(exception); + }); + + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + + app.UseExceptionHandler(); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); + + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync(string.Empty); + response.EnsureSuccessStatusCode(); + Assert.Equal("Handled", await response.Content.ReadAsStringAsync()); + } + } + + [Fact] + public async Task ExceptionHandlerWithExceptionHandlerNotReplacedWithGlobalRouteBuilder() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(services => + { + services.AddRouting(); + }) + .UseTestServer() + .Configure(app => + { + app.Use(async (httpContext, next) => + { + Exception exception = null; + try + { + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } + + Assert.Null(exception); + }); + + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + + app.UseExceptionHandler(new ExceptionHandlerOptions() + { + ExceptionHandler = httpContext => + { + httpContext.Response.StatusCode = StatusCodes.Status404NotFound; + return httpContext.Response.WriteAsync("Custom handler"); + } + }); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); + + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync(string.Empty); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + Assert.Equal("Custom handler", await response.Content.ReadAsStringAsync()); + } + } } } diff --git a/src/Middleware/Rewrite/src/Microsoft.AspNetCore.Rewrite.csproj b/src/Middleware/Rewrite/src/Microsoft.AspNetCore.Rewrite.csproj index 7ea6d0556c2c..f2d2f2233038 100644 --- a/src/Middleware/Rewrite/src/Microsoft.AspNetCore.Rewrite.csproj +++ b/src/Middleware/Rewrite/src/Microsoft.AspNetCore.Rewrite.csproj @@ -20,6 +20,7 @@ + diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 874fc0f68531..3cc10a2546dc 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -1,8 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Rewrite; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Builder @@ -24,7 +26,7 @@ public static IApplicationBuilder UseRewriter(this IApplicationBuilder app) throw new ArgumentNullException(nameof(app)); } - return app.UseMiddleware(); + return AddRewriteMiddleware(app, options: null); } /// @@ -46,7 +48,47 @@ public static IApplicationBuilder UseRewriter(this IApplicationBuilder app, Rewr } // put middleware in pipeline - return app.UseMiddleware(Options.Create(options)); + return AddRewriteMiddleware(app, Options.Create(options)); + } + + private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, IOptions? options) + { + // Check if UseRouting() has been called so we know if it's safe to call UseRouting() + // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + { + return app.Use(next => + { + var loggerFactory = app.ApplicationServices.GetRequiredService(); + var hostEnvironment = app.ApplicationServices.GetRequiredService(); + + if (options is null) + { + options = app.ApplicationServices.GetRequiredService>(); + } + + app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + // start a new middleware pipeline + var builder = app.New(); + builder.UseMiddleware(options); + if (routeBuilder is not null) + { + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + } + builder.UseRouting(); + builder.Run(next); + // apply the next middleware + return builder.Build(); + }); + } + + if (options is null) + { + return app.UseMiddleware(); + } + + return app.UseMiddleware(options); } } } diff --git a/src/Middleware/Rewrite/src/RewriteMiddleware.cs b/src/Middleware/Rewrite/src/RewriteMiddleware.cs index bfb28fc0fe06..858968319775 100644 --- a/src/Middleware/Rewrite/src/RewriteMiddleware.cs +++ b/src/Middleware/Rewrite/src/RewriteMiddleware.cs @@ -73,6 +73,8 @@ public Task Invoke(HttpContext context) Result = RuleResult.ContinueRules }; + var originalPath = context.Request.Path; + foreach (var rule in _options.Rules) { rule.ApplyRule(rewriteContext); @@ -93,6 +95,16 @@ public Task Invoke(HttpContext context) throw new ArgumentOutOfRangeException($"Invalid rule termination {rewriteContext.Result}"); } } + + // If a rule changed the path we want routing to find the new endpoint + if (originalPath != context.Request.Path) + { + if (context.GetEndpoint() is not null) + { + context.SetEndpoint(null); + } + } + return _next(context); } } diff --git a/src/Middleware/Rewrite/test/MiddlewareTests.cs b/src/Middleware/Rewrite/test/MiddlewareTests.cs index cd38a0a00963..cd87ab528006 100644 --- a/src/Middleware/Rewrite/test/MiddlewareTests.cs +++ b/src/Middleware/Rewrite/test/MiddlewareTests.cs @@ -680,5 +680,97 @@ public async Task CheckRedirectToWwwWithStatusCodeInWhitelistedDomains(int statu Assert.Equal(statusCode, (int)response.StatusCode); } + [Theory] + [InlineData("(.*)", "http://example.com/g")] + [InlineData("/", "no rule")] + public async Task Rewrite_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string regex, string output) + { + var options = new RewriteOptions().AddRewrite(regex, "http://example.com/g", skipRemainingRules: false); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .ConfigureServices(services => + { + services.AddRouting(); + }) + .Configure(app => + { + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRewriter(options); + + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/foo", context => context.Response.WriteAsync( + "no rule")); + + endpoints.MapGet("/g", context => context.Response.WriteAsync( + context.Request.Scheme + + "://" + + context.Request.Host + + context.Request.Path + + context.Request.QueryString)); + }); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetStringAsync("foo"); + + Assert.Equal(output, response); + } + + [Theory] + [InlineData("(.*)", "http://example.com/g")] + [InlineData("/", "no rule")] + public async Task RewriteFromOptions_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string regex, string output) + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .ConfigureServices(services => + { + services.AddRouting(); + services.Configure(options => + { + options.AddRewrite(regex, "http://example.com/g", skipRemainingRules: false); + }); + }) + .Configure(app => + { + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRewriter(); + + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/foo", context => context.Response.WriteAsync( + "no rule")); + + endpoints.MapGet("/g", context => context.Response.WriteAsync( + context.Request.Scheme + + "://" + + context.Request.Host + + context.Request.Path + + context.Request.QueryString)); + }); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetStringAsync("foo"); + + Assert.Equal(output, response); + } } } From 6322ff2e2a0e84247b0af73e181e99247e9ec910 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 18 Aug 2021 18:27:39 -0700 Subject: [PATCH 03/14] status code --- .../StatusCodePagesExtensions.cs | 28 +++++++++++- .../test/UnitTests/ExceptionHandlerTest.cs | 42 +++++++++++++++++ .../UnitTests/StatusCodeMiddlewareTest.cs | 45 +++++++++++++++++++ .../Rewrite/src/RewriteBuilderExtensions.cs | 3 -- .../Rewrite/src/RewriteMiddleware.cs | 7 +-- 5 files changed, 115 insertions(+), 10 deletions(-) diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 960574f04bcc..6def4dc8e9a1 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -174,7 +174,7 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( throw new ArgumentNullException(nameof(app)); } - return app.UseStatusCodePages(async context => + Func handler = async context => { var newPath = new PathString( string.Format(CultureInfo.InvariantCulture, pathFormat, context.HttpContext.Response.StatusCode)); @@ -210,7 +210,31 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( context.HttpContext.Request.Path = originalPath; context.HttpContext.Features.Set(null); } - }); + }; + + // Check if UseRouting() has been called so we know if it's safe to call UseRouting() + // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + { + return app.Use(next => + { + app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + // start a new middleware pipeline + var builder = app.New(); + if (routeBuilder is not null) + { + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + } + builder.UseStatusCodePages(handler); + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + return builder.Build(); + }); + } + + return app.UseStatusCodePages(handler); } } } diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index 05dc11b1ba9f..8c746d34df6d 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -658,6 +658,48 @@ public async Task ExceptionHandler_CanReturn404Responses_WhenAllowed() && w.Message == "No exception handler was found, rethrowing original exception."); } + [Fact] + public async Task ExceptionHandlerWithOwnBuilder() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseExceptionHandler(builder => + { + builder.Run(c => + { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("separate pipeline"); + }); + }); + + app.Map("/throw", (innerAppBuilder) => + { + innerAppBuilder.Run(httpContext => + { + throw new InvalidOperationException("Something bad happened."); + }); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync("throw"); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("separate pipeline", await response.Content.ReadAsStringAsync()); + } + } + [Fact] public async Task ExceptionHandlerWithPathWorksAfterUseRoutingIfGlobalRouteBuilderUsed() { diff --git a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs index 1dc5e4912914..e819ba4a4b57 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs @@ -193,5 +193,50 @@ await httpContext.Response.WriteAsync( var content = await response.Content.ReadAsStringAsync(); Assert.Equal($"?id={expectedStatusCode}, /location, ?name=James", content); } + + [Fact] + public async Task Reexecute_WorksAfterUseRoutingWithGlobalRouteBuilder() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(services => + { + services.AddRouting(); + }) + .UseTestServer() + .Configure(app => + { + app.UseRouting(); + app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + + app.UseStatusCodePagesWithReExecute(pathFormat: "/errorPage", queryFormat: "?id={0}"); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/", c => + { + c.Response.StatusCode = 404; + return Task.CompletedTask; + }); + + endpoints.MapGet("/errorPage", () => "errorPage"); + }); + + app.Run((context) => + { + throw new InvalidOperationException("Invalid input provided."); + }); + }); + }).Build(); + + await host.StartAsync(); + + using var server = host.GetTestServer(); + var client = server.CreateClient(); + var response = await client.GetAsync("/"); + var content = await response.Content.ReadAsStringAsync(); + Assert.Equal("errorPage", content); + } } } diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 3cc10a2546dc..d6bfdce84b25 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -59,9 +59,6 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, { return app.Use(next => { - var loggerFactory = app.ApplicationServices.GetRequiredService(); - var hostEnvironment = app.ApplicationServices.GetRequiredService(); - if (options is null) { options = app.ApplicationServices.GetRequiredService>(); diff --git a/src/Middleware/Rewrite/src/RewriteMiddleware.cs b/src/Middleware/Rewrite/src/RewriteMiddleware.cs index 858968319775..cb844daaeb0c 100644 --- a/src/Middleware/Rewrite/src/RewriteMiddleware.cs +++ b/src/Middleware/Rewrite/src/RewriteMiddleware.cs @@ -96,13 +96,10 @@ public Task Invoke(HttpContext context) } } - // If a rule changed the path we want routing to find the new endpoint + // If a rule changed the path we want routing to find a new endpoint if (originalPath != context.Request.Path) { - if (context.GetEndpoint() is not null) - { - context.SetEndpoint(null); - } + context.SetEndpoint(null); } return _next(context); From 83c324d918b262caf6ef61141b8c8748a57e5b87 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 19 Aug 2021 19:24:38 -0700 Subject: [PATCH 04/14] fb --- .../src/ExceptionHandler/ExceptionHandlerExtensions.cs | 7 ++++--- .../src/StatusCodePage/StatusCodePagesExtensions.cs | 7 ++++--- src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 6e54a8873c4b..2eb96ea0947b 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -103,9 +103,10 @@ public static IApplicationBuilder UseExceptionHandler(this IApplicationBuilder a private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBuilder app, IOptions? options) { + const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Check if UseRouting() has been called so we know if it's safe to call UseRouting() // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) { return app.Use(next => { @@ -119,13 +120,13 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui if (!string.IsNullOrEmpty(options.Value.ExceptionHandlingPath) && options.Value.ExceptionHandler is null) { - app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); // start a new middleware pipeline var builder = app.New(); if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic - builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + builder.Properties[globalRouteBuilderKey] = routeBuilder; } builder.UseRouting(); // apply the next middleware diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 6def4dc8e9a1..1b1ead48614e 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -212,19 +212,20 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( } }; + const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Check if UseRouting() has been called so we know if it's safe to call UseRouting() // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) { return app.Use(next => { - app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); // start a new middleware pipeline var builder = app.New(); if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic - builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + builder.Properties[globalRouteBuilderKey] = routeBuilder; } builder.UseStatusCodePages(handler); builder.UseRouting(); diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index d6bfdce84b25..31b8f809f483 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -53,9 +53,10 @@ public static IApplicationBuilder UseRewriter(this IApplicationBuilder app, Rewr private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, IOptions? options) { + const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Check if UseRouting() has been called so we know if it's safe to call UseRouting() // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) { return app.Use(next => { @@ -64,14 +65,14 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, options = app.ApplicationServices.GetRequiredService>(); } - app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var routeBuilder); + app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); // start a new middleware pipeline var builder = app.New(); builder.UseMiddleware(options); if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic - builder.Properties["__GlobalEndpointRouteBuilder"] = routeBuilder; + builder.Properties[globalRouteBuilderKey] = routeBuilder; } builder.UseRouting(); builder.Run(next); From bfd37edf173bd788d82db3d54a5b8093ddd49289 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 09:57:56 -0700 Subject: [PATCH 05/14] fb --- .../ExceptionHandlerExtensions.cs | 3 +- .../StatusCodePagesExtensions.cs | 66 +++++++++++-------- .../Rewrite/src/RewriteBuilderExtensions.cs | 16 +++-- .../Rewrite/src/RewriteMiddleware.cs | 4 ++ src/Middleware/Rewrite/src/RewriteOptions.cs | 3 + 5 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 2eb96ea0947b..68c080cf9506 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -106,7 +106,7 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Check if UseRouting() has been called so we know if it's safe to call UseRouting() // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) { return app.Use(next => { @@ -120,7 +120,6 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui if (!string.IsNullOrEmpty(options.Value.ExceptionHandlingPath) && options.Value.ExceptionHandler is null) { - app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); // start a new middleware pipeline var builder = app.New(); if (routeBuilder is not null) diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 1b1ead48614e..2df6aa6b5dbf 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -174,7 +174,37 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( throw new ArgumentNullException(nameof(app)); } - Func handler = async context => + const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; + // Check if UseRouting() has been called so we know if it's safe to call UseRouting() + // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) + { + return app.Use(next => + { + RequestDelegate? newNext = null; + // start a new middleware pipeline + var builder = app.New(); + if (routeBuilder is not null) + { + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + builder.Properties[globalRouteBuilderKey] = routeBuilder; + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + newNext = builder.Build(); + } + + return new StatusCodePagesMiddleware(next, + Options.Create(new StatusCodePagesOptions() { HandleAsync = CreateHandler(pathFormat, queryFormat, newNext) })).Invoke; + }); + } + + return app.UseStatusCodePages(CreateHandler(pathFormat, queryFormat)); + } + + private static Func CreateHandler(string pathFormat, string? queryFormat, RequestDelegate? next = null) + { + var handler = async (StatusCodeContext context) => { var newPath = new PathString( string.Format(CultureInfo.InvariantCulture, pathFormat, context.HttpContext.Response.StatusCode)); @@ -202,7 +232,14 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( context.HttpContext.Request.QueryString = newQueryString; try { - await context.Next(context.HttpContext); + if (next is not null) + { + await next(context.HttpContext); + } + else + { + await context.Next(context.HttpContext); + } } finally { @@ -212,30 +249,7 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( } }; - const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; - // Check if UseRouting() has been called so we know if it's safe to call UseRouting() - // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) - { - return app.Use(next => - { - app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); - // start a new middleware pipeline - var builder = app.New(); - if (routeBuilder is not null) - { - // use the old routing pipeline if it exists so we preserve all the routes and matching logic - builder.Properties[globalRouteBuilderKey] = routeBuilder; - } - builder.UseStatusCodePages(handler); - builder.UseRouting(); - // apply the next middleware - builder.Run(next); - return builder.Build(); - }); - } - - return app.UseStatusCodePages(handler); + return handler; } } } diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 31b8f809f483..6c3325c98788 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -56,7 +56,7 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Check if UseRouting() has been called so we know if it's safe to call UseRouting() // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out _) || app.Properties.TryGetValue(globalRouteBuilderKey, out _)) + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) { return app.Use(next => { @@ -65,7 +65,9 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, options = app.ApplicationServices.GetRequiredService>(); } - app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder); + var webHostEnv = app.ApplicationServices.GetRequiredService(); + var loggerFactory = app.ApplicationServices.GetRequiredService(); + // start a new middleware pipeline var builder = app.New(); builder.UseMiddleware(options); @@ -73,11 +75,13 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, { // use the old routing pipeline if it exists so we preserve all the routes and matching logic builder.Properties[globalRouteBuilderKey] = routeBuilder; + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + options.Value.BranchedNext = builder.Build(); } - builder.UseRouting(); - builder.Run(next); - // apply the next middleware - return builder.Build(); + + return new RewriteMiddleware(next, webHostEnv, loggerFactory, options).Invoke; }); } diff --git a/src/Middleware/Rewrite/src/RewriteMiddleware.cs b/src/Middleware/Rewrite/src/RewriteMiddleware.cs index cb844daaeb0c..92679572c4e9 100644 --- a/src/Middleware/Rewrite/src/RewriteMiddleware.cs +++ b/src/Middleware/Rewrite/src/RewriteMiddleware.cs @@ -100,6 +100,10 @@ public Task Invoke(HttpContext context) if (originalPath != context.Request.Path) { context.SetEndpoint(null); + if (_options.BranchedNext is not null) + { + return _options.BranchedNext(context); + } } return _next(context); diff --git a/src/Middleware/Rewrite/src/RewriteOptions.cs b/src/Middleware/Rewrite/src/RewriteOptions.cs index 55a043948003..b1fb54fbc00b 100644 --- a/src/Middleware/Rewrite/src/RewriteOptions.cs +++ b/src/Middleware/Rewrite/src/RewriteOptions.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using Microsoft.Extensions.FileProviders; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Rewrite { @@ -24,5 +25,7 @@ public class RewriteOptions /// Defaults to . /// public IFileProvider StaticFileProvider { get; set; } = default!; + + internal RequestDelegate? BranchedNext { get; set; } } } From 1a3fba798898f4490191badcfc06ff074d55948c Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 10:13:15 -0700 Subject: [PATCH 06/14] fb --- .../src/ExceptionHandler/ExceptionHandlerExtensions.cs | 3 +-- .../src/StatusCodePage/StatusCodePagesExtensions.cs | 3 +-- src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 68c080cf9506..8cb082e015ac 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -104,8 +104,7 @@ public static IApplicationBuilder UseExceptionHandler(this IApplicationBuilder a private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBuilder app, IOptions? options) { const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; - // Check if UseRouting() has been called so we know if it's safe to call UseRouting() - // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + // Only use this path if there's a global router (in the 'WebApplication' case). if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) { return app.Use(next => diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 2df6aa6b5dbf..ea7fa9af8796 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -175,8 +175,7 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( } const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; - // Check if UseRouting() has been called so we know if it's safe to call UseRouting() - // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + // Only use this path if there's a global router (in the 'WebApplication' case). if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) { return app.Use(next => diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 6c3325c98788..d60d87adfd3d 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -54,8 +54,7 @@ public static IApplicationBuilder UseRewriter(this IApplicationBuilder app, Rewr private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, IOptions? options) { const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; - // Check if UseRouting() has been called so we know if it's safe to call UseRouting() - // otherwise we might call UseRouting() when AddRouting() hasn't been called which would fail + // Only use this path if there's a global router (in the 'WebApplication' case). if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) { return app.Use(next => From bb671a283dc3cc973d0dcb2136c964bded6bb027 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 13:28:39 -0700 Subject: [PATCH 07/14] changed tests --- .../test/UnitTests/ExceptionHandlerTest.cs | 26 ++++++++++++++++--- .../UnitTests/StatusCodeMiddlewareTest.cs | 17 +++++++++++- .../Rewrite/test/MiddlewareTests.cs | 24 +++++++++++++++-- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index 8c746d34df6d..a418f3a64cd5 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -18,6 +18,7 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Diagnostics @@ -732,7 +733,7 @@ public async Task ExceptionHandlerWithPathWorksAfterUseRoutingIfGlobalRouteBuild app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseExceptionHandler("/handle-errors"); + GetMockWebApplication(app).UseExceptionHandler("/handle-errors"); app.UseEndpoints(endpoints => { @@ -792,7 +793,7 @@ public async Task ExceptionHandlerWithOptionsWorksAfterUseRoutingIfGlobalRouteBu app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseExceptionHandler(new ExceptionHandlerOptions() + GetMockWebApplication(app).UseExceptionHandler(new ExceptionHandlerOptions() { ExceptionHandlingPath = "/handle-errors" }); @@ -856,7 +857,7 @@ public async Task ExceptionHandlerWithAddWorksAfterUseRoutingIfGlobalRouteBuilde app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseExceptionHandler(); + GetMockWebApplication(app).UseExceptionHandler(); app.UseEndpoints(endpoints => { @@ -916,7 +917,7 @@ public async Task ExceptionHandlerWithExceptionHandlerNotReplacedWithGlobalRoute app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseExceptionHandler(new ExceptionHandlerOptions() + GetMockWebApplication(app).UseExceptionHandler(new ExceptionHandlerOptions() { ExceptionHandler = httpContext => { @@ -950,5 +951,22 @@ public async Task ExceptionHandlerWithExceptionHandlerNotReplacedWithGlobalRoute Assert.Equal("Custom handler", await response.Content.ReadAsStringAsync()); } } + + private IApplicationBuilder GetMockWebApplication(IApplicationBuilder builder) + { + var mockApp = new Mock(); + mockApp.Setup(m => m.New()).Returns(() => + { + builder.Properties.Remove("__GlobalEndpointRouteBuilder"); + return builder.New(); + }); + mockApp.Setup(m => m.ApplicationServices).Returns(builder.ApplicationServices); + mockApp.Setup(m => m.Properties).Returns(builder.Properties); + mockApp.Setup(m => m.Build()).Returns(() => builder.Build()); + mockApp.Setup(m => m.Use(It.IsAny>())) + .Returns>((f) => builder.Use(f)); + + return mockApp.Object; + } } } diff --git a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs index e819ba4a4b57..8778a0fc3420 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Diagnostics @@ -211,7 +212,21 @@ public async Task Reexecute_WorksAfterUseRoutingWithGlobalRouteBuilder() app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseStatusCodePagesWithReExecute(pathFormat: "/errorPage", queryFormat: "?id={0}"); + // Mock app builder to simulate WebApplicationBuilders New() method + var mockApp = new Mock(); + mockApp.Setup(m => m.New()).Returns(() => + { + app.Properties.Remove("__GlobalEndpointRouteBuilder"); + return app.New(); + }); + mockApp.Setup(m => m.ApplicationServices).Returns(app.ApplicationServices); + mockApp.Setup(m => m.Properties).Returns(app.Properties); + mockApp.Setup(m => m.Build()).Returns(() => app.Build()); + mockApp.Setup(m => m.Use(It.IsAny>())) + .Returns>((f) => app.Use(f)); + + mockApp.Object.UseStatusCodePagesWithReExecute(pathFormat: "/errorPage", queryFormat: "?id={0}"); + app.UseEndpoints(endpoints => { endpoints.MapGet("/", c => diff --git a/src/Middleware/Rewrite/test/MiddlewareTests.cs b/src/Middleware/Rewrite/test/MiddlewareTests.cs index cd87ab528006..37550dd2ed70 100644 --- a/src/Middleware/Rewrite/test/MiddlewareTests.cs +++ b/src/Middleware/Rewrite/test/MiddlewareTests.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Rewrite.Tests.CodeRules @@ -699,7 +700,8 @@ public async Task Rewrite_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string re { app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseRewriter(options); + + GetMockWebApplication(app).UseRewriter(options); app.UseEndpoints(endpoints => { @@ -747,7 +749,8 @@ public async Task RewriteFromOptions_WorksAfterUseRoutingIfGlobalRouteBuilderUse { app.UseRouting(); app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - app.UseRewriter(); + + GetMockWebApplication(app).UseRewriter(); app.UseEndpoints(endpoints => { @@ -772,5 +775,22 @@ public async Task RewriteFromOptions_WorksAfterUseRoutingIfGlobalRouteBuilderUse Assert.Equal(output, response); } + + private IApplicationBuilder GetMockWebApplication(IApplicationBuilder builder) + { + var mockApp = new Mock(); + mockApp.Setup(m => m.New()).Returns(() => + { + builder.Properties.Remove("__GlobalEndpointRouteBuilder"); + return builder.New(); + }); + mockApp.Setup(m => m.ApplicationServices).Returns(builder.ApplicationServices); + mockApp.Setup(m => m.Properties).Returns(builder.Properties); + mockApp.Setup(m => m.Build()).Returns(() => builder.Build()); + mockApp.Setup(m => m.Use(It.IsAny>())) + .Returns>((f) => builder.Use(f)); + + return mockApp.Object; + } } } From 19ee4f252ab9fd7c9e406860d425a50a2e911252 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 16:21:53 -0700 Subject: [PATCH 08/14] convert to webapplication --- .../test/UnitTests/ExceptionHandlerTest.cs | 327 ++++++++---------- ...rosoft.AspNetCore.Diagnostics.Tests.csproj | 1 + .../UnitTests/StatusCodeMiddlewareTest.cs | 63 ++-- .../Microsoft.AspNetCore.Rewrite.Tests.csproj | 1 + .../Rewrite/test/MiddlewareTests.cs | 122 +++---- 5 files changed, 199 insertions(+), 315 deletions(-) diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index a418f3a64cd5..75f49e4b4b04 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -704,55 +704,45 @@ public async Task ExceptionHandlerWithOwnBuilder() [Fact] public async Task ExceptionHandlerWithPathWorksAfterUseRoutingIfGlobalRouteBuilderUsed() { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.Use(async (httpContext, next) => + { + Exception exception = null; + try { - webHostBuilder - .ConfigureServices(services => - { - services.AddRouting(); - }) - .UseTestServer() - .Configure(app => - { - app.Use(async (httpContext, next) => - { - Exception exception = null; - try - { - await next(httpContext); - } - catch (InvalidOperationException ex) - { - exception = ex; - } + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } - Assert.Null(exception); - }); + Assert.Null(exception); + }); - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRouting(); - GetMockWebApplication(app).UseExceptionHandler("/handle-errors"); + app.UseExceptionHandler("/handle-errors"); - app.UseEndpoints(endpoints => - { - endpoints.Map("/handle-errors", c => { - c.Response.StatusCode = 200; - return c.Response.WriteAsync("Handled"); - }); - }); + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); - app.Run((httpContext) => - { - throw new InvalidOperationException("Something bad happened"); - }); - }); - }).Build(); + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); - await host.StartAsync(); + await app.StartAsync(); - using (var server = host.GetTestServer()) + using (var server = app.GetTestServer()) { var client = server.CreateClient(); var response = await client.GetAsync(string.Empty); @@ -764,58 +754,48 @@ public async Task ExceptionHandlerWithPathWorksAfterUseRoutingIfGlobalRouteBuild [Fact] public async Task ExceptionHandlerWithOptionsWorksAfterUseRoutingIfGlobalRouteBuilderUsed() { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.Use(async (httpContext, next) => + { + Exception exception = null; + try { - webHostBuilder - .ConfigureServices(services => - { - services.AddRouting(); - }) - .UseTestServer() - .Configure(app => - { - app.Use(async (httpContext, next) => - { - Exception exception = null; - try - { - await next(httpContext); - } - catch (InvalidOperationException ex) - { - exception = ex; - } + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } - Assert.Null(exception); - }); + Assert.Null(exception); + }); - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRouting(); - GetMockWebApplication(app).UseExceptionHandler(new ExceptionHandlerOptions() - { - ExceptionHandlingPath = "/handle-errors" - }); + app.UseExceptionHandler(new ExceptionHandlerOptions() + { + ExceptionHandlingPath = "/handle-errors" + }); - app.UseEndpoints(endpoints => - { - endpoints.Map("/handle-errors", c => { - c.Response.StatusCode = 200; - return c.Response.WriteAsync("Handled"); - }); - }); + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); - app.Run((httpContext) => - { - throw new InvalidOperationException("Something bad happened"); - }); - }); - }).Build(); + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); - await host.StartAsync(); + await app.StartAsync(); - using (var server = host.GetTestServer()) + using (var server = app.GetTestServer()) { var client = server.CreateClient(); var response = await client.GetAsync(string.Empty); @@ -827,56 +807,46 @@ public async Task ExceptionHandlerWithOptionsWorksAfterUseRoutingIfGlobalRouteBu [Fact] public async Task ExceptionHandlerWithAddWorksAfterUseRoutingIfGlobalRouteBuilderUsed() { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => + var builder = WebApplication.CreateBuilder(); + builder.Services.AddExceptionHandler(o => o.ExceptionHandlingPath = "/handle-errors"); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.Use(async (httpContext, next) => + { + Exception exception = null; + try { - webHostBuilder - .ConfigureServices(services => - { - services.AddRouting(); - services.AddExceptionHandler(o => o.ExceptionHandlingPath = "/handle-errors"); - }) - .UseTestServer() - .Configure(app => - { - app.Use(async (httpContext, next) => - { - Exception exception = null; - try - { - await next(httpContext); - } - catch (InvalidOperationException ex) - { - exception = ex; - } + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } - Assert.Null(exception); - }); + Assert.Null(exception); + }); - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRouting(); - GetMockWebApplication(app).UseExceptionHandler(); + app.UseExceptionHandler(); - app.UseEndpoints(endpoints => - { - endpoints.Map("/handle-errors", c => { - c.Response.StatusCode = 200; - return c.Response.WriteAsync("Handled"); - }); - }); + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); - app.Run((httpContext) => - { - throw new InvalidOperationException("Something bad happened"); - }); - }); - }).Build(); + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); - await host.StartAsync(); + await app.StartAsync(); - using (var server = host.GetTestServer()) + using (var server = app.GetTestServer()) { var client = server.CreateClient(); var response = await client.GetAsync(string.Empty); @@ -888,62 +858,52 @@ public async Task ExceptionHandlerWithAddWorksAfterUseRoutingIfGlobalRouteBuilde [Fact] public async Task ExceptionHandlerWithExceptionHandlerNotReplacedWithGlobalRouteBuilder() { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.Use(async (httpContext, next) => + { + Exception exception = null; + try { - webHostBuilder - .ConfigureServices(services => - { - services.AddRouting(); - }) - .UseTestServer() - .Configure(app => - { - app.Use(async (httpContext, next) => - { - Exception exception = null; - try - { - await next(httpContext); - } - catch (InvalidOperationException ex) - { - exception = ex; - } + await next(httpContext); + } + catch (InvalidOperationException ex) + { + exception = ex; + } - Assert.Null(exception); - }); + Assert.Null(exception); + }); - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + app.UseRouting(); - GetMockWebApplication(app).UseExceptionHandler(new ExceptionHandlerOptions() - { - ExceptionHandler = httpContext => - { - httpContext.Response.StatusCode = StatusCodes.Status404NotFound; - return httpContext.Response.WriteAsync("Custom handler"); - } - }); + app.UseExceptionHandler(new ExceptionHandlerOptions() + { + ExceptionHandler = httpContext => + { + httpContext.Response.StatusCode = StatusCodes.Status404NotFound; + return httpContext.Response.WriteAsync("Custom handler"); + } + }); - app.UseEndpoints(endpoints => - { - endpoints.Map("/handle-errors", c => { - c.Response.StatusCode = 200; - return c.Response.WriteAsync("Handled"); - }); - }); + app.UseEndpoints(endpoints => + { + endpoints.Map("/handle-errors", c => { + c.Response.StatusCode = 200; + return c.Response.WriteAsync("Handled"); + }); + }); - app.Run((httpContext) => - { - throw new InvalidOperationException("Something bad happened"); - }); - }); - }).Build(); + app.Run((httpContext) => + { + throw new InvalidOperationException("Something bad happened"); + }); - await host.StartAsync(); + await app.StartAsync(); - using (var server = host.GetTestServer()) + using (var server = app.GetTestServer()) { var client = server.CreateClient(); var response = await client.GetAsync(string.Empty); @@ -951,22 +911,5 @@ public async Task ExceptionHandlerWithExceptionHandlerNotReplacedWithGlobalRoute Assert.Equal("Custom handler", await response.Content.ReadAsStringAsync()); } } - - private IApplicationBuilder GetMockWebApplication(IApplicationBuilder builder) - { - var mockApp = new Mock(); - mockApp.Setup(m => m.New()).Returns(() => - { - builder.Properties.Remove("__GlobalEndpointRouteBuilder"); - return builder.New(); - }); - mockApp.Setup(m => m.ApplicationServices).Returns(builder.ApplicationServices); - mockApp.Setup(m => m.Properties).Returns(builder.Properties); - mockApp.Setup(m => m.Build()).Returns(() => builder.Build()); - mockApp.Setup(m => m.Use(It.IsAny>())) - .Returns>((f) => builder.Use(f)); - - return mockApp.Object; - } } } diff --git a/src/Middleware/Diagnostics/test/UnitTests/Microsoft.AspNetCore.Diagnostics.Tests.csproj b/src/Middleware/Diagnostics/test/UnitTests/Microsoft.AspNetCore.Diagnostics.Tests.csproj index c1b33b09a299..8a7999f7eeb0 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/Microsoft.AspNetCore.Diagnostics.Tests.csproj +++ b/src/Middleware/Diagnostics/test/UnitTests/Microsoft.AspNetCore.Diagnostics.Tests.csproj @@ -19,6 +19,7 @@ + diff --git a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs index 8778a0fc3420..cc853db20408 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/StatusCodeMiddlewareTest.cs @@ -198,56 +198,33 @@ await httpContext.Response.WriteAsync( [Fact] public async Task Reexecute_WorksAfterUseRoutingWithGlobalRouteBuilder() { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => - { - webHostBuilder - .ConfigureServices(services => - { - services.AddRouting(); - }) - .UseTestServer() - .Configure(app => - { - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); - // Mock app builder to simulate WebApplicationBuilders New() method - var mockApp = new Mock(); - mockApp.Setup(m => m.New()).Returns(() => - { - app.Properties.Remove("__GlobalEndpointRouteBuilder"); - return app.New(); - }); - mockApp.Setup(m => m.ApplicationServices).Returns(app.ApplicationServices); - mockApp.Setup(m => m.Properties).Returns(app.Properties); - mockApp.Setup(m => m.Build()).Returns(() => app.Build()); - mockApp.Setup(m => m.Use(It.IsAny>())) - .Returns>((f) => app.Use(f)); + app.UseRouting(); - mockApp.Object.UseStatusCodePagesWithReExecute(pathFormat: "/errorPage", queryFormat: "?id={0}"); + app.UseStatusCodePagesWithReExecute(pathFormat: "/errorPage", queryFormat: "?id={0}"); - app.UseEndpoints(endpoints => - { - endpoints.MapGet("/", c => - { - c.Response.StatusCode = 404; - return Task.CompletedTask; - }); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/", c => + { + c.Response.StatusCode = 404; + return Task.CompletedTask; + }); - endpoints.MapGet("/errorPage", () => "errorPage"); - }); + endpoints.MapGet("/errorPage", () => "errorPage"); + }); - app.Run((context) => - { - throw new InvalidOperationException("Invalid input provided."); - }); - }); - }).Build(); + app.Run((context) => + { + throw new InvalidOperationException("Invalid input provided."); + }); - await host.StartAsync(); + await app.StartAsync(); - using var server = host.GetTestServer(); + using var server = app.GetTestServer(); var client = server.CreateClient(); var response = await client.GetAsync("/"); var content = await response.Content.ReadAsStringAsync(); diff --git a/src/Middleware/Rewrite/test/Microsoft.AspNetCore.Rewrite.Tests.csproj b/src/Middleware/Rewrite/test/Microsoft.AspNetCore.Rewrite.Tests.csproj index da3f5187898c..f62c4140b7f1 100644 --- a/src/Middleware/Rewrite/test/Microsoft.AspNetCore.Rewrite.Tests.csproj +++ b/src/Middleware/Rewrite/test/Microsoft.AspNetCore.Rewrite.Tests.csproj @@ -7,6 +7,7 @@ + diff --git a/src/Middleware/Rewrite/test/MiddlewareTests.cs b/src/Middleware/Rewrite/test/MiddlewareTests.cs index 37550dd2ed70..05361411584c 100644 --- a/src/Middleware/Rewrite/test/MiddlewareTests.cs +++ b/src/Middleware/Rewrite/test/MiddlewareTests.cs @@ -687,40 +687,30 @@ public async Task CheckRedirectToWwwWithStatusCodeInWhitelistedDomains(int statu public async Task Rewrite_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string regex, string output) { var options = new RewriteOptions().AddRewrite(regex, "http://example.com/g", skipRemainingRules: false); - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => - { - webHostBuilder - .UseTestServer() - .ConfigureServices(services => - { - services.AddRouting(); - }) - .Configure(app => - { - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); - GetMockWebApplication(app).UseRewriter(options); + app.UseRouting(); - app.UseEndpoints(endpoints => - { - endpoints.MapGet("/foo", context => context.Response.WriteAsync( - "no rule")); + app.UseRewriter(options); - endpoints.MapGet("/g", context => context.Response.WriteAsync( - context.Request.Scheme + - "://" + - context.Request.Host + - context.Request.Path + - context.Request.QueryString)); - }); - }); - }).Build(); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/foo", context => context.Response.WriteAsync( + "no rule")); + + endpoints.MapGet("/g", context => context.Response.WriteAsync( + context.Request.Scheme + + "://" + + context.Request.Host + + context.Request.Path + + context.Request.QueryString)); + }); - await host.StartAsync(); + await app.StartAsync(); - var server = host.GetTestServer(); + var server = app.GetTestServer(); var response = await server.CreateClient().GetStringAsync("foo"); @@ -732,65 +722,37 @@ public async Task Rewrite_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string re [InlineData("/", "no rule")] public async Task RewriteFromOptions_WorksAfterUseRoutingIfGlobalRouteBuilderUsed(string regex, string output) { - using var host = new HostBuilder() - .ConfigureWebHost(webHostBuilder => - { - webHostBuilder - .UseTestServer() - .ConfigureServices(services => - { - services.AddRouting(); - services.Configure(options => - { - options.AddRewrite(regex, "http://example.com/g", skipRemainingRules: false); - }); - }) - .Configure(app => - { - app.UseRouting(); - app.Properties["__GlobalEndpointRouteBuilder"] = app.Properties["__EndpointRouteBuilder"]; - - GetMockWebApplication(app).UseRewriter(); + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + builder.Services.Configure(options => + { + options.AddRewrite(regex, "http://example.com/g", skipRemainingRules: false); + }); + await using var app = builder.Build(); + app.UseRouting(); - app.UseEndpoints(endpoints => - { - endpoints.MapGet("/foo", context => context.Response.WriteAsync( - "no rule")); + app.UseRewriter(); - endpoints.MapGet("/g", context => context.Response.WriteAsync( - context.Request.Scheme + - "://" + - context.Request.Host + - context.Request.Path + - context.Request.QueryString)); - }); - }); - }).Build(); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/foo", context => context.Response.WriteAsync( + "no rule")); + + endpoints.MapGet("/g", context => context.Response.WriteAsync( + context.Request.Scheme + + "://" + + context.Request.Host + + context.Request.Path + + context.Request.QueryString)); + }); - await host.StartAsync(); + await app.StartAsync(); - var server = host.GetTestServer(); + var server = app.GetTestServer(); var response = await server.CreateClient().GetStringAsync("foo"); Assert.Equal(output, response); } - - private IApplicationBuilder GetMockWebApplication(IApplicationBuilder builder) - { - var mockApp = new Mock(); - mockApp.Setup(m => m.New()).Returns(() => - { - builder.Properties.Remove("__GlobalEndpointRouteBuilder"); - return builder.New(); - }); - mockApp.Setup(m => m.ApplicationServices).Returns(builder.ApplicationServices); - mockApp.Setup(m => m.Properties).Returns(builder.Properties); - mockApp.Setup(m => m.Build()).Returns(() => builder.Build()); - mockApp.Setup(m => m.Use(It.IsAny>())) - .Returns>((f) => builder.Use(f)); - - return mockApp.Object; - } } } From 44deaa1cf0ef253cd36f21bc084d76bbc3220f40 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 18:29:41 -0700 Subject: [PATCH 09/14] Apply suggestions from code review Co-authored-by: Stephen Halter --- .../src/ExceptionHandler/ExceptionHandlerExtensions.cs | 1 + .../Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs | 1 + src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 8cb082e015ac..903eaec8b378 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -124,6 +124,7 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. builder.Properties[globalRouteBuilderKey] = routeBuilder; } builder.UseRouting(); diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index ea7fa9af8796..25f15435efaa 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -186,6 +186,7 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. builder.Properties[globalRouteBuilderKey] = routeBuilder; builder.UseRouting(); // apply the next middleware diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index d60d87adfd3d..1cf11950dec7 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -73,6 +73,7 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, if (routeBuilder is not null) { // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. builder.Properties[globalRouteBuilderKey] = routeBuilder; builder.UseRouting(); // apply the next middleware From f5670e163ee6026a503eb3f7de070eecf5065567 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 19:35:26 -0700 Subject: [PATCH 10/14] fb --- .../ExceptionHandlerExtensions.cs | 11 ++++------- .../StatusCodePagesExtensions.cs | 19 ++++++++----------- .../Rewrite/src/RewriteBuilderExtensions.cs | 19 ++++++++----------- src/submodules/googletest | 2 +- src/submodules/spa-templates | 2 +- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs index 903eaec8b378..efc144de55e9 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerExtensions.cs @@ -105,7 +105,7 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui { const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Only use this path if there's a global router (in the 'WebApplication' case). - if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder) && routeBuilder is not null) { return app.Use(next => { @@ -121,12 +121,9 @@ private static IApplicationBuilder SetExceptionHandlerMiddleware(IApplicationBui { // start a new middleware pipeline var builder = app.New(); - if (routeBuilder is not null) - { - // use the old routing pipeline if it exists so we preserve all the routes and matching logic - // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. - builder.Properties[globalRouteBuilderKey] = routeBuilder; - } + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. + builder.Properties[globalRouteBuilderKey] = routeBuilder; builder.UseRouting(); // apply the next middleware builder.Run(next); diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 25f15435efaa..06c4a349c053 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -176,23 +176,20 @@ public static IApplicationBuilder UseStatusCodePagesWithReExecute( const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Only use this path if there's a global router (in the 'WebApplication' case). - if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder) && routeBuilder is not null) { return app.Use(next => { RequestDelegate? newNext = null; // start a new middleware pipeline var builder = app.New(); - if (routeBuilder is not null) - { - // use the old routing pipeline if it exists so we preserve all the routes and matching logic - // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. - builder.Properties[globalRouteBuilderKey] = routeBuilder; - builder.UseRouting(); - // apply the next middleware - builder.Run(next); - newNext = builder.Build(); - } + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. + builder.Properties[globalRouteBuilderKey] = routeBuilder; + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + newNext = builder.Build(); return new StatusCodePagesMiddleware(next, Options.Create(new StatusCodePagesOptions() { HandleAsync = CreateHandler(pathFormat, queryFormat, newNext) })).Invoke; diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 1cf11950dec7..1a03fb165849 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -55,7 +55,7 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, { const string globalRouteBuilderKey = "__GlobalEndpointRouteBuilder"; // Only use this path if there's a global router (in the 'WebApplication' case). - if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder)) + if (app.Properties.TryGetValue(globalRouteBuilderKey, out var routeBuilder) && routeBuilder is not null) { return app.Use(next => { @@ -70,16 +70,13 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, // start a new middleware pipeline var builder = app.New(); builder.UseMiddleware(options); - if (routeBuilder is not null) - { - // use the old routing pipeline if it exists so we preserve all the routes and matching logic - // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. - builder.Properties[globalRouteBuilderKey] = routeBuilder; - builder.UseRouting(); - // apply the next middleware - builder.Run(next); - options.Value.BranchedNext = builder.Build(); - } + // use the old routing pipeline if it exists so we preserve all the routes and matching logic + // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. + builder.Properties[globalRouteBuilderKey] = routeBuilder; + builder.UseRouting(); + // apply the next middleware + builder.Run(next); + options.Value.BranchedNext = builder.Build(); return new RewriteMiddleware(next, webHostEnv, loggerFactory, options).Invoke; }); diff --git a/src/submodules/googletest b/src/submodules/googletest index 0134d73a4902..8d51ffdfab10 160000 --- a/src/submodules/googletest +++ b/src/submodules/googletest @@ -1 +1 @@ -Subproject commit 0134d73a4902574269ff2e42827f7573d3df08ae +Subproject commit 8d51ffdfab10b3fba636ae69bc03da4b54f8c235 diff --git a/src/submodules/spa-templates b/src/submodules/spa-templates index 373712a22074..7370afad2138 160000 --- a/src/submodules/spa-templates +++ b/src/submodules/spa-templates @@ -1 +1 @@ -Subproject commit 373712a22074c9cec3461dc5322300ccfc229ef7 +Subproject commit 7370afad2138b3919bca1abdb365f8b1e760a83f From 1dc8c0257c422f1786f3e2648104ab62dd2b660b Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Aug 2021 20:31:17 -0700 Subject: [PATCH 11/14] revert submodule --- src/submodules/googletest | 2 +- src/submodules/spa-templates | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/submodules/googletest b/src/submodules/googletest index 8d51ffdfab10..2f80c2ba71c0 160000 --- a/src/submodules/googletest +++ b/src/submodules/googletest @@ -1 +1 @@ -Subproject commit 8d51ffdfab10b3fba636ae69bc03da4b54f8c235 +Subproject commit 2f80c2ba71c0e8922a03b9b855e5b019ad1f7064 diff --git a/src/submodules/spa-templates b/src/submodules/spa-templates index 7370afad2138..dc078f5b8efa 160000 --- a/src/submodules/spa-templates +++ b/src/submodules/spa-templates @@ -1 +1 @@ -Subproject commit 7370afad2138b3919bca1abdb365f8b1e760a83f +Subproject commit dc078f5b8efa3a2e6d92c40c8838326f7a1da940 From de2ba874be07b808518f2d0c00f1ecedf0ffd952 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 24 Aug 2021 08:24:48 -0700 Subject: [PATCH 12/14] remove extra middleware call --- src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs index 1a03fb165849..d7149ffcc8bd 100644 --- a/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs +++ b/src/Middleware/Rewrite/src/RewriteBuilderExtensions.cs @@ -69,7 +69,6 @@ private static IApplicationBuilder AddRewriteMiddleware(IApplicationBuilder app, // start a new middleware pipeline var builder = app.New(); - builder.UseMiddleware(options); // use the old routing pipeline if it exists so we preserve all the routes and matching logic // ((IApplicationBuilder)WebApplication).New() does not copy globalRouteBuilderKey automatically like it does for all other properties. builder.Properties[globalRouteBuilderKey] = routeBuilder; From a13a27b142013ccd40e0ab90069d467ec714642a Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 24 Aug 2021 09:16:54 -0700 Subject: [PATCH 13/14] userouter --- src/Middleware/Rewrite/src/RewriteMiddleware.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Middleware/Rewrite/src/RewriteMiddleware.cs b/src/Middleware/Rewrite/src/RewriteMiddleware.cs index 92679572c4e9..3cd2be91632c 100644 --- a/src/Middleware/Rewrite/src/RewriteMiddleware.cs +++ b/src/Middleware/Rewrite/src/RewriteMiddleware.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Rewrite.Logging; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; @@ -99,7 +100,14 @@ public Task Invoke(HttpContext context) // If a rule changed the path we want routing to find a new endpoint if (originalPath != context.Request.Path) { - context.SetEndpoint(null); + // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset + // the endpoint and route values to ensure things are re-calculated. + context.SetEndpoint(endpoint: null); + var routeValuesFeature = context.Features.Get(); + if (routeValuesFeature is not null) + { + routeValuesFeature.RouteValues = null!; + } if (_options.BranchedNext is not null) { return _options.BranchedNext(context); From 4ba89f20238c76ec15d1f0ccb850f665d4979591 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 24 Aug 2021 11:10:23 -0700 Subject: [PATCH 14/14] undefined --- .../Rewrite/src/RewriteMiddleware.cs | 16 ++++---- .../Rewrite/test/MiddlewareTests.cs | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/Middleware/Rewrite/src/RewriteMiddleware.cs b/src/Middleware/Rewrite/src/RewriteMiddleware.cs index 3cd2be91632c..1600788df06d 100644 --- a/src/Middleware/Rewrite/src/RewriteMiddleware.cs +++ b/src/Middleware/Rewrite/src/RewriteMiddleware.cs @@ -100,16 +100,16 @@ public Task Invoke(HttpContext context) // If a rule changed the path we want routing to find a new endpoint if (originalPath != context.Request.Path) { - // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset - // the endpoint and route values to ensure things are re-calculated. - context.SetEndpoint(endpoint: null); - var routeValuesFeature = context.Features.Get(); - if (routeValuesFeature is not null) - { - routeValuesFeature.RouteValues = null!; - } if (_options.BranchedNext is not null) { + // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset + // the endpoint and route values to ensure things are re-calculated. + context.SetEndpoint(endpoint: null); + var routeValuesFeature = context.Features.Get(); + if (routeValuesFeature is not null) + { + routeValuesFeature.RouteValues = null!; + } return _options.BranchedNext(context); } } diff --git a/src/Middleware/Rewrite/test/MiddlewareTests.cs b/src/Middleware/Rewrite/test/MiddlewareTests.cs index 05361411584c..d27715620ef7 100644 --- a/src/Middleware/Rewrite/test/MiddlewareTests.cs +++ b/src/Middleware/Rewrite/test/MiddlewareTests.cs @@ -514,6 +514,43 @@ public async Task CheckIfEmptyStringRedirectCorrectly() Assert.Equal("/", response.Headers.Location.OriginalString); } + [Fact] + public async Task RewriteAfterUseRoutingHitsOriginalEndpoint() + { + // This is an edge case where users setup routing incorrectly, but we don't want to accidentally change behavior in case someone + // relies on it, so we have this test + var options = new RewriteOptions().AddRewrite("(.*)", "$1s", skipRemainingRules: false); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .ConfigureServices(s => + { + s.AddRouting(); + }) + .Configure(app => + { + app.UseRouting(); + app.UseRewriter(options); + + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/foos", context => context.Response.WriteAsync("bad")); + endpoints.MapGet("/foo", context => context.Response.WriteAsync($"{context.GetEndpoint()?.DisplayName} from {context.Request.Path}")); + }); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetStringAsync("foo"); + + Assert.Equal("/foo HTTP: GET from /foos", response); + } + [Fact] public async Task CheckIfEmptyStringRewriteCorrectly() {