Skip to content

Commit 1b935dd

Browse files
[release/8.0] Fix Kestrel host header mismatch handling when port in Url (#59403)
* Fix Kestrel host header mismatch handling when port in Url * avoid some allocs
1 parent 555dec7 commit 1b935dd

File tree

3 files changed

+25
-10
lines changed

3 files changed

+25
-10
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Buffers;
55
using System.Diagnostics;
6-
using System.Globalization;
76
using System.IO.Pipelines;
87
using Microsoft.AspNetCore.Connections;
98
using Microsoft.AspNetCore.Connections.Features;
@@ -626,16 +625,24 @@ private void ValidateNonOriginHostHeader(string hostText)
626625
// authority component, excluding any userinfo subcomponent and its "@"
627626
// delimiter.
628627

628+
// Accessing authority always allocates, store it in a local to only allocate once
629+
var authority = _absoluteRequestTarget!.Authority;
630+
629631
// System.Uri doesn't not tell us if the port was in the original string or not.
630632
// When IsDefaultPort = true, we will allow Host: with or without the default port
631-
if (hostText != _absoluteRequestTarget!.Authority)
633+
if (hostText != authority)
632634
{
633635
if (!_absoluteRequestTarget.IsDefaultPort
634-
|| hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture))
636+
|| hostText != $"{authority}:{_absoluteRequestTarget.Port}")
635637
{
636638
if (_context.ServiceContext.ServerOptions.AllowHostHeaderOverride)
637639
{
638-
hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture);
640+
// No need to include the port here, it's either already in the Authority
641+
// or it's the default port
642+
// see: https://datatracker.ietf.org/doc/html/rfc2616/#section-14.23
643+
// A "host" without any trailing port information implies the default
644+
// port for the service requested (e.g., "80" for an HTTP URL).
645+
hostText = authority;
639646
HttpRequestHeaders.HeaderHost = hostText;
640647
}
641648
else

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,10 @@ public static TheoryData<string, string> HostHeaderData
497497
{ "GET /pub/WWW/", "www.example.org" },
498498
{ "GET http://localhost/", "localhost" },
499499
{ "GET http://localhost:80/", "localhost:80" },
500+
{ "GET http://localhost:80/", "localhost" },
500501
{ "GET https://localhost/", "localhost" },
501502
{ "GET https://localhost:443/", "localhost:443" },
503+
{ "GET https://localhost:443/", "localhost" },
502504
{ "CONNECT asp.net:80", "asp.net:80" },
503505
{ "CONNECT asp.net:443", "asp.net:443" },
504506
{ "CONNECT user-images.githubusercontent.com:443", "user-images.githubusercontent.com:443" },
@@ -534,10 +536,13 @@ public static TheoryData<string, string> HostHeaderInvalidData
534536
data.Add("CONNECT contoso.com", host);
535537
}
536538

537-
// port mismatch when target contains port
539+
// port mismatch when target contains default https port
538540
data.Add("GET https://contoso.com:443/", "contoso.com:5000");
539541
data.Add("CONNECT contoso.com:443", "contoso.com:5000");
540542

543+
// port mismatch when target contains default http port
544+
data.Add("GET http://contoso.com:80/", "contoso.com:5000");
545+
541546
return data;
542547
}
543548
}

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,12 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget
142142
}
143143

144144
[Theory]
145-
[InlineData("Host: www.foo.comConnection: keep-alive")] // Corrupted - missing line-break
146-
[InlineData("Host: www.notfoo.com")] // Syntactically correct but not matching
147-
public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string hostHeader)
145+
[InlineData("http://www.foo.com", "Host: www.foo.comConnection: keep-alive", "www.foo.com")] // Corrupted - missing line-break
146+
[InlineData("http://www.foo.com/", "Host: www.notfoo.com", "www.foo.com")] // Syntactically correct but not matching
147+
[InlineData("http://www.foo.com:80", "Host: www.notfoo.com", "www.foo.com")] // Explicit default port in request string
148+
[InlineData("http://www.foo.com:5129", "Host: www.foo.com", "www.foo.com:5129")] // Non-default port in request string
149+
[InlineData("http://www.foo.com:5129", "Host: www.foo.com:5128", "www.foo.com:5129")] // Different port in host header
150+
public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestString, string hostHeader, string expectedHost)
148151
{
149152
var receivedHost = StringValues.Empty;
150153
await using var server = new TestServer(context =>
@@ -160,11 +163,11 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(str
160163
});
161164
using var client = server.CreateConnection();
162165

163-
await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\n{hostHeader}\r\n\r\n");
166+
await client.SendAll($"GET {requestString} HTTP/1.1\r\n{hostHeader}\r\n\r\n");
164167

165168
await client.Receive("HTTP/1.1 200 OK");
166169

167-
Assert.Equal("www.foo.com:80", receivedHost);
170+
Assert.Equal(expectedHost, receivedHost);
168171
}
169172

170173
[Fact]

0 commit comments

Comments
 (0)