diff --git a/src/Microsoft.AspNetCore.Diagnostics.HealthChecks/Builder/HealthCheckApplicationBuilderExtensions.cs b/src/Microsoft.AspNetCore.Diagnostics.HealthChecks/Builder/HealthCheckApplicationBuilderExtensions.cs index 6463889f..6f54f517 100644 --- a/src/Microsoft.AspNetCore.Diagnostics.HealthChecks/Builder/HealthCheckApplicationBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Diagnostics.HealthChecks/Builder/HealthCheckApplicationBuilderExtensions.cs @@ -21,8 +21,10 @@ public static class HealthCheckApplicationBuilderExtensions /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests. If is set to a non-empty + /// value, the health check middleware will process requests with a URL that matches the provided value + /// of case-insensitively, allowing for an extra trailing slash ('/') character. /// /// /// The health check middleware will use default settings from . @@ -48,8 +50,10 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests. If is set to a non-empty + /// value, the health check middleware will process requests with a URL that matches the provided value + /// of case-insensitively, allowing for an extra trailing slash ('/') character. /// /// public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, PathString path, HealthCheckOptions options) @@ -77,8 +81,11 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path and port. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests on the specified port. If is + /// set to a non-empty value, the health check middleware will process requests with a URL that matches the + /// provided value of case-insensitively, allowing for an extra trailing slash ('/') + /// character. /// /// /// The health check middleware will use default settings from . @@ -104,8 +111,11 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path and port. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests on the specified port. If is + /// set to a non-empty value, the health check middleware will process requests with a URL that matches the + /// provided value of case-insensitively, allowing for an extra trailing slash ('/') + /// character. /// /// /// The health check middleware will use default settings from . @@ -142,8 +152,11 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests on the specified port. If is + /// set to a non-empty value, the health check middleware will process requests with a URL that matches the + /// provided value of case-insensitively, allowing for an extra trailing slash ('/') + /// character. /// /// public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, PathString path, int port, HealthCheckOptions options) @@ -172,8 +185,11 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, /// A reference to the after the operation has completed. /// /// - /// This method will use to - /// listen to health checks requests on the specified URL path. + /// If is set to null or the empty string then the health check middleware + /// will ignore the URL path and process all requests on the specified port. If is + /// set to a non-empty value, the health check middleware will process requests with a URL that matches the + /// provided value of case-insensitively, allowing for an extra trailing slash ('/') + /// character. /// /// public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, PathString path, string port, HealthCheckOptions options) @@ -204,16 +220,35 @@ public static IApplicationBuilder UseHealthChecks(this IApplicationBuilder app, private static void UseHealthChecksCore(IApplicationBuilder app, PathString path, int? port, object[] args) { - if (port == null) - { - app.Map(path, b => b.UseMiddleware(args)); - } - else + // NOTE: we explicitly don't use Map here because it's really common for multiple health + // check middleware to overlap in paths. Ex: `/health`, `/health/detailed` - this is order + // sensititive with Map, and it's really surprising to people. + // + // See: + // https://github.com/aspnet/Diagnostics/issues/511 + // https://github.com/aspnet/Diagnostics/issues/512 + // https://github.com/aspnet/Diagnostics/issues/514 + + Func predicate = c => { - app.MapWhen( - c => c.Connection.LocalPort == port, - b0 => b0.Map(path, b1 => b1.UseMiddleware(args))); - } + return + + // Process the port if we have one + (port == null || c.Connection.LocalPort == port) && + + // We allow you to listen on all URLs by providing the empty PathString. + (!path.HasValue || + + // If you do provide a PathString, want to handle all of the special cases that + // StartsWithSegments handles, but we also want it to have exact match semantics. + // + // Ex: /Foo/ == /Foo (true) + // Ex: /Foo/Bar == /Foo (false) + (c.Request.Path.StartsWithSegments(path, out var remaining) && + string.IsNullOrEmpty(remaining))); + }; + + app.MapWhen(predicate, b => b.UseMiddleware(args)); } } } diff --git a/test/Microsoft.AspNetCore.Diagnostics.HealthChecks.Tests/HealthCheckMiddlewareTests.cs b/test/Microsoft.AspNetCore.Diagnostics.HealthChecks.Tests/HealthCheckMiddlewareTests.cs index 4715a36d..7dab1cc2 100644 --- a/test/Microsoft.AspNetCore.Diagnostics.HealthChecks.Tests/HealthCheckMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.Diagnostics.HealthChecks.Tests/HealthCheckMiddlewareTests.cs @@ -395,6 +395,131 @@ public async Task CanListenWithoutPath_AcceptsRequest() Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); } + [Fact] + public async Task CanListenWithPath_AcceptsRequestWithExtraSlash() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseHealthChecks("/health"); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health/"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task CanListenWithPath_AcceptsRequestWithCaseInsensitiveMatch() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseHealthChecks("/health"); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/HEALTH"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task CanListenWithPath_RejectsRequestWithExtraSegments() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseHealthChecks("/health"); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health/detailed"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + // See: https://github.com/aspnet/Diagnostics/issues/511 + [Fact] + public async Task CanListenWithPath_MultipleMiddleware_LeastSpecificFirst() + { + var builder = new WebHostBuilder() + .Configure(app => + { + // Throws if used + app.UseHealthChecks("/health", new HealthCheckOptions() + { + ResponseWriter = (c, r) => throw null, + }); + + app.UseHealthChecks("/health/detailed"); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health/detailed"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); + } + + // See: https://github.com/aspnet/Diagnostics/issues/511 + [Fact] + public async Task CanListenWithPath_MultipleMiddleware_MostSpecificFirst() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseHealthChecks("/health/detailed"); + + // Throws if used + app.UseHealthChecks("/health", new HealthCheckOptions() + { + ResponseWriter = (c, r) => throw null, + }); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health/detailed"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); + } + [Fact] public async Task CanListenOnPort_AcceptsRequest_OnSpecifiedPort() { @@ -486,6 +611,78 @@ public async Task CanListenOnPort_RejectsRequest_OnOtherPort() Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Fact] + public async Task CanListenOnPort_MultipleMiddleware() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.Use(next => async (context) => + { + // Need to fake setting the connection info. TestServer doesn't + // do that, because it doesn't have a connection. + context.Connection.LocalPort = context.Request.Host.Port.Value; + await next(context); + }); + + // Throws if used + app.UseHealthChecks("/health", port: 5001, new HealthCheckOptions() + { + ResponseWriter = (c, r) => throw null, + }); + + app.UseHealthChecks("/health/detailed", port: 5001); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health/detailed"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task CanListenOnPort_MultipleMiddleware_DifferentPorts() + { + var builder = new WebHostBuilder() + .Configure(app => + { + app.Use(next => async (context) => + { + // Need to fake setting the connection info. TestServer doesn't + // do that, because it doesn't have a connection. + context.Connection.LocalPort = context.Request.Host.Port.Value; + await next(context); + }); + + // Throws if used + app.UseHealthChecks("/health", port: 5002, new HealthCheckOptions() + { + ResponseWriter = (c, r) => throw null, + }); + app.UseHealthChecks("/health", port: 5001); + }) + .ConfigureServices(services => + { + services.AddHealthChecks(); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var response = await client.GetAsync("http://localhost:5001/health"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + Assert.Equal("Healthy", await response.Content.ReadAsStringAsync()); + } } }