Improve HostFiltering perf (#9359)

Author: benaadams

src/Http/Http.Abstractions/src/HostString.cs

@@ -247,7 +247,8 @@ public static bool MatchesAny(StringSegment value, IList<StringSegment> patterns
                 }
             }
 
-            for (int i = 0; i < patterns.Count; i++)
+            var count = patterns.Count;
+            for (int i = 0; i < count; i++)
             {
                 var pattern = patterns[i];
 

src/Http/Http/src/HeaderDictionary.cs

@@ -74,7 +74,7 @@ public StringValues this[string key]
                 }
                 ThrowIfReadOnly();
 
-                if (StringValues.IsNullOrEmpty(value))
+                if (value.Count == 0)
                 {
                     Store?.Remove(key);
                 }

src/Middleware/HostFiltering/src/HostFilteringMiddleware.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Text;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
@@ -68,30 +69,42 @@ public Task Invoke(HttpContext context)
 
             if (!CheckHost(context, allowedHosts))
             {
-                context.Response.StatusCode = 400;
-                if (_options.IncludeFailureMessage)
-                {
-                    context.Response.ContentLength = DefaultResponse.Length;
-                    context.Response.ContentType = "text/html";
-                    return context.Response.Body.WriteAsync(DefaultResponse, 0, DefaultResponse.Length);
-                }
-                return Task.CompletedTask;
+                return HostValidationFailed(context);
             }
 
             return _next(context);
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private IList<StringSegment> EnsureConfigured()
         {
             if (_allowAnyNonEmptyHost == true || _allowedHosts?.Count > 0)
             {
                 return _allowedHosts;
             }
 
+            return Configure();
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private Task HostValidationFailed(HttpContext context)
+        {
+            context.Response.StatusCode = 400;
+            if (_options.IncludeFailureMessage)
+            {
+                context.Response.ContentLength = DefaultResponse.Length;
+                context.Response.ContentType = "text/html";
+                return context.Response.Body.WriteAsync(DefaultResponse, 0, DefaultResponse.Length);
+            }
+            return Task.CompletedTask;
+        }
+
+        private IList<StringSegment> Configure()
+        {
             var allowedHosts = new List<StringSegment>();
             if (_options.AllowedHosts?.Count > 0 && !TryProcessHosts(_options.AllowedHosts, allowedHosts))
             {
-                _logger.LogDebug("Wildcard detected, all requests with hosts will be allowed.");
+                _logger.WildcardDetected();
                 _allowedHosts = allowedHosts;
                 _allowAnyNonEmptyHost = true;
                 return _allowedHosts;
@@ -104,7 +117,7 @@ private IList<StringSegment> EnsureConfigured()
 
             if (_logger.IsEnabled(LogLevel.Debug))
             {
-                _logger.LogDebug("Allowed hosts: {Hosts}", string.Join("; ", allowedHosts));
+                _logger.AllowedHosts(string.Join("; ", allowedHosts));
             }
 
             _allowedHosts = allowedHosts;
@@ -142,37 +155,50 @@ private bool IsTopLevelWildcard(string host)
         }
 
         // This does not duplicate format validations that are expected to be performed by the host.
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private bool CheckHost(HttpContext context, IList<StringSegment> allowedHosts)
         {
-            var host = new StringSegment(context.Request.Headers[HeaderNames.Host].ToString()).Trim();
+            var host = context.Request.Headers[HeaderNames.Host].ToString();
 
-            if (StringSegment.IsNullOrEmpty(host))
+            if (host.Length == 0)
             {
-                // Http/1.0 does not require the host header.
-                // Http/1.1 requires the header but the value may be empty.
-                if (!_options.AllowEmptyHosts)
-                {
-                    _logger.LogInformation("{Protocol} request rejected due to missing or empty host header.", context.Request.Protocol);
-                    return false;
-                }
-                _logger.LogDebug("{Protocol} request allowed with missing or empty host header.", context.Request.Protocol);
-                return true;
+                return IsEmptyHostAllowed(context);
             }
 
             if (_allowAnyNonEmptyHost == true)
             {
-                _logger.LogTrace("All hosts are allowed.");
+                _logger.AllHostsAllowed();
                 return true;
             }
 
-            if (HostString.MatchesAny(host, allowedHosts))
+            return CheckHostInAllowList(allowedHosts, host);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private bool CheckHostInAllowList(IList<StringSegment> allowedHosts, string host)
+        {
+            if (HostString.MatchesAny(new StringSegment(host), allowedHosts))
             {
-                _logger.LogTrace("The host '{Host}' matches an allowed host.", host);
+                _logger.AllowedHostMatched(host);
                 return true;
             }
-            
-            _logger.LogInformation("The host '{Host}' does not match an allowed host.", host);
+
+            _logger.NoAllowedHostMatched(host);
             return false;
         }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private bool IsEmptyHostAllowed(HttpContext context)
+        {
+            // Http/1.0 does not require the host header.
+            // Http/1.1 requires the header but the value may be empty.
+            if (!_options.AllowEmptyHosts)
+            {
+                _logger.RequestRejectedMissingHost(context.Request.Protocol);
+                return false;
+            }
+            _logger.RequestAllowedMissingHost(context.Request.Protocol);
+            return true;
+        }
     }
 }

src/Middleware/HostFiltering/src/LoggerExtensions.cs

@@ -0,0 +1,40 @@
+// 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 Microsoft.Extensions.Logging;
+
+namespace Microsoft.AspNetCore.HostFiltering
+{
+    internal static class LoggerExtensions
+    {
+        private static readonly Action<ILogger, Exception> _wildcardDetected =
+            LoggerMessage.Define(LogLevel.Debug, new EventId(0, nameof(WildcardDetected)), "Wildcard detected, all requests with hosts will be allowed.");
+
+        private static readonly Action<ILogger, string, Exception> _allowedHosts =
+            LoggerMessage.Define<string>(LogLevel.Debug, new EventId(1, nameof(AllowedHosts)), "Allowed hosts: {Hosts}");
+
+        private static readonly Action<ILogger, Exception> _allHostsAllowed =
+            LoggerMessage.Define(LogLevel.Trace, new EventId(2, nameof(AllHostsAllowed)), "All hosts are allowed.");
+
+        private static readonly Action<ILogger, string, Exception> _requestRejectedMissingHost =
+            LoggerMessage.Define<string>(LogLevel.Information, new EventId(3, nameof(RequestRejectedMissingHost)), "{Protocol} request rejected due to missing or empty host header.");
+
+        private static readonly Action<ILogger, string, Exception> _requestAllowedMissingHost =
+            LoggerMessage.Define<string>(LogLevel.Debug, new EventId(4, nameof(RequestAllowedMissingHost)), "{Protocol} request allowed with missing or empty host header.");
+
+        private static readonly Action<ILogger, string, Exception> _allowedHostMatched =
+            LoggerMessage.Define<string>(LogLevel.Trace, new EventId(5, nameof(AllowedHostMatched)), "The host '{Host}' matches an allowed host.");
+
+        private static readonly Action<ILogger, string, Exception> _noAllowedHostMatched =
+            LoggerMessage.Define<string>(LogLevel.Information, new EventId(6, nameof(NoAllowedHostMatched)), "The host '{Host}' does not match an allowed host.");
+
+        public static void WildcardDetected(this ILogger logger) => _wildcardDetected(logger, null);
+        public static void AllowedHosts(this ILogger logger, string allowedHosts) => _allowedHosts(logger, allowedHosts, null);
+        public static void AllHostsAllowed(this ILogger logger) => _allHostsAllowed(logger, null);
+        public static void RequestRejectedMissingHost(this ILogger logger, string protocol) => _requestRejectedMissingHost(logger, protocol, null);
+        public static void RequestAllowedMissingHost(this ILogger logger, string protocol) => _requestAllowedMissingHost(logger, protocol, null);
+        public static void AllowedHostMatched(this ILogger logger, string host) => _allowedHostMatched(logger, host, null);
+        public static void NoAllowedHostMatched(this ILogger logger, string host) => _noAllowedHostMatched(logger, host, null);
+    }
+}

src/Middleware/HostFiltering/test/HostFilteringMiddlewareTests.cs

@@ -80,14 +80,14 @@ public async Task AllowsEmptyHost(bool allowed, int status)
                 {
                     app.Use((ctx, next) =>
                     {
-                        ctx.Request.Headers[HeaderNames.Host] = " ";
+                        ctx.Request.Headers[HeaderNames.Host] = "";
                         return next();
                     });
                     app.UseHostFiltering();
                     app.Run(c =>
                     {
                         Assert.True(c.Request.Headers.TryGetValue(HeaderNames.Host, out var host));
-                        Assert.True(StringValues.Equals(" ", host));
+                        Assert.True(StringValues.Equals("", host));
                         return Task.CompletedTask;
                     });
                     app.Run(c => Task.CompletedTask);

src/Mvc/Mvc.Core/src/ConsumesAttribute.cs

@@ -79,7 +79,7 @@ public void OnResourceExecuting(ResourceExecutingContext context)
                 //
                 // Requests without a content type do not return a 415. It is a common pattern to place [Consumes] on
                 // a controller and have GET actions
-                if (requestContentType != null && !IsSubsetOfAnyContentType(requestContentType))
+                if (!string.IsNullOrEmpty(requestContentType) && !IsSubsetOfAnyContentType(requestContentType))
                 {
                     context.Result = new UnsupportedMediaTypeResult();
                 }
@@ -127,7 +127,7 @@ public bool Accept(ActionConstraintContext context)
             // In case there is a single candidate with a constraint it should be selected.
             // If there are multiple actions with consumes action constraints this should result in ambiguous exception
             // unless there is another action without a consumes constraint.
-            if (requestContentType == null)
+            if (string.IsNullOrEmpty(requestContentType))
             {
                 var isActionWithoutConsumeConstraintPresent = context.Candidates.Any(
                     candidate => candidate.Constraints == null ||

src/Mvc/Mvc.Core/src/Formatters/TextInputFormatter.cs

@@ -91,7 +91,7 @@ protected Encoding SelectCharacterEncoding(InputFormatterContext context)
             }
 
             var requestContentType = context.HttpContext.Request.ContentType;
-            var requestMediaType = requestContentType == null ? default(MediaType) : new MediaType(requestContentType);
+            var requestMediaType = string.IsNullOrEmpty(requestContentType) ? default : new MediaType(requestContentType);
             if (requestMediaType.Charset.HasValue)
             {
                 // Create Encoding based on requestMediaType.Charset to support charset aliases and custom Encoding