diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index b8714d601f9c..178007ec06ac 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/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 diff --git a/src/Servers/Kestrel/shared/test/HttpParsingData.cs b/src/Servers/Kestrel/shared/test/HttpParsingData.cs index 6b240e18b5c8..a301e27e3877 100644 --- a/src/Servers/Kestrel/shared/test/HttpParsingData.cs +++ b/src/Servers/Kestrel/shared/test/HttpParsingData.cs @@ -497,8 +497,10 @@ public static TheoryData 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 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; } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index d7076dacfd4c..af9d93aea3d6 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/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(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)); }