From e30a02cee5ff4919a3be82bf53bab6bb5146e7de Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Sat, 14 Apr 2018 02:21:50 +0100 Subject: [PATCH] Less StringValue struct copies for header checks (#2488) --- .../Internal/Http/Http1Connection.cs | 39 ++++--- .../Internal/Http/Http1MessageBody.cs | 11 +- .../Internal/Http/HttpHeaders.Generated.cs | 11 ++ src/Kestrel.Core/Internal/Http/HttpHeaders.cs | 11 +- .../Internal/Http/HttpProtocol.cs | 4 +- .../Internal/Http/HttpResponseHeaders.cs | 8 -- .../Internal/Http2/Http2Stream.cs | 5 +- .../Internal/Infrastructure/HttpUtilities.cs | 100 ++++++++++-------- test/Kestrel.Core.Tests/HttpUtilitiesTest.cs | 5 +- tools/CodeGenerator/KnownHeaders.cs | 27 ++++- 10 files changed, 137 insertions(+), 84 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index c6215fb1a4..6fd58d08b0 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -360,9 +360,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // 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 +370,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } 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); + } + } + + private void ValidateNonOrginHostHeader(string hostText) + { + if (_requestTargetForm == HttpRequestTarget.AuthorityForm) + { + if (hostText != RawTarget) { BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } @@ -390,20 +404,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // 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 +466,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { 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 6f82e27dd0..90c05f53ee 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -213,11 +213,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // 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 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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 27473181b7..d84f15706d 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs @@ -17,6 +17,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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 92061b5d87..444ac62774 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs @@ -45,7 +45,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { ThrowHeadersReadOnlyException(); } - SetValueFast(key, value); + if (value.Count == 0) + { + RemoveFast(key); + } + else + { + SetValueFast(key, value); + } } } @@ -164,7 +171,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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 da1416c1a3..93cb96499d 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -1111,7 +1111,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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) { @@ -1123,7 +1122,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // 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 f559102c6a..1df80f3dc6 100644 --- a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs @@ -17,14 +17,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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 fd73b5ac98..b1f754884a 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -110,10 +110,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } 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 bfc3baa89b..0af41644ff 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs @@ -426,45 +426,44 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } } - public static bool IsValidHostHeader(string hostText) + public static void ValidateHostHeader(string hostText) { - // The spec allows empty values if (string.IsNullOrEmpty(hostText)) { - 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; - } - - var i = 0; - for (; i < hostText.Length; i++) - { - if (!IsValidHostChar(hostText[i])) + if (firstChar == ':') { - break; + // Only a port + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + } + + // Enregister array + var hostCharValidity = HostCharValidity; + for (var i = 0; i < hostText.Length; i++) + { + if (!hostCharValidity[hostText[i]]) + { + // Tail call + ValidateHostPort(hostText, i); + return; + } } } - return IsValidHostPort(hostText, i); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsValidHostChar(char ch) - { - return ch < HostCharValidity.Length && HostCharValidity[ch]; } // 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 +473,69 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure // [::1] is the shortest valid IPv6 host if (i < 4) { - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } - return IsValidHostPort(hostText, i + 1); + else if (i + 1 < hostText.Length) + { + // Tail call + ValidateHostPort(hostText, i + 1); + } + return; } 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) - { - return true; - } - - if (hostText[offset] != ':' || hostText.Length == offset + 1) + var firstChar = hostText[offset]; + offset++; + if (firstChar != ':' || offset == 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++) + for (var i = offset; i < hostText.Length; i++) { if (!IsNumeric(hostText[i])) { - return false; + BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } } - - return 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 d2c1233cee..b503eab04e 100644 --- a/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs +++ b/test/Kestrel.Core.Tests/HttpUtilitiesTest.cs @@ -170,7 +170,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [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 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [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 6aa1654681..08646dc61a 100644 --- a/tools/CodeGenerator/KnownHeaders.cs +++ b/tools/CodeGenerator/KnownHeaders.cs @@ -68,6 +68,8 @@ namespace CodeGenerator 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 @@ namespace CodeGenerator "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 @@ namespace CodeGenerator { 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 @@ namespace CodeGenerator 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 @@ namespace CodeGenerator Name = header, Index = index, EnhancedSetter = enhancedHeaders.Contains(header), + ExistenceCheck = responseHeadersExistence.Contains(header), PrimaryHeader = responsePrimaryHeaders.Contains(header) }) .Concat(new[] { new KnownHeader @@ -311,6 +332,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http 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" ? $@"