Always call into next middleware in WebApplicationBuilder (dotnet#34448)

Author: halter73

src/DefaultBuilder/src/WebApplication.cs

@@ -1,11 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
-using System.Collections.Generic;
 using System.Reflection;
-using System.Threading;
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Hosting.Server;
 using Microsoft.AspNetCore.Hosting.Server.Features;
@@ -24,8 +20,6 @@ namespace Microsoft.AspNetCore.Builder
     /// </summary>
     public sealed class WebApplication : IHost, IApplicationBuilder, IEndpointRouteBuilder, IAsyncDisposable
     {
-        internal const string EndpointRouteBuilder = "__EndpointRouteBuilder";
-
         private readonly IHost _host;
         private readonly List<EndpointDataSource> _dataSources = new();
 
@@ -82,15 +76,6 @@ IServiceProvider IApplicationBuilder.ApplicationServices
         internal ICollection<EndpointDataSource> DataSources => _dataSources;
         ICollection<EndpointDataSource> IEndpointRouteBuilder.DataSources => DataSources;
 
-        internal IEndpointRouteBuilder RouteBuilder
-        {
-            get
-            {
-                Properties.TryGetValue(EndpointRouteBuilder, out var value);
-                return (IEndpointRouteBuilder)value!;
-            }
-        }
-
         internal ApplicationBuilder ApplicationBuilder { get; }
 
         IServiceProvider IEndpointRouteBuilder.ServiceProvider => Services;

src/DefaultBuilder/src/WebApplicationBuilder.cs

@@ -17,6 +17,8 @@ namespace Microsoft.AspNetCore.Builder
     /// </summary>
     public sealed class WebApplicationBuilder
     {
+        private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder";
+
         private readonly HostBuilder _hostBuilder = new();
         private readonly ConfigureHostBuilder _deferredHostBuilder;
         private readonly ConfigureWebHostBuilder _deferredWebHostBuilder;
@@ -112,6 +114,8 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui
         {
             Debug.Assert(_builtApplication is not null);
 
+            var implicitRouting = false;
+
             // The endpoints were already added on the outside
             if (_builtApplication.DataSources.Count > 0)
             {
@@ -120,49 +124,49 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui
                 // destination.UseRouting()
                 // destination.Run(source)
                 // destination.UseEndpoints()
-                if (_builtApplication.RouteBuilder == null)
-                {
-                    app.UseRouting();
 
-                    // Copy the route data sources over to the destination pipeline, this should be available since we just called
-                    // UseRouting()
-                    var routes = (IEndpointRouteBuilder)app.Properties[WebApplication.EndpointRouteBuilder]!;
+                // Copy endpoints to the IEndpointRouteBuilder created by an explicit call to UseRouting() if possible.
+                var targetRouteBuilder = GetEndpointRouteBuilder(_builtApplication);
 
-                    foreach (var ds in _builtApplication.DataSources)
-                    {
-                        routes.DataSources.Add(ds);
-                    }
+                if (targetRouteBuilder is null)
+                {
+                    // The app defined endpoints without calling UseRouting() explicitly, so call UseRouting() implicitly.
+                    app.UseRouting();
 
-                    // Chain the execution of the source pipeline into the destination pipeline
-                    app.Use(next =>
-                    {
-                        _builtApplication.Run(next);
-                        return _builtApplication.BuildRequestDelegate();
-                    });
+                    // An implicitly created IEndpointRouteBuilder was addeded to app.Properties by the UseRouting() call above.
+                    targetRouteBuilder = GetEndpointRouteBuilder(app)!;
+                    implicitRouting = true;
+                }
 
-                    // Add a UseEndpoints at the end
-                    app.UseEndpoints(e => { });
+                // Copy the endpoints to the explicitly or implicitly created IEndopintRouteBuilder.
+                foreach (var ds in _builtApplication.DataSources)
+                {
+                    targetRouteBuilder.DataSources.Add(ds);
                 }
-                else
+
+                // UseEndpoints consumes the DataSources immediately to populate CompositeEndpointDataSource via RouteOptions,
+                // so it must be called after we copy the endpoints.
+                if (!implicitRouting)
                 {
-                    // Since we register routes into the source pipeline's route builder directly,
-                    // if the user called UseRouting, we need to copy the data sources
-                    foreach (var ds in _builtApplication.DataSources)
-                    {
-                        _builtApplication.RouteBuilder.DataSources.Add(ds);
-                    }
-
-                    // We then implicitly call UseEndpoints at the end of the pipeline
+                    // UseRouting() was called explicitely, but we may still need to call UseEndpoints() implicitely at
+                    // the end of the pipeline.
                     _builtApplication.UseEndpoints(_ => { });
-
-                    // Wire the source pipeline to run in the destination pipeline
-                    app.Run(_builtApplication.BuildRequestDelegate());
                 }
             }
-            else
+
+            // Wire the source pipeline to run in the destination pipeline
+            app.Use(next =>
+            {
+                _builtApplication.Run(next);
+                return _builtApplication.BuildRequestDelegate();
+            });
+
+            // Implicitly call UseEndpoints() at the end of the pipeline if UseRouting() was called implicitly.
+            // We could add this to the end of _buildApplication instead if UseEndpoints() was not so picky about
+            // being called with the same IApplicationBluilder instance as UseRouting().
+            if (implicitRouting)
             {
-                // Wire the source pipeline to run in the destination pipeline
-                app.Run(_builtApplication.BuildRequestDelegate());
+                app.UseEndpoints(_ => { });
             }
 
             // Copy the properties to the destination app builder
@@ -223,6 +227,12 @@ private void ConfigureWebHost(IWebHostBuilder genericWebHostBuilder)
             _environment.ApplyEnvironmentSettings(genericWebHostBuilder);
         }
 
+        private static IEndpointRouteBuilder? GetEndpointRouteBuilder(IApplicationBuilder app)
+        {
+            app.Properties.TryGetValue(EndpointRouteBuilderKey, out var value);
+            return (IEndpointRouteBuilder?)value;
+        }
+
         private class LoggingBuilder : ILoggingBuilder
         {
             public LoggingBuilder(IServiceCollection services)

src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs

@@ -1,14 +1,8 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Concurrent;
-using System.Collections.Generic;
 using System.Diagnostics.Tracing;
-using System.IO;
-using System.Linq;
-using System.Threading;
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.HostFiltering;
 using Microsoft.AspNetCore.Hosting;
@@ -262,10 +256,7 @@ public async Task WebApplicationConfiguration_EnablesForwardedHeadersFromConfig(
         {
             var builder = WebApplication.CreateBuilder();
             builder.WebHost.UseTestServer();
-            builder.Configuration.AddInMemoryCollection(new[]
-            {
-                new KeyValuePair<string, string>("FORWARDEDHEADERS_ENABLED", "true" ),
-            });
+            builder.Configuration["FORWARDEDHEADERS_ENABLED"] = "true";
             await using var app = builder.Build();
 
             app.Run(context =>
@@ -275,6 +266,7 @@ public async Task WebApplicationConfiguration_EnablesForwardedHeadersFromConfig(
             });
 
             await app.StartAsync();
+
             var client = app.GetTestClient();
             client.DefaultRequestHeaders.Add("x-forwarded-proto", "https");
             var result = await client.GetAsync("http://localhost/");
@@ -335,6 +327,44 @@ public void WebApplication_CanResolveDefaultServicesFromServiceCollectionInCorre
             Assert.Equal(hostLifetimes1.Length, hostLifetimes0.Length);
         }
 
+        [Fact]
+        public async Task WebApplication_CanCallUseRoutingWithouUseEndpoints()
+        {
+            var builder = WebApplication.CreateBuilder();
+            builder.WebHost.UseTestServer();
+            await using var app = builder.Build();
+
+            app.MapGet("/new", () => "new");
+
+            // Rewrite "/old" to "/new" before matching routes
+            app.Use((context, next) =>
+            {
+                if (context.Request.Path == "/old")
+                {
+                    context.Request.Path = "/new";
+                }
+
+                return next(context);
+            });
+
+            app.UseRouting();
+
+            await app.StartAsync();
+
+            var endpointDataSource = app.Services.GetRequiredService<EndpointDataSource>();
+
+            var newEndpoint = Assert.Single(endpointDataSource.Endpoints);
+            var newRouteEndpoint = Assert.IsType<RouteEndpoint>(newEndpoint);
+            Assert.Equal("/new", newRouteEndpoint.RoutePattern.RawText);
+
+            var client = app.GetTestClient();
+
+            var oldResult = await client.GetAsync("http://localhost/old");
+            oldResult.EnsureSuccessStatusCode();
+
+            Assert.Equal("new", await oldResult.Content.ReadAsStringAsync());
+        }
+
         [Fact]
         public void WebApplicationCreate_RegistersEventSourceLogger()
         {
@@ -394,6 +424,27 @@ public void WebApplicationBuilder_CanSetWebRootPaths(bool useSetter)
             Assert.Equal(fullWebRootPath, app.Environment.WebRootPath);
         }
 
+        [Fact]
+        public async Task WebApplicationBuilder_StartupFilterCanAddTerminalMiddleware()
+        {
+            var builder = WebApplication.CreateBuilder();
+            builder.WebHost.UseTestServer();
+            builder.Services.AddSingleton<IStartupFilter, TerminalMiddlewareStartupFilter>();
+            await using var app = builder.Build();
+
+            app.MapGet("/defined", () => { });
+
+            await app.StartAsync();
+
+            var client = app.GetTestClient();
+
+            var definedResult = await client.GetAsync("http://localhost/defined");
+            definedResult.EnsureSuccessStatusCode();
+
+            var terminalResult = await client.GetAsync("http://localhost/undefined");
+            Assert.Equal(418, (int)terminalResult.StatusCode);
+        }
+
         private class CustomHostLifetime : IHostLifetime
         {
             public Task StopAsync(CancellationToken cancellationToken)
@@ -505,5 +556,21 @@ private class MockServerAddressesFeature : IServerAddressesFeature
                 public bool PreferHostingUrls { get; set; }
             }
         }
+
+        private class TerminalMiddlewareStartupFilter : IStartupFilter
+        {
+            public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
+            {
+                return app =>
+                {
+                    next(app);
+                    app.Run(context =>
+                    {
+                        context.Response.StatusCode = 418; // I'm a teapot
+                        return Task.CompletedTask;
+                    });
+                };
+            }
+        }
     }
 }

src/ProjectTemplates/Shared/AspNetProcess.cs

@@ -151,7 +151,7 @@ public async Task ContainsLinks(Page page)
                 IHtmlAnchorElement anchor = (IHtmlAnchorElement)link;
                 if (string.Equals(anchor.Protocol, "about:"))
                 {
-                    Assert.True(anchor.PathName.EndsWith(expectedLink, StringComparison.Ordinal), $"Expected next link on {page.Url} to be {expectedLink} but it was {anchor.PathName}.");
+                    Assert.True(anchor.PathName.EndsWith(expectedLink, StringComparison.Ordinal), $"Expected next link on {page.Url} to be {expectedLink} but it was {anchor.PathName}: {html.Source.Text}");
                     await AssertOk(anchor.PathName);
                 }
                 else