diff --git a/src/Kestrel.Core/Adapter/Internal/RawStream.cs b/src/Kestrel.Core/Adapter/Internal/RawStream.cs index 4f35b95ebe..137b87ea04 100644 --- a/src/Kestrel.Core/Adapter/Internal/RawStream.cs +++ b/src/Kestrel.Core/Adapter/Internal/RawStream.cs @@ -131,7 +131,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal readableBuffer.CopyTo(destination.Span); return count; } - else if (result.IsCompleted) + + if (result.IsCompleted) { return 0; } diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index 6b9ca5514d..7ef91a6b83 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -524,6 +524,9 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Invalid HTTP/2 connection preface. + + Header name cannot be a null or empty string. + The connection or stream was aborted because a write operation was aborted with a CancellationToken. diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index 61eac8a37c..849b3b1ce8 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -274,13 +274,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // This is not complete validation. It is just a quick scan for invalid characters // but doesn't check that the target fully matches the URI spec. - for (var i = 0; i < target.Length; i++) + if (HttpCharacters.ContainsInvalidAuthorityChar(target)) { - var ch = target[i]; - if (!UriUtilities.IsValidAuthorityCharacter(ch)) - { - ThrowRequestTargetRejected(target); - } + ThrowRequestTargetRejected(target); } // The authority-form of request-target is only used for CONNECT diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index d65805cc83..f11619d7f3 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -87,7 +87,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http break; } } - else if (result.IsCompleted) + + // Read() will have already have greedily consumed the entire request body if able. + if (result.IsCompleted) { // Treat any FIN from an upgraded request as expected. // It's up to higher-level consumer (i.e. WebSocket middleware) to determine diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs index f81e87a06c..182e90aa01 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs @@ -5862,7 +5862,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected override void SetValueFast(string key, in StringValues value) { - ValidateHeaderCharacters(value); + ValidateHeaderValueCharacters(value); switch (key.Length) { case 13: @@ -6167,7 +6167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected override bool AddValueFast(string key, in StringValues value) { - ValidateHeaderCharacters(value); + ValidateHeaderValueCharacters(value); switch (key.Length) { case 13: @@ -6611,7 +6611,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http break; } - ValidateHeaderCharacters(key); + ValidateHeaderNameCharacters(key); Unknown.Add(key, value); // Return true, above will throw and exit for false return true; diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs index 444ac62774..b537e44e5f 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http @@ -45,6 +46,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { ThrowHeadersReadOnlyException(); } + if (string.IsNullOrEmpty(key)) + { + ThrowInvalidEmtpyHeaderName(); + } if (value.Count == 0) { RemoveFast(key); @@ -170,6 +175,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { ThrowHeadersReadOnlyException(); } + if (string.IsNullOrEmpty(key)) + { + ThrowInvalidEmtpyHeaderName(); + } if (value.Count > 0 && !AddValueFast(key, value)) { @@ -241,30 +250,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return TryGetValueFast(key, out value); } - public static void ValidateHeaderCharacters(in StringValues headerValues) + public static void ValidateHeaderValueCharacters(in StringValues headerValues) { var count = headerValues.Count; for (var i = 0; i < count; i++) { - ValidateHeaderCharacters(headerValues[i]); + ValidateHeaderValueCharacters(headerValues[i]); } } - public static void ValidateHeaderCharacters(string headerCharacters) + public static void ValidateHeaderValueCharacters(string headerCharacters) { if (headerCharacters != null) { - foreach (var ch in headerCharacters) + var invalid = HttpCharacters.IndexOfInvalidFieldValueChar(headerCharacters); + if (invalid >= 0) { - if (ch < 0x20 || ch > 0x7E) - { - ThrowInvalidHeaderCharacter(ch); - } + ThrowInvalidHeaderCharacter(headerCharacters[invalid]); } } } + public static void ValidateHeaderNameCharacters(string headerCharacters) + { + var invalid = HttpCharacters.IndexOfInvalidTokenChar(headerCharacters); + if (invalid >= 0) + { + ThrowInvalidHeaderCharacter(headerCharacters[invalid]); + } + } + public static unsafe ConnectionOptions ParseConnection(in StringValues connection) { var connectionOptions = ConnectionOptions.None; @@ -440,5 +456,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { throw new InvalidOperationException(CoreStrings.FormatInvalidAsciiOrControlChar(string.Format("0x{0:X4}", (ushort)ch))); } + + private static void ThrowInvalidEmtpyHeaderName() + { + throw new InvalidOperationException(CoreStrings.InvalidEmptyHeaderName); + } } } diff --git a/src/Kestrel.Core/Internal/Http/HttpParser.cs b/src/Kestrel.Core/Internal/Http/HttpParser.cs index b3d8d90d9c..2bc58f1483 100644 --- a/src/Kestrel.Core/Internal/Http/HttpParser.cs +++ b/src/Kestrel.Core/Internal/Http/HttpParser.cs @@ -363,6 +363,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var valueEnd = length - 3; var nameEnd = FindEndOfName(headerLine, length); + // Header name is empty + if (nameEnd == 0) + { + RejectRequestHeader(headerLine, length); + } + if (headerLine[valueEnd + 2] != ByteLF) { RejectRequestHeader(headerLine, length); @@ -437,55 +443,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http [MethodImpl(MethodImplOptions.NoInlining)] private unsafe Span GetUnknownMethod(byte* data, int length, out int methodLength) { - methodLength = 0; - for (var i = 0; i < length; i++) + var invalidIndex = HttpCharacters.IndexOfInvalidTokenChar(data, length); + + if (invalidIndex <= 0 || data[invalidIndex] != ByteSpace) { - var ch = data[i]; - - if (ch == ByteSpace) - { - if (i == 0) - { - RejectRequestLine(data, length); - } - - methodLength = i; - break; - } - else if (!IsValidTokenChar((char)ch)) - { - RejectRequestLine(data, length); - } + RejectRequestLine(data, length); } + methodLength = invalidIndex; return new Span(data, methodLength); } - private static bool IsValidTokenChar(char c) - { - // Determines if a character is valid as a 'token' as defined in the - // HTTP spec: https://tools.ietf.org/html/rfc7230#section-3.2.6 - return - (c >= '0' && c <= '9') || - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z') || - c == '!' || - c == '#' || - c == '$' || - c == '%' || - c == '&' || - c == '\'' || - c == '*' || - c == '+' || - c == '-' || - c == '.' || - c == '^' || - c == '_' || - c == '`' || - c == '|' || - c == '~'; - } - [StackTraceHidden] private unsafe void RejectRequestLine(byte* requestLine, int length) => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length); diff --git a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs index a4b81cf69a..41a27c4aac 100644 --- a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http [MethodImpl(MethodImplOptions.NoInlining)] private void SetValueUnknown(string key, in StringValues value) { - ValidateHeaderCharacters(key); + ValidateHeaderNameCharacters(key); Unknown[key] = value; } diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index 339c9d182a..3ed2b86c58 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -57,7 +57,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http slice.CopyTo(buffer.Span); return actual; } - else if (result.IsCompleted) + + if (result.IsCompleted) { return 0; } @@ -98,7 +99,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http #endif } } - else if (result.IsCompleted) + + if (result.IsCompleted) { return; } diff --git a/src/Kestrel.Core/Internal/Http/UrlDecoder.cs b/src/Kestrel.Core/Internal/Http/UrlDecoder.cs index e5ed2c86fa..3f6533e3cd 100644 --- a/src/Kestrel.Core/Internal/Http/UrlDecoder.cs +++ b/src/Kestrel.Core/Internal/Http/UrlDecoder.cs @@ -8,8 +8,6 @@ namespace Microsoft.AspNetCore.Connections.Abstractions { internal class UrlDecoder { - static bool[] IsAllowed = new bool[0x7F + 1]; - /// /// Unescape a URL path /// @@ -334,78 +332,5 @@ namespace Microsoft.AspNetCore.Connections.Abstractions return false; } - static UrlDecoder() - { - // Unreserved - IsAllowed['A'] = true; - IsAllowed['B'] = true; - IsAllowed['C'] = true; - IsAllowed['D'] = true; - IsAllowed['E'] = true; - IsAllowed['F'] = true; - IsAllowed['G'] = true; - IsAllowed['H'] = true; - IsAllowed['I'] = true; - IsAllowed['J'] = true; - IsAllowed['K'] = true; - IsAllowed['L'] = true; - IsAllowed['M'] = true; - IsAllowed['N'] = true; - IsAllowed['O'] = true; - IsAllowed['P'] = true; - IsAllowed['Q'] = true; - IsAllowed['R'] = true; - IsAllowed['S'] = true; - IsAllowed['T'] = true; - IsAllowed['U'] = true; - IsAllowed['V'] = true; - IsAllowed['W'] = true; - IsAllowed['X'] = true; - IsAllowed['Y'] = true; - IsAllowed['Z'] = true; - - IsAllowed['a'] = true; - IsAllowed['b'] = true; - IsAllowed['c'] = true; - IsAllowed['d'] = true; - IsAllowed['e'] = true; - IsAllowed['f'] = true; - IsAllowed['g'] = true; - IsAllowed['h'] = true; - IsAllowed['i'] = true; - IsAllowed['j'] = true; - IsAllowed['k'] = true; - IsAllowed['l'] = true; - IsAllowed['m'] = true; - IsAllowed['n'] = true; - IsAllowed['o'] = true; - IsAllowed['p'] = true; - IsAllowed['q'] = true; - IsAllowed['r'] = true; - IsAllowed['s'] = true; - IsAllowed['t'] = true; - IsAllowed['u'] = true; - IsAllowed['v'] = true; - IsAllowed['w'] = true; - IsAllowed['x'] = true; - IsAllowed['y'] = true; - IsAllowed['z'] = true; - - IsAllowed['0'] = true; - IsAllowed['1'] = true; - IsAllowed['2'] = true; - IsAllowed['3'] = true; - IsAllowed['4'] = true; - IsAllowed['5'] = true; - IsAllowed['6'] = true; - IsAllowed['7'] = true; - IsAllowed['8'] = true; - IsAllowed['9'] = true; - - IsAllowed['-'] = true; - IsAllowed['_'] = true; - IsAllowed['.'] = true; - IsAllowed['~'] = true; - } } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 15c47b381c..a2110bee0a 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -147,7 +147,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 await ProcessFrameAsync(application); } } - else if (result.IsCompleted) + + if (result.IsCompleted) { return; } @@ -254,6 +255,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return true; } } + if (result.IsCompleted) { return false; diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpCharacters.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpCharacters.cs new file mode 100644 index 0000000000..341bd00407 --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/HttpCharacters.cs @@ -0,0 +1,202 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Runtime.CompilerServices; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + internal static class HttpCharacters + { + private static readonly int _tableSize = 128; + private static readonly bool[] _alphaNumeric = InitializeAlphaNumeric(); + private static readonly bool[] _authority = InitializeAuthority(); + private static readonly bool[] _token = InitializeToken(); + private static readonly bool[] _host = InitializeHost(); + private static readonly bool[] _fieldValue = InitializeFieldValue(); + + internal static void Initialize() + { + // Access _alphaNumeric to initialize static fields + var initialize = _alphaNumeric; + } + + private static bool[] InitializeAlphaNumeric() + { + // ALPHA and DIGIT https://tools.ietf.org/html/rfc5234#appendix-B.1 + var alphaNumeric = new bool[_tableSize]; + for (var c = '0'; c <= '9'; c++) + { + alphaNumeric[c] = true; + } + for (var c = 'A'; c <= 'Z'; c++) + { + alphaNumeric[c] = true; + } + for (var c = 'a'; c <= 'z'; c++) + { + alphaNumeric[c] = true; + } + return alphaNumeric; + } + + private static bool[] InitializeAuthority() + { + // Authority https://tools.ietf.org/html/rfc3986#section-3.2 + // Examples: + // microsoft.com + // hostname:8080 + // [::]:8080 + // [fe80::] + // 127.0.0.1 + // user@host.com + // user:password@host.com + var authority = new bool[_tableSize]; + Array.Copy(_alphaNumeric, authority, _tableSize); + authority[':'] = true; + authority['.'] = true; + authority['['] = true; + authority[']'] = true; + authority['@'] = true; + return authority; + } + + private static bool[] InitializeToken() + { + // tchar https://tools.ietf.org/html/rfc7230#appendix-B + var token = new bool[_tableSize]; + Array.Copy(_alphaNumeric, token, _tableSize); + token['!'] = true; + token['#'] = true; + token['$'] = true; + token['%'] = true; + token['&'] = true; + token['\''] = true; + token['*'] = true; + token['+'] = true; + token['-'] = true; + token['.'] = true; + token['^'] = true; + token['_'] = true; + token['`'] = true; + token['|'] = true; + token['~'] = true; + return token; + } + + private static bool[] InitializeHost() + { + // Matches Http.Sys + // Matches RFC 3986 except "*" / "+" / "," / ";" / "=" and "%" HEXDIG HEXDIG which are not allowed by Http.Sys + var host = new bool[_tableSize]; + Array.Copy(_alphaNumeric, host, _tableSize); + host['!'] = true; + host['$'] = true; + host['&'] = true; + host['\''] = true; + host['('] = true; + host[')'] = true; + host['-'] = true; + host['.'] = true; + host['_'] = true; + host['~'] = true; + return host; + } + + private static bool[] InitializeFieldValue() + { + // field-value https://tools.ietf.org/html/rfc7230#section-3.2 + var fieldValue = new bool[_tableSize]; + for (var c = 0x20; c <= 0x7e; c++) // VCHAR and SP + { + fieldValue[c] = true; + } + return fieldValue; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool ContainsInvalidAuthorityChar(Span s) + { + var authority = _authority; + + for (var i = 0; i < s.Length; i++) + { + var c = s[i]; + if (c >= (uint)authority.Length || !authority[c]) + { + return true; + } + } + + return false; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int IndexOfInvalidHostChar(string s) + { + var host = _host; + + for (var i = 0; i < s.Length; i++) + { + var c = s[i]; + if (c >= (uint)host.Length || !host[c]) + { + return i; + } + } + + return -1; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int IndexOfInvalidTokenChar(string s) + { + var token = _token; + + for (var i = 0; i < s.Length; i++) + { + var c = s[i]; + if (c >= (uint)token.Length || !token[c]) + { + return i; + } + } + + return -1; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public unsafe static int IndexOfInvalidTokenChar(byte* s, int length) + { + var token = _token; + + for (var i = 0; i < length; i++) + { + var c = s[i]; + if (c >= (uint)token.Length || !token[c]) + { + return i; + } + } + + return -1; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int IndexOfInvalidFieldValueChar(string s) + { + var fieldValue = _fieldValue; + + for (var i = 0; i < s.Length; i++) + { + var c = s[i]; + if (c >= (uint)fieldValue.Length || !fieldValue[c]) + { + return i; + } + } + + return -1; + } + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.Generated.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.Generated.cs index 15e3e1cd7b..1dd2e252c2 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.Generated.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.Generated.cs @@ -51,7 +51,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure SetKnownMethod(_mask8Chars, _httpConnectMethodLong, HttpMethod.Connect, 7); SetKnownMethod(_mask8Chars, _httpOptionsMethodLong, HttpMethod.Options, 7); FillKnownMethodsGaps(); - InitializeHostCharValidity(); _methodNames[(byte)HttpMethod.Connect] = HttpMethods.Connect; _methodNames[(byte)HttpMethod.Delete] = HttpMethods.Delete; _methodNames[(byte)HttpMethod.Get] = HttpMethods.Get; diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs index 0af41644ff..d4d807aa13 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs @@ -13,8 +13,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public static partial class HttpUtilities { - private static readonly bool[] HostCharValidity = new bool[127]; - public const string Http10Version = "HTTP/1.0"; public const string Http11Version = "HTTP/1.1"; public const string Http2Version = "HTTP/2"; @@ -31,35 +29,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure private const ulong _http10VersionLong = 3471766442030158920; // GetAsciiStringAsLong("HTTP/1.0"); const results in better codegen private const ulong _http11VersionLong = 3543824036068086856; // GetAsciiStringAsLong("HTTP/1.1"); const results in better codegen - // Only called from the static constructor - private static void InitializeHostCharValidity() - { - // Matches Http.Sys - // Matches RFC 3986 except "*" / "+" / "," / ";" / "=" and "%" HEXDIG HEXDIG which are not allowed by Http.Sys - HostCharValidity['!'] = true; - HostCharValidity['$'] = true; - HostCharValidity['&'] = true; - HostCharValidity['\''] = true; - HostCharValidity['('] = true; - HostCharValidity[')'] = true; - HostCharValidity['-'] = true; - HostCharValidity['.'] = true; - HostCharValidity['_'] = true; - HostCharValidity['~'] = true; - for (var ch = '0'; ch <= '9'; ch++) - { - HostCharValidity[ch] = true; - } - for (var ch = 'A'; ch <= 'Z'; ch++) - { - HostCharValidity[ch] = true; - } - for (var ch = 'a'; ch <= 'z'; ch++) - { - HostCharValidity[ch] = true; - } - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void SetKnownMethod(ulong mask, ulong knownMethodUlong, HttpMethod knownMethod, int length) { @@ -448,16 +417,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); } - // Enregister array - var hostCharValidity = HostCharValidity; - for (var i = 0; i < hostText.Length; i++) + var invalid = HttpCharacters.IndexOfInvalidHostChar(hostText); + if (invalid >= 0) { - if (!hostCharValidity[hostText[i]]) - { - // Tail call - ValidateHostPort(hostText, i); - return; - } + // Tail call + ValidateHostPort(hostText, invalid); } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/UriUtilities.cs b/src/Kestrel.Core/Internal/Infrastructure/UriUtilities.cs deleted file mode 100644 index 272b30cabf..0000000000 --- a/src/Kestrel.Core/Internal/Infrastructure/UriUtilities.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure -{ - public class UriUtilities - { - /// - /// Returns true if character is valid in the 'authority' section of a URI. - /// - /// - /// The character - /// - public static bool IsValidAuthorityCharacter(byte ch) - { - // Examples: - // microsoft.com - // hostname:8080 - // [::]:8080 - // [fe80::] - // 127.0.0.1 - // user@host.com - // user:password@host.com - return - (ch >= '0' && ch <= '9') || - (ch >= 'A' && ch <= 'Z') || - (ch >= 'a' && ch <= 'z') || - ch == ':' || - ch == '.' || - ch == '[' || - ch == ']' || - ch == '@'; - } - } -} diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index a65e858eb1..9caf8d0505 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -57,6 +57,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core Features = new FeatureCollection(); _serverAddresses = new ServerAddressesFeature(); Features.Set(_serverAddresses); + + HttpCharacters.Initialize(); } private static ServiceContext CreateServiceContext(IOptions options, ILoggerFactory loggerFactory) diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 6e6f832906..20c07bf02b 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1904,6 +1904,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatHttp2ErrorInvalidPreface() => GetString("Http2ErrorInvalidPreface"); + /// + /// Header name cannot be a null or empty string. + /// + internal static string InvalidEmptyHeaderName + { + get => GetString("InvalidEmptyHeaderName"); + } + + /// + /// Header name cannot be a null or empty string. + /// + internal static string FormatInvalidEmptyHeaderName() + => GetString("InvalidEmptyHeaderName"); + /// /// The connection or stream was aborted because a write operation was aborted with a CancellationToken. /// diff --git a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs index e6871565ab..ce15b17a46 100644 --- a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs @@ -3841,6 +3841,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { return frame; } + + if (result.IsCompleted) + { + throw new IOException("The reader completed without returning a frame."); + } } finally { diff --git a/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs b/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs index a8e2e7f441..1afb559ad1 100644 --- a/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs +++ b/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs @@ -79,6 +79,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [InlineData("Server", "Dašta")] [InlineData("Unknownš-Header", "Data")] [InlineData("Seršver", "Data")] + [InlineData("Server\"", "Data")] + [InlineData("Server(", "Data")] + [InlineData("Server)", "Data")] + [InlineData("Server,", "Data")] + [InlineData("Server/", "Data")] + [InlineData("Server:", "Data")] + [InlineData("Server;", "Data")] + [InlineData("Server<", "Data")] + [InlineData("Server=", "Data")] + [InlineData("Server>", "Data")] + [InlineData("Server?", "Data")] + [InlineData("Server@", "Data")] + [InlineData("Server[", "Data")] + [InlineData("Server\\", "Data")] + [InlineData("Server]", "Data")] + [InlineData("Server{", "Data")] + [InlineData("Server}", "Data")] + [InlineData("", "Data")] + [InlineData(null, "Data")] public void AddingControlOrNonAsciiCharactersToHeadersThrows(string key, string value) { var responseHeaders = new HttpResponseHeaders(); diff --git a/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs b/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs index 3bc0c9d286..02122cff42 100644 --- a/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs +++ b/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs @@ -1,14 +1,17 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.IO; using System.Linq; using System.Net; +using System.Net.Sockets; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Testing; using Xunit; @@ -639,6 +642,54 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } } + + [Theory] + [MemberData(nameof(ConnectionAdapterData))] + public async Task ClosingConnectionMidChunkPrefixThrows(ListenOptions listenOptions) + { + var testContext = new TestServiceContext(LoggerFactory); + var readStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var exTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + using (var server = new TestServer(async httpContext => + { + var readTask = httpContext.Request.Body.CopyToAsync(Stream.Null); + readStartedTcs.SetResult(null); + + try + { + await readTask; + } + catch (BadHttpRequestException badRequestEx) + { + exTcs.TrySetResult(badRequestEx); + } + catch (Exception ex) + { + exTcs.SetException(ex); + } + }, testContext, listenOptions)) + { + using (var connection = server.CreateConnection()) + { + await connection.SendAll( + "POST / HTTP/1.1", + "Host:", + "Transfer-Encoding: chunked", + "", + "1"); + + await readStartedTcs.Task.TimeoutAfter(TestConstants.DefaultTimeout); + + connection.Socket.Shutdown(SocketShutdown.Send); + + await connection.ReceiveEnd(); + + var badReqEx = await exTcs.Task.TimeoutAfter(TestConstants.DefaultTimeout); + Assert.Equal(RequestRejectionReason.UnexpectedEndOfRequestContent, badReqEx.Reason); + } + } + } } } diff --git a/test/Kestrel.FunctionalTests/Http2/TlsTests.cs b/test/Kestrel.FunctionalTests/Http2/TlsTests.cs index d65ec34f14..28eef0675d 100644 --- a/test/Kestrel.FunctionalTests/Http2/TlsTests.cs +++ b/test/Kestrel.FunctionalTests/Http2/TlsTests.cs @@ -100,19 +100,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 var consumed = buffer.Start; var examined = buffer.End; - if (buffer.IsEmpty && result.IsCompleted) - { - throw new IOException("The reader completed without returning a frame."); - } - try { - // Assert.True(buffer.Length > 0); - if (Http2FrameReader.ReadFrame(buffer, frame, 16_384, out consumed, out examined)) { return frame; } + + if (result.IsCompleted) + { + throw new IOException("The reader completed without returning a frame."); + } } finally { diff --git a/test/Kestrel.FunctionalTests/RequestTests.cs b/test/Kestrel.FunctionalTests/RequestTests.cs index 732ed8074a..e4ff7237bc 100644 --- a/test/Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.FunctionalTests/RequestTests.cs @@ -1696,8 +1696,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [Fact] public async Task DoesNotEnforceRequestBodyMinimumDataRateOnUpgradedRequest() { - var appEvent = new TaskCompletionSource(); - var delayEvent = new TaskCompletionSource(); + var appEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var delayEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory) { SystemClock = new SystemClock() diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 94ac2c8e39..8d7e2a7fa9 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -428,6 +428,9 @@ namespace Microsoft.AspNetCore.Testing new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r\r", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r \n", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, + + // Empty header name + new[] { ": value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@": value\x0D\x0A") }, }; public static TheoryData HostHeaderData diff --git a/test/shared/TestConnection.cs b/test/shared/TestConnection.cs index 04e547214d..2d760a5bc8 100644 --- a/test/shared/TestConnection.cs +++ b/test/shared/TestConnection.cs @@ -47,6 +47,8 @@ namespace Microsoft.AspNetCore.Testing _reader = new StreamReader(_stream, Encoding.ASCII); } + public Socket Socket => _socket; + public Stream Stream => _stream; public StreamReader Reader => _reader; diff --git a/tools/CodeGenerator/HttpUtilities/HttpUtilities.cs b/tools/CodeGenerator/HttpUtilities/HttpUtilities.cs index 0c70c0c188..7263d1fd16 100644 --- a/tools/CodeGenerator/HttpUtilities/HttpUtilities.cs +++ b/tools/CodeGenerator/HttpUtilities/HttpUtilities.cs @@ -84,7 +84,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure {{ {4} FillKnownMethodsGaps(); - InitializeHostCharValidity(); {5} }} diff --git a/tools/CodeGenerator/KnownHeaders.cs b/tools/CodeGenerator/KnownHeaders.cs index 12de0493fd..84aec20eab 100644 --- a/tools/CodeGenerator/KnownHeaders.cs +++ b/tools/CodeGenerator/KnownHeaders.cs @@ -403,7 +403,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected override void SetValueFast(string key, in StringValues value) {{{(loop.ClassName == "HttpResponseHeaders" ? @" - ValidateHeaderCharacters(value);" : "")} + ValidateHeaderValueCharacters(value);" : "")} switch (key.Length) {{{Each(loop.HeadersByLength, byLength => $@" case {byLength.Key}: @@ -425,7 +425,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected override bool AddValueFast(string key, in StringValues value) {{{(loop.ClassName == "HttpResponseHeaders" ? @" - ValidateHeaderCharacters(value);" : "")} + ValidateHeaderValueCharacters(value);" : "")} switch (key.Length) {{{Each(loop.HeadersByLength, byLength => $@" case {byLength.Key}: @@ -451,7 +451,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http break;")} }} {(loop.ClassName == "HttpResponseHeaders" ? @" - ValidateHeaderCharacters(key);" : "")} + ValidateHeaderNameCharacters(key);" : "")} Unknown.Add(key, value); // Return true, above will throw and exit for false return true;