diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index d0105f9bf..733569c51 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Globalization; using System.IO.Pipelines; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; using System.Threading.Tasks; @@ -360,9 +361,9 @@ internal void EnsureHostHeaderExists() // request message that contains more than one Host header field or a // Host header field with an invalid field-value. - var host = HttpRequestHeaders.HeaderHost; - var hostText = host.ToString(); - if (host.Count <= 0) + var hostCount = HttpRequestHeaders.HostCount; + var hostText = HttpRequestHeaders.HeaderHost.ToString(); + if (hostCount <= 0) { if (_httpVersion == Http.HttpVersion.Http10) { @@ -370,13 +371,28 @@ internal void EnsureHostHeaderExists() } BadHttpRequestException.Throw(RequestRejectionReason.MissingHostHeader); } - else if (host.Count > 1) + else if (hostCount > 1) { BadHttpRequestException.Throw(RequestRejectionReason.MultipleHostHeaders); } - else if (_requestTargetForm == HttpRequestTarget.AuthorityForm) + else if (_requestTargetForm != HttpRequestTarget.OriginForm) { - if (!host.Equals(RawTarget)) + // Tail call + ValidateNonOrginHostHeader(hostText); + } + else + { + // Tail call + HttpUtilities.ValidateHostHeader(hostText); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void ValidateNonOrginHostHeader(string hostText) + { + if (_requestTargetForm == HttpRequestTarget.AuthorityForm) + { + if (hostText != RawTarget) { BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } @@ -390,20 +406,18 @@ internal void EnsureHostHeaderExists() // 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 (host != _absoluteRequestTarget.Authority) + if (hostText != _absoluteRequestTarget.Authority) { if (!_absoluteRequestTarget.IsDefaultPort - || host != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture)) + || hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture)) { BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } } } - if (!HttpUtilities.IsValidHostHeader(hostText)) - { - BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); - } + // Tail call + HttpUtilities.ValidateHostHeader(hostText); } protected override void OnReset() @@ -454,8 +468,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio { if (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders) { - BadHttpRequestException.Throw(RequestRejectionReason - .MalformedRequestInvalidHeaders); + BadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders); } throw; } diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 5ac13c69a..f6e66c245 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -213,11 +213,10 @@ public static MessageBody For( // see also http://tools.ietf.org/html/rfc2616#section-4.4 var keepAlive = httpVersion != HttpVersion.Http10; - var connection = headers.HeaderConnection; var upgrade = false; - if (connection.Count > 0) + if (headers.HasConnection) { - var connectionOptions = HttpHeaders.ParseConnection(connection); + var connectionOptions = HttpHeaders.ParseConnection(headers.HeaderConnection); upgrade = (connectionOptions & ConnectionOptions.Upgrade) == ConnectionOptions.Upgrade; keepAlive = (connectionOptions & ConnectionOptions.KeepAlive) == ConnectionOptions.KeepAlive; @@ -233,10 +232,10 @@ public static MessageBody For( return new ForUpgrade(context); } - var transferEncoding = headers.HeaderTransferEncoding; - if (transferEncoding.Count > 0) + if (headers.HasTransferEncoding) { - var transferCoding = HttpHeaders.GetFinalTransferCoding(headers.HeaderTransferEncoding); + var transferEncoding = headers.HeaderTransferEncoding; + var transferCoding = HttpHeaders.GetFinalTransferCoding(transferEncoding); // https://tools.ietf.org/html/rfc7230#section-3.3.3 // If a Transfer-Encoding header field diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs index 27473181b..d84f15706 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs @@ -17,6 +17,11 @@ public partial class HttpRequestHeaders private long _bits = 0; private HeaderReferences _headers; + + public bool HasConnection => (_bits & 2L) != 0; + public bool HasTransferEncoding => (_bits & 64L) != 0; + + public int HostCount => _headers._Host.Count; public StringValues HeaderCacheControl { @@ -4794,6 +4799,12 @@ public partial class HttpResponseHeaders private long _bits = 0; private HeaderReferences _headers; + + public bool HasConnection => (_bits & 2L) != 0; + public bool HasDate => (_bits & 4L) != 0; + public bool HasTransferEncoding => (_bits & 64L) != 0; + public bool HasServer => (_bits & 33554432L) != 0; + public StringValues HeaderCacheControl { diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs index 92061b5d8..444ac6277 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs @@ -45,7 +45,14 @@ StringValues IHeaderDictionary.this[string key] { ThrowHeadersReadOnlyException(); } - SetValueFast(key, value); + if (value.Count == 0) + { + RemoveFast(key); + } + else + { + SetValueFast(key, value); + } } } @@ -164,7 +171,7 @@ void IDictionary.Add(string key, StringValues value) ThrowHeadersReadOnlyException(); } - if (!AddValueFast(key, value)) + if (value.Count > 0 && !AddValueFast(key, value)) { ThrowDuplicateKeyException(); } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 01bde8560..e58b3eebb 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -1110,7 +1110,6 @@ private void CreateResponseHeader(bool appCompleted) var hasConnection = responseHeaders.HasConnection; var connectionOptions = HttpHeaders.ParseConnection(responseHeaders.HeaderConnection); var hasTransferEncoding = responseHeaders.HasTransferEncoding; - var transferCoding = HttpHeaders.GetFinalTransferCoding(responseHeaders.HeaderTransferEncoding); if (_keepAlive && hasConnection && (connectionOptions & ConnectionOptions.KeepAlive) != ConnectionOptions.KeepAlive) { @@ -1122,7 +1121,8 @@ private void CreateResponseHeader(bool appCompleted) // chunked is applied to a response payload body, the sender MUST either // apply chunked as the final transfer coding or terminate the message // by closing the connection. - if (hasTransferEncoding && transferCoding != TransferCoding.Chunked) + if (hasTransferEncoding && + HttpHeaders.GetFinalTransferCoding(responseHeaders.HeaderTransferEncoding) != TransferCoding.Chunked) { _keepAlive = false; } diff --git a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs index f559102c6..1df80f3dc 100644 --- a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs @@ -17,14 +17,6 @@ public partial class HttpResponseHeaders : HttpHeaders private static readonly byte[] _CrLf = new[] { (byte)'\r', (byte)'\n' }; private static readonly byte[] _colonSpace = new[] { (byte)':', (byte)' ' }; - public bool HasConnection => HeaderConnection.Count != 0; - - public bool HasTransferEncoding => HeaderTransferEncoding.Count != 0; - - public bool HasServer => HeaderServer.Count != 0; - - public bool HasDate => HeaderDate.Count != 0; - public Enumerator GetEnumerator() { return new Enumerator(this); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index fd73b5ac9..b1f754884 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -110,10 +110,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio } var hostText = host.ToString(); - if (!HttpUtilities.IsValidHostHeader(hostText)) - { - BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); - } + HttpUtilities.ValidateHostHeader(hostText); endConnection = false; return true; diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs index bfc3baa89..3626835ee 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs @@ -426,45 +426,53 @@ public static string SchemeToString(HttpScheme scheme) } } - public static bool IsValidHostHeader(string hostText) + public static void ValidateHostHeader(string hostText) { - // The spec allows empty values - if (string.IsNullOrEmpty(hostText)) + // This is a string.IsNullOrEmpty test, but arranged to elmininate the + // bounds check from accessing the firstChar of the string + if (hostText is null || 0u >= (uint)hostText.Length) { - return true; + // The spec allows empty values + return; } - if (hostText[0] == '[') + var firstChar = hostText[0]; + if (firstChar == '[') { - return IsValidIPv6Host(hostText); + // Tail call + ValidateIPv6Host(hostText); } - - if (hostText[0] == ':') + else { - // Only a port - return false; - } + if (firstChar == ':') + { + // Only a port + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + } - var i = 0; - for (; i < hostText.Length; i++) - { - if (!IsValidHostChar(hostText[i])) + // Enregister array + var hostCharValidity = HostCharValidity; + var i = 0; + for (; i < hostText.Length; i++) { - break; + var ch = (int)hostText[i]; + // Bounds check and elimiate second bounds check + if ((uint)ch >= (uint)hostCharValidity.Length || !hostCharValidity[ch]) + { + break; + } } - } - return IsValidHostPort(hostText, i); - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsValidHostChar(char ch) - { - return ch < HostCharValidity.Length && HostCharValidity[ch]; + if (i < hostText.Length) + { + // Tail call + ValidateHostPort(hostText, i); + } + } } // The lead '[' was already checked - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsValidIPv6Host(string hostText) + private static void ValidateIPv6Host(string hostText) { for (var i = 1; i < hostText.Length; i++) { @@ -474,58 +482,86 @@ private static bool IsValidIPv6Host(string hostText) // [::1] is the shortest valid IPv6 host if (i < 4) { - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + } + else + { + // Tail call + ValidateHostPort(hostText, i + 1); + return; } - return IsValidHostPort(hostText, i + 1); } if (!IsHex(ch) && ch != ':' && ch != '.') { - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } } // Must contain a ']' - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsValidHostPort(string hostText, int offset) + private static void ValidateHostPort(string hostText, int offset) { - if (offset == hostText.Length) + // Skip bounds check for accessing the [offset] element + if ((uint)offset >= (uint)hostText.Length) { - return true; + return; } - if (hostText[offset] != ':' || hostText.Length == offset + 1) + var firstChar = hostText[offset]; + offset++; + if (firstChar != ':' || (uint)offset >= (uint)hostText.Length) { // Must have at least one number after the colon if present. - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } - for (var i = offset + 1; i < hostText.Length; i++) + // This do+if check rather than for loop is to elimitate the bounds check, since + // the Jit doesn't currently pick up on it when starting at a variable offset + do { - if (!IsNumeric(hostText[i])) + // Elminate bounds check for array access + if ((uint)offset >= (uint)hostText.Length) { - return false; + // Length reached, end of loop + break; } - } - return true; + var ch = hostText[offset]; + offset++; + if (!IsNumeric(ch)) + { + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + } + } while (true); } [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool IsNumeric(char ch) { - return '0' <= ch && ch <= '9'; + // '0' <= ch && ch <= '9' + // (uint)(ch - '0') <= (uint)('9' - '0') + + // Subtract start of range '0' + // Cast to uint to change negative numbers to large numbers + // Check if less than 10 representing chars '0' - '9' + return (uint)(ch - '0') < 10u; } [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool IsHex(char ch) { return IsNumeric(ch) - || ('a' <= ch && ch <= 'f') - || ('A' <= ch && ch <= 'F'); + // || ('a' <= ch && ch <= 'f') + // || ('A' <= ch && ch <= 'F'); + + // Lowercase indiscriminately (or with 32) + // Subtract start of range 'a' + // Cast to uint to change negative numbers to large numbers + // Check if less than 6 representing chars 'a' - 'f' + || (uint)((ch | 32) - 'a') < 6u; } } } diff --git a/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs b/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs index d2c1233ce..b503eab04 100644 --- a/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs +++ b/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs @@ -170,7 +170,8 @@ public static TheoryData HostHeaderData [MemberData(nameof(HostHeaderData))] public void ValidHostHeadersParsed(string host) { - Assert.True(HttpUtilities.IsValidHostHeader(host)); + HttpUtilities.ValidateHostHeader(host); + // Shouldn't throw } public static TheoryData HostHeaderInvalidData @@ -224,7 +225,7 @@ public static TheoryData HostHeaderInvalidData [MemberData(nameof(HostHeaderInvalidData))] public void InvalidHostHeadersRejected(string host) { - Assert.False(HttpUtilities.IsValidHostHeader(host)); + Assert.Throws(() => HttpUtilities.ValidateHostHeader(host)); } } } \ No newline at end of file diff --git a/tools/CodeGenerator/KnownHeaders.cs b/tools/CodeGenerator/KnownHeaders.cs index 6aa165468..08646dc61 100644 --- a/tools/CodeGenerator/KnownHeaders.cs +++ b/tools/CodeGenerator/KnownHeaders.cs @@ -68,6 +68,8 @@ class KnownHeader public byte[] Bytes => Encoding.ASCII.GetBytes($"\r\n{Name}: "); public int BytesOffset { get; set; } public int BytesCount { get; set; } + public bool ExistenceCheck { get; set; } + public bool FastCount { get; set; } public bool EnhancedSetter { get; set; } public bool PrimaryHeader { get; set; } public string TestBit() => $"(_bits & {1L << Index}L) != 0"; @@ -168,6 +170,15 @@ public static string GeneratedFile() "Access-Control-Request-Method", "Access-Control-Request-Headers", }; + var requestHeadersExistence = new[] + { + "Connection", + "Transfer-Encoding", + }; + var requestHeadersCount = new[] + { + "Host" + }; var requestHeaders = commonHeaders.Concat(new[] { "Accept", @@ -197,7 +208,9 @@ public static string GeneratedFile() { Name = header, Index = index, - PrimaryHeader = requestPrimaryHeaders.Contains(header) + PrimaryHeader = requestPrimaryHeaders.Contains(header), + ExistenceCheck = requestHeadersExistence.Contains(header), + FastCount = requestHeadersCount.Contains(header) }) .Concat(new[] { new KnownHeader { @@ -209,6 +222,13 @@ public static string GeneratedFile() Debug.Assert(requestHeaders.Length <= 64); Debug.Assert(requestHeaders.Max(x => x.Index) <= 62); + var responseHeadersExistence = new[] + { + "Connection", + "Server", + "Date", + "Transfer-Encoding" + }; var enhancedHeaders = new[] { "Connection", @@ -245,6 +265,7 @@ public static string GeneratedFile() Name = header, Index = index, EnhancedSetter = enhancedHeaders.Contains(header), + ExistenceCheck = responseHeadersExistence.Contains(header), PrimaryHeader = responsePrimaryHeaders.Contains(header) }) .Concat(new[] { new KnownHeader @@ -311,6 +332,10 @@ public partial class {loop.ClassName} private long _bits = 0; private HeaderReferences _headers; +{Each(loop.Headers.Where(header => header.ExistenceCheck), header => $@" + public bool Has{header.Identifier} => {header.TestBit()};")} +{Each(loop.Headers.Where(header => header.FastCount), header => $@" + public int {header.Identifier}Count => _headers._{header.Identifier}.Count;")} {Each(loop.Headers, header => $@" public StringValues Header{header.Identifier} {{{(header.Identifier == "ContentLength" ? $@"