Fix Kestrel host header mismatch handling when port in Url (#59352)

Author: BrennanConroy

src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs

@@ -3,7 +3,6 @@
 
 using System.Buffers;
 using System.Diagnostics;
-using System.Globalization;
 using System.IO.Pipelines;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Connections.Features;
@@ -644,16 +643,24 @@ private void ValidateNonOriginHostHeader(string hostText)
             // authority component, excluding any userinfo subcomponent and its "@"
             // delimiter.
 
+            // Accessing authority always allocates, store it in a local to only allocate once
+            var authority = _absoluteRequestTarget!.Authority;
+
             // System.Uri doesn't not tell us if the port was in the original string or not.
             // When IsDefaultPort = true, we will allow Host: with or without the default port
-            if (hostText != _absoluteRequestTarget!.Authority)
+            if (hostText != authority)
             {
                 if (!_absoluteRequestTarget.IsDefaultPort
-                    || hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture))
+                    || hostText != $"{authority}:{_absoluteRequestTarget.Port}")
                 {
                     if (_context.ServiceContext.ServerOptions.AllowHostHeaderOverride)
                     {
-                        hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture);
+                        // No need to include the port here, it's either already in the Authority
+                        // or it's the default port
+                        // see: https://datatracker.ietf.org/doc/html/rfc2616/#section-14.23
+                        // A "host" without any trailing port information implies the default
+                        // port for the service requested (e.g., "80" for an HTTP URL).
+                        hostText = authority;
                         HttpRequestHeaders.HeaderHost = hostText;
                     }
                     else

src/Servers/Kestrel/shared/test/HttpParsingData.cs

@@ -497,8 +497,10 @@ public static TheoryData<string, string> HostHeaderData
                     { "GET /pub/WWW/", "www.example.org" },
                     { "GET http://localhost/", "localhost" },
                     { "GET http://localhost:80/", "localhost:80" },
+                    { "GET http://localhost:80/", "localhost" },
                     { "GET https://localhost/", "localhost" },
                     { "GET https://localhost:443/", "localhost:443" },
+                    { "GET https://localhost:443/", "localhost" },
                     { "CONNECT asp.net:80", "asp.net:80" },
                     { "CONNECT asp.net:443", "asp.net:443" },
                     { "CONNECT user-images.githubusercontent.com:443", "user-images.githubusercontent.com:443" },
@@ -534,10 +536,13 @@ public static TheoryData<string, string> HostHeaderInvalidData
                 data.Add("CONNECT contoso.com", host);
             }
 
-            // port mismatch when target contains port
+            // port mismatch when target contains default https port
             data.Add("GET https://contoso.com:443/", "contoso.com:5000");
             data.Add("CONNECT contoso.com:443", "contoso.com:5000");
 
+            // port mismatch when target contains default http port
+            data.Add("GET http://contoso.com:80/", "contoso.com:5000");
+
             return data;
         }
     }

src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs

@@ -153,9 +153,12 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget
     }
 
     [Theory]
-    [InlineData("Host: www.foo.comConnection: keep-alive")] // Corrupted - missing line-break
-    [InlineData("Host: www.notfoo.com")] // Syntactically correct but not matching
-    public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string hostHeader)
+    [InlineData("http://www.foo.com", "Host: www.foo.comConnection: keep-alive", "www.foo.com")] // Corrupted - missing line-break
+    [InlineData("http://www.foo.com/", "Host: www.notfoo.com", "www.foo.com")] // Syntactically correct but not matching
+    [InlineData("http://www.foo.com:80", "Host: www.notfoo.com", "www.foo.com")] // Explicit default port in request string
+    [InlineData("http://www.foo.com:5129", "Host: www.foo.com", "www.foo.com:5129")] // Non-default port in request string
+    [InlineData("http://www.foo.com:5129", "Host: www.foo.com:5128", "www.foo.com:5129")] // Different port in host header
+    public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestString, string hostHeader, string expectedHost)
     {
         var testMeterFactory = new TestMeterFactory();
         using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
@@ -175,13 +178,13 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(str
         {
             using (var client = server.CreateConnection())
             {
-                await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\n{hostHeader}\r\n\r\n");
+                await client.SendAll($"GET {requestString} HTTP/1.1\r\n{hostHeader}\r\n\r\n");
 
                 await client.Receive("HTTP/1.1 200 OK");
             }
         }
 
-        Assert.Equal("www.foo.com:80", receivedHost);
+        Assert.Equal(expectedHost, receivedHost);
 
         Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m => MetricsAssert.NoError(m.Tags));
     }