From a8968a736cb31ac516624ed53e053054d1afb637 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 8 Jan 2020 19:18:08 -0800 Subject: [PATCH 1/7] Add option to interpret request headers as Latin1 --- .../Core/src/Internal/ConfigurationReader.cs | 17 +++- .../Core/src/Internal/Http/Http1Connection.cs | 2 +- .../Internal/Http/HttpHeaders.Generated.cs | 4 +- .../Core/src/Internal/Http/HttpProtocol.cs | 13 ++- .../src/Internal/Http/HttpRequestHeaders.cs | 6 +- .../Core/src/Internal/Http2/Http2Stream.cs | 2 +- .../Internal/Infrastructure/HttpUtilities.cs | 18 ++-- .../Infrastructure/StringUtilities.cs | 87 +++++++++++++++++++ .../Core/src/KestrelConfigurationLoader.cs | 2 + .../Kestrel/Core/src/KestrelServerOptions.cs | 5 ++ src/Servers/Kestrel/Core/test/UTF8Decoding.cs | 4 +- .../BytesToStringBenchmark.cs | 4 +- src/Servers/Kestrel/shared/KnownHeaders.cs | 4 +- .../Http2/Http2TestBase.cs | 2 +- 14 files changed, 148 insertions(+), 22 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 259f2c61b681..79dc84d5c1c1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -15,11 +15,13 @@ internal class ConfigurationReader private const string EndpointDefaultsKey = "EndpointDefaults"; private const string EndpointsKey = "Endpoints"; private const string UrlKey = "Url"; + private const string Latin1RequestHeadersKey = "Latin1RequestHeaders"; private IConfiguration _configuration; private IDictionary _certificates; private IList _endpoints; private EndpointDefaults _endpointDefaults; + private bool? _latin1RequestHeaders; public ConfigurationReader(IConfiguration configuration) { @@ -65,6 +67,19 @@ public IEnumerable Endpoints } } + public bool Latin1RequestHeaders + { + get + { + if (_latin1RequestHeaders is null) + { + _latin1RequestHeaders = _configuration.GetValue(Latin1RequestHeadersKey); + } + + return _latin1RequestHeaders.Value; + } + } + private void ReadCertificates() { _certificates = new Dictionary(0); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 8cf1bd068be4..986fa9d77412 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -37,7 +37,7 @@ internal partial class Http1Connection : HttpProtocol, IRequestProcessor private int _remainingRequestHeadersBytesAllowed; public Http1Connection(HttpConnectionContext context) - : base(context) + : base(context, isHttp1: true) { _context = context; _parser = ServiceContext.HttpParser; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs index e626a19d9cf4..aa844379d423 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -6105,7 +6105,7 @@ public unsafe void Append(Span name, Span value) } // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(); + var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); if ((_bits & flag) == 0) { // We didn't already have a header set, so add a new one. @@ -6123,7 +6123,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the "known" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(); + var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); AppendUnknownHeaders(name, valueStr); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 7f10c3af64cc..906f4e030cbb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -72,12 +72,19 @@ internal abstract partial class HttpProtocol : IHttpResponseControl private Stream _requestStreamInternal; private Stream _responseStreamInternal; - public HttpProtocol(HttpConnectionContext context) + private readonly bool _useLatin1; + + public HttpProtocol(HttpConnectionContext context, bool isHttp1) { _context = context; ServerOptions = ServiceContext.ServerOptions; - HttpRequestHeaders = new HttpRequestHeaders(reuseHeaderValues: !ServerOptions.DisableStringReuse); + _useLatin1 = isHttp1 && ServerOptions.Latin1RequestHeaders; + + HttpRequestHeaders = new HttpRequestHeaders( + reuseHeaderValues: !ServerOptions.DisableStringReuse, + useLatin1: _useLatin1); + HttpResponseControl = this; } @@ -513,7 +520,7 @@ public void OnTrailer(Span name, Span value) } string key = name.GetHeaderName(); - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(); + var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); RequestTrailers.Append(key, valueStr); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 32e9e41d84d5..00890b2a887a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -15,11 +15,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http internal sealed partial class HttpRequestHeaders : HttpHeaders { private readonly bool _reuseHeaderValues; + private readonly bool _useLatin1; private long _previousBits = 0; - public HttpRequestHeaders(bool reuseHeaderValues = true) + public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false) { _reuseHeaderValues = reuseHeaderValues; + _useLatin1 = useLatin1; } public void OnHeadersComplete() @@ -80,7 +82,7 @@ private void AppendContentLength(Span value) parsed < 0 || consumed != value.Length) { - BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetAsciiOrUTF8StringNonNullCharacters()); + BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1)); } _contentLength = parsed; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 27dd7762ae63..695e51c5e9a2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -33,7 +33,7 @@ internal abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem private readonly object _completionLock = new object(); public Http2Stream(Http2StreamContext context) - : base(context) + : base(context, isHttp1: false) { _context = context; diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs index f57c296e1ef0..5f164a99284e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs @@ -130,7 +130,7 @@ public static unsafe string GetAsciiStringNonNullCharacters(this Span span return asciiString; } - public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) + public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span, bool useLatin1) { if (span.IsEmpty) { @@ -152,16 +152,24 @@ public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span>(output), return isValid; } + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + public static unsafe void GetLatin1String(byte* input, char* output, int count) + { + // Calculate end position + var end = input + count; + + do + { + // If Vector not-accelerated or remaining less than vector size + if (!Vector.IsHardwareAccelerated || input > end - Vector.Count) + { + if (IntPtr.Size == 8) // Use Intrinsic switch for branch elimination + { + // 64-bit: Loop longs by default + while (input <= end - sizeof(long)) + { + output[0] = (char)input[0]; + output[1] = (char)input[1]; + output[2] = (char)input[2]; + output[3] = (char)input[3]; + output[4] = (char)input[4]; + output[5] = (char)input[5]; + output[6] = (char)input[6]; + output[7] = (char)input[7]; + + input += sizeof(long); + output += sizeof(long); + } + if (input <= end - sizeof(int)) + { + output[0] = (char)input[0]; + output[1] = (char)input[1]; + output[2] = (char)input[2]; + output[3] = (char)input[3]; + + input += sizeof(int); + output += sizeof(int); + } + } + else + { + // 32-bit: Loop ints by default + while (input <= end - sizeof(int)) + { + output[0] = (char)input[0]; + output[1] = (char)input[1]; + output[2] = (char)input[2]; + output[3] = (char)input[3]; + + input += sizeof(int); + output += sizeof(int); + } + } + if (input <= end - sizeof(short)) + { + output[0] = (char)input[0]; + output[1] = (char)input[1]; + + input += sizeof(short); + output += sizeof(short); + } + if (input < end) + { + output[0] = (char)input[0]; + } + + return; + } + + // do/while as entry condition already checked + do + { + var vector = Unsafe.AsRef>(input); + Vector.Widen( + vector, + out Unsafe.AsRef>(output), + out Unsafe.AsRef>(output + Vector.Count)); + + input += Vector.Count; + output += Vector.Count; + } while (input <= end - Vector.Count); + + // Vector path done, loop back to do non-Vector + // If is a exact multiple of vector size, bail now + } while (input < end); + } + [MethodImpl(MethodImplOptions.AggressiveOptimization)] public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue, Span newValue) { diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 4522fedd6293..5e202f3efa29 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -222,6 +222,8 @@ public void Load() } _loaded = true; + Options.Latin1RequestHeaders = ConfigurationReader.Latin1RequestHeaders; + LoadDefaultCert(ConfigurationReader); foreach (var endpoint in ConfigurationReader.Endpoints) diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index c383945c98b5..a3c79dba377a 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -92,6 +92,11 @@ public class KestrelServerOptions /// internal bool IsDevCertLoaded { get; set; } + /// + /// Treat request headers as Latin-1 or ISO/IEC 8859-1 instead of UTF-8. Still reject ASCII control characters. + /// + internal bool Latin1RequestHeaders { get; set; } + /// /// Specifies a configuration Action to run for each newly created endpoint. Calling this again will replace /// the prior action. diff --git a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs index 532e13781d99..2bc3e29e855b 100644 --- a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs +++ b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs @@ -17,7 +17,7 @@ public class UTF8DecodingTests [InlineData(new byte[] { 0xef, 0xbf, 0xbd })] // 3 bytes: Replacement character, highest UTF-8 character currently encoded in the UTF-8 code page private void FullUTF8RangeSupported(byte[] encodedBytes) { - var s = encodedBytes.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(); + var s = encodedBytes.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false); Assert.Equal(1, s.Length); } @@ -35,7 +35,7 @@ private void ExceptionThrownForZeroOrNonAscii(byte[] bytes) var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray(); Array.Copy(bytes, 0, byteRange, position, bytes.Length); - Assert.Throws(() => byteRange.AsSpan().GetAsciiOrUTF8StringNonNullCharacters()); + Assert.Throws(() => byteRange.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false)); } } } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs index 28f365d7da45..7c96a2aa805e 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 BenchmarkDotNet.Attributes; @@ -67,7 +67,7 @@ public void Utf8BytesToString() { for (uint i = 0; i < Iterations; i++) { - HttpUtilities.GetAsciiOrUTF8StringNonNullCharacters(_utf8Bytes); + HttpUtilities.GetAsciiOrUTF8StringNonNullCharacters(_utf8Bytes, useLatin1: false); } } diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index e38dfb19a931..f67795d52c21 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -985,7 +985,7 @@ public unsafe void Append(Span name, Span value) }} // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(); + var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); if ((_bits & flag) == 0) {{ // We didn't already have a header set, so add a new one. @@ -1003,7 +1003,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the ""known"" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(); + var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); AppendUnknownHeaders(name, valueStr); }} }}" : "")} diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 5b875bffab16..fdce1f5121de 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -402,7 +402,7 @@ public override void Dispose() void IHttpHeadersHandler.OnHeader(Span name, Span value) { - _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(); + _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false); } void IHttpHeadersHandler.OnHeadersComplete() { } From f8c8829e95adaf0ad75e84b71ad47d60135d93ea Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 9 Jan 2020 19:19:44 -0800 Subject: [PATCH 2/7] Work around inability to make signature changes visible to tests --- .../Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs | 5 +++++ .../Core/src/Internal/Infrastructure/HttpUtilities.cs | 5 +++++ src/Servers/Kestrel/Core/test/UTF8Decoding.cs | 4 ++-- .../perf/Kestrel.Performance/BytesToStringBenchmark.cs | 2 +- .../test/InMemory.FunctionalTests/Http2/Http2TestBase.cs | 2 +- 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 00890b2a887a..c52b621f5d63 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -18,6 +18,11 @@ internal sealed partial class HttpRequestHeaders : HttpHeaders private readonly bool _useLatin1; private long _previousBits = 0; + public HttpRequestHeaders(bool reuseHeaderValues = true) + : this(reuseHeaderValues, useLatin1: false) + { + } + public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false) { _reuseHeaderValues = reuseHeaderValues; diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs index 5f164a99284e..446643dbf7ec 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs @@ -130,6 +130,11 @@ public static unsafe string GetAsciiStringNonNullCharacters(this Span span return asciiString; } + public static string GetAsciiOrUTF8StringNonNullCharacters(this Span span) + { + return GetAsciiOrUTF8StringNonNullCharacters(span, useLatin1: false); + } + public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span, bool useLatin1) { if (span.IsEmpty) diff --git a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs index 2bc3e29e855b..532e13781d99 100644 --- a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs +++ b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs @@ -17,7 +17,7 @@ public class UTF8DecodingTests [InlineData(new byte[] { 0xef, 0xbf, 0xbd })] // 3 bytes: Replacement character, highest UTF-8 character currently encoded in the UTF-8 code page private void FullUTF8RangeSupported(byte[] encodedBytes) { - var s = encodedBytes.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false); + var s = encodedBytes.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(); Assert.Equal(1, s.Length); } @@ -35,7 +35,7 @@ private void ExceptionThrownForZeroOrNonAscii(byte[] bytes) var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray(); Array.Copy(bytes, 0, byteRange, position, bytes.Length); - Assert.Throws(() => byteRange.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false)); + Assert.Throws(() => byteRange.AsSpan().GetAsciiOrUTF8StringNonNullCharacters()); } } } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs index 7c96a2aa805e..9341da2b0534 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs @@ -67,7 +67,7 @@ public void Utf8BytesToString() { for (uint i = 0; i < Iterations; i++) { - HttpUtilities.GetAsciiOrUTF8StringNonNullCharacters(_utf8Bytes, useLatin1: false); + HttpUtilities.GetAsciiOrUTF8StringNonNullCharacters(_utf8Bytes); } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index fdce1f5121de..5b875bffab16 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -402,7 +402,7 @@ public override void Dispose() void IHttpHeadersHandler.OnHeader(Span name, Span value) { - _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(useLatin1: false); + _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(); } void IHttpHeadersHandler.OnHeadersComplete() { } From e24e0bc914c635e4650e0a44039e9173c44d2fac Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 10 Jan 2020 15:00:27 -0800 Subject: [PATCH 3/7] Address PR feedback --- .../Core/src/Internal/Http/Http1Connection.cs | 2 +- .../Internal/Http/HttpHeaders.Generated.cs | 4 +- .../Core/src/Internal/Http/HttpProtocol.cs | 9 +-- .../src/Internal/Http/HttpRequestHeaders.cs | 2 +- .../Core/src/Internal/Http2/Http2Stream.cs | 2 +- .../Internal/Infrastructure/HttpUtilities.cs | 59 ++++++++++++------ .../Infrastructure/StringUtilities.cs | 61 +++++++++++++++++-- src/Servers/Kestrel/shared/KnownHeaders.cs | 4 +- 8 files changed, 108 insertions(+), 35 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 986fa9d77412..8cf1bd068be4 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -37,7 +37,7 @@ internal partial class Http1Connection : HttpProtocol, IRequestProcessor private int _remainingRequestHeadersBytesAllowed; public Http1Connection(HttpConnectionContext context) - : base(context, isHttp1: true) + : base(context) { _context = context; _parser = ServiceContext.HttpParser; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs index aa844379d423..5215875b3838 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -6105,7 +6105,7 @@ public unsafe void Append(Span name, Span value) } // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); + var valueStr = value.GetRequestHeaderString(_useLatin1); if ((_bits & flag) == 0) { // We didn't already have a header set, so add a new one. @@ -6123,7 +6123,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the "known" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); + var valueStr = value.GetRequestHeaderString(_useLatin1); AppendUnknownHeaders(name, valueStr); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 906f4e030cbb..6ed1558b18d2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -72,18 +72,15 @@ internal abstract partial class HttpProtocol : IHttpResponseControl private Stream _requestStreamInternal; private Stream _responseStreamInternal; - private readonly bool _useLatin1; - - public HttpProtocol(HttpConnectionContext context, bool isHttp1) + public HttpProtocol(HttpConnectionContext context) { _context = context; ServerOptions = ServiceContext.ServerOptions; - _useLatin1 = isHttp1 && ServerOptions.Latin1RequestHeaders; HttpRequestHeaders = new HttpRequestHeaders( reuseHeaderValues: !ServerOptions.DisableStringReuse, - useLatin1: _useLatin1); + useLatin1: ServerOptions.Latin1RequestHeaders); HttpResponseControl = this; } @@ -520,7 +517,7 @@ public void OnTrailer(Span name, Span value) } string key = name.GetHeaderName(); - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); + var valueStr = value.GetRequestHeaderString(ServerOptions.Latin1RequestHeaders); RequestTrailers.Append(key, valueStr); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index c52b621f5d63..7f24397d2b93 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -87,7 +87,7 @@ private void AppendContentLength(Span value) parsed < 0 || consumed != value.Length) { - BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1)); + BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetRequestHeaderString(_useLatin1)); } _contentLength = parsed; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 695e51c5e9a2..27dd7762ae63 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -33,7 +33,7 @@ internal abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem private readonly object _completionLock = new object(); public Http2Stream(Http2StreamContext context) - : base(context, isHttp1: false) + : base(context) { _context = context; diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs index 446643dbf7ec..2e3b0d8293f1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs @@ -130,12 +130,7 @@ public static unsafe string GetAsciiStringNonNullCharacters(this Span span return asciiString; } - public static string GetAsciiOrUTF8StringNonNullCharacters(this Span span) - { - return GetAsciiOrUTF8StringNonNullCharacters(span, useLatin1: false); - } - - public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span, bool useLatin1) + public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) { if (span.IsEmpty) { @@ -147,7 +142,7 @@ public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) + { + if (span.IsEmpty) + { + return string.Empty; + } + + var resultString = new string('\0', span.Length); + + fixed (char* output = resultString) + fixed (byte* buffer = span) + { + // This returns false if there are any null (0 byte) characters in the string. + if (!StringUtilities.TryGetLatin1String(buffer, output, span.Length)) + { + // null characters are considered invalid + throw new InvalidOperationException(); + } + } + + return resultString; + } + + public static unsafe string GetRequestHeaderString(this Span span, bool useLatin1) + { + if (useLatin1) + { + return GetLatin1StringNonNullCharacters(span); + } + else + { + return GetAsciiOrUTF8StringNonNullCharacters(span); + } + } + public static string GetAsciiStringEscaped(this Span span, int maxChars) { var sb = new StringBuilder(); diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs index 5ab1f5c73c4e..fd1e3a0daea8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers.Binary; using System.Diagnostics; using System.Numerics; using System.Runtime.CompilerServices; @@ -17,6 +16,9 @@ internal class StringUtilities [MethodImpl(MethodImplOptions.AggressiveOptimization)] public static unsafe bool TryGetAsciiString(byte* input, char* output, int count) { + Debug.Assert(input != null); + Debug.Assert(output != null); + // Calculate end position var end = input + count; // Start as valid @@ -116,10 +118,15 @@ out Unsafe.AsRef>(output), } [MethodImpl(MethodImplOptions.AggressiveOptimization)] - public static unsafe void GetLatin1String(byte* input, char* output, int count) + public static unsafe bool TryGetLatin1String(byte* input, char* output, int count) { + Debug.Assert(input != null); + Debug.Assert(output != null); + // Calculate end position var end = input + count; + // Start as valid + var isValid = true; do { @@ -131,6 +138,8 @@ public static unsafe void GetLatin1String(byte* input, char* output, int count) // 64-bit: Loop longs by default while (input <= end - sizeof(long)) { + isValid &= CheckBytesNotNull(((long*)input)[0]); + output[0] = (char)input[0]; output[1] = (char)input[1]; output[2] = (char)input[2]; @@ -145,6 +154,8 @@ public static unsafe void GetLatin1String(byte* input, char* output, int count) } if (input <= end - sizeof(int)) { + isValid &= CheckBytesNotNull(((int*)input)[0]); + output[0] = (char)input[0]; output[1] = (char)input[1]; output[2] = (char)input[2]; @@ -159,6 +170,8 @@ public static unsafe void GetLatin1String(byte* input, char* output, int count) // 32-bit: Loop ints by default while (input <= end - sizeof(int)) { + isValid &= CheckBytesNotNull(((int*)input)[0]); + output[0] = (char)input[0]; output[1] = (char)input[1]; output[2] = (char)input[2]; @@ -170,6 +183,8 @@ public static unsafe void GetLatin1String(byte* input, char* output, int count) } if (input <= end - sizeof(short)) { + isValid &= CheckBytesNotNull(((short*)input)[0]); + output[0] = (char)input[0]; output[1] = (char)input[1]; @@ -178,16 +193,18 @@ public static unsafe void GetLatin1String(byte* input, char* output, int count) } if (input < end) { + isValid &= CheckBytesNotNull(((sbyte*)input)[0]); output[0] = (char)input[0]; } - return; + return isValid; } // do/while as entry condition already checked do { var vector = Unsafe.AsRef>(input); + isValid &= CheckBytesNotNull(vector); Vector.Widen( vector, out Unsafe.AsRef>(output), @@ -200,6 +217,8 @@ out Unsafe.AsRef>(output), // Vector path done, loop back to do non-Vector // If is a exact multiple of vector size, bail now } while (input < end); + + return isValid; } [MethodImpl(MethodImplOptions.AggressiveOptimization)] @@ -508,7 +527,7 @@ private static bool CheckBytesInAsciiRange(Vector check) // Validate: bytes != 0 && bytes <= 127 // Subtract 1 from all bytes to move 0 to high bits // bitwise or with self to catch all > 127 bytes - // mask off high bits and check if 0 + // mask off non high bits and check if 0 [MethodImpl(MethodImplOptions.AggressiveInlining)] // Needs a push private static bool CheckBytesInAsciiRange(long check) @@ -531,5 +550,39 @@ private static bool CheckBytesInAsciiRange(short check) private static bool CheckBytesInAsciiRange(sbyte check) => check > 0; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] // Needs a push + private static bool CheckBytesNotNull(Vector check) + { + // Vectorized byte range check, signed byte != null + return !Vector.EqualsAny(check, Vector.Zero); + } + + // Validate: bytes != 0 + // Subtract 1 from all bytes to move 0 to high bits + // bitwise and with ~check so high bits are only set for bytes that were originally 0 + // mask off non high bits and check if 0 + + [MethodImpl(MethodImplOptions.AggressiveInlining)] // Needs a push + private static bool CheckBytesNotNull(long check) + { + const long HighBits = unchecked((long)0x8080808080808080L); + return ((check - 0x0101010101010101L) & ~check & HighBits) == 0; + } + + private static bool CheckBytesNotNull(int check) + { + const int HighBits = unchecked((int)0x80808080); + return ((check - 0x01010101) & ~check & HighBits) == 0; + } + + private static bool CheckBytesNotNull(short check) + { + const short HighBits = unchecked((short)0x8080); + return ((check - 0x0101) & ~check & HighBits) == 0; + } + + private static bool CheckBytesNotNull(sbyte check) + => check != 0; } } diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index f67795d52c21..66e6ada42b1a 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -985,7 +985,7 @@ public unsafe void Append(Span name, Span value) }} // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); + var valueStr = value.GetRequestHeaderString(_useLatin1); if ((_bits & flag) == 0) {{ // We didn't already have a header set, so add a new one. @@ -1003,7 +1003,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the ""known"" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetAsciiOrUTF8StringNonNullCharacters(_useLatin1); + var valueStr = value.GetRequestHeaderString(_useLatin1); AppendUnknownHeaders(name, valueStr); }} }}" : "")} From 6db7e3b054eb514928b7536a2bf347aadb3202f6 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 10 Jan 2020 15:43:16 -0800 Subject: [PATCH 4/7] Cleanup --- ...t.AspNetCore.Server.Kestrel.Core.Manual.cs | 5 +++-- .../Internal/Http/HttpHeaders.Generated.cs | 4 ++-- .../Core/src/Internal/Http/HttpProtocol.cs | 2 +- .../src/Internal/Http/HttpRequestHeaders.cs | 7 +------ .../Internal/Infrastructure/HttpUtilities.cs | 19 +++++-------------- src/Servers/Kestrel/Core/test/UTF8Decoding.cs | 4 ++-- .../BytesToStringBenchmark.cs | 2 +- src/Servers/Kestrel/shared/KnownHeaders.cs | 4 ++-- .../Http2/Http2TestBase.cs | 2 +- 9 files changed, 18 insertions(+), 31 deletions(-) diff --git a/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs b/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs index a0d91f86060b..de4e0a615ea3 100644 --- a/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs +++ b/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs @@ -879,7 +879,7 @@ public void ThrowRequestTargetRejected(System.Span target) { } } internal sealed partial class HttpRequestHeaders : Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpHeaders { - public HttpRequestHeaders(bool reuseHeaderValues = true) { } + public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false) { } public bool HasConnection { get { throw null; } } public bool HasTransferEncoding { get { throw null; } } public Microsoft.Extensions.Primitives.StringValues HeaderAccept { get { throw null; } set { } } @@ -1614,7 +1614,6 @@ internal static partial class HttpUtilities public const string Http2Version = "HTTP/2"; public const string HttpsUriScheme = "https://"; public const string HttpUriScheme = "http://"; - public static string GetAsciiOrUTF8StringNonNullCharacters(this System.Span span) { throw null; } public static string GetAsciiStringEscaped(this System.Span span, int maxChars) { throw null; } public static string GetAsciiStringNonNullCharacters(this System.Span span) { throw null; } public static string GetHeaderName(this System.Span span) { throw null; } @@ -1624,6 +1623,7 @@ internal static partial class HttpUtilities public static Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod GetKnownMethod(string value) { throw null; } [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]internal unsafe static Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpVersion GetKnownVersion(byte* location, int length) { throw null; } [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]public static bool GetKnownVersion(this System.Span span, out Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpVersion knownVersion, out byte length) { throw null; } + public static string GetRequestHeaderStringNonNullCharacters(this System.Span span, bool useLatin1) { throw null; } public static bool IsHostHeaderValid(string hostText) { throw null; } public static string MethodToString(Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod method) { throw null; } public static string SchemeToString(Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpScheme scheme) { throw null; } @@ -1683,6 +1683,7 @@ public StringUtilities() { } public static bool BytesOrdinalEqualsStringAndAscii(string previousValue, System.Span newValue) { throw null; } public static string ConcatAsHexSuffix(string str, char separator, uint number) { throw null; } public unsafe static bool TryGetAsciiString(byte* input, char* output, int count) { throw null; } + public unsafe static bool TryGetLatin1String(byte* input, char* output, int count) { throw null; } } internal partial class TimeoutControl : Microsoft.AspNetCore.Server.Kestrel.Core.Features.IConnectionTimeoutFeature, Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure.ITimeoutControl { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs index 5215875b3838..aa22d87a8631 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -6105,7 +6105,7 @@ public unsafe void Append(Span name, Span value) } // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetRequestHeaderString(_useLatin1); + var valueStr = value.GetRequestHeaderStringNonNullCharacters(_useLatin1); if ((_bits & flag) == 0) { // We didn't already have a header set, so add a new one. @@ -6123,7 +6123,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the "known" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetRequestHeaderString(_useLatin1); + var valueStr = value.GetRequestHeaderStringNonNullCharacters(_useLatin1); AppendUnknownHeaders(name, valueStr); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 6ed1558b18d2..f2d1564334bb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -517,7 +517,7 @@ public void OnTrailer(Span name, Span value) } string key = name.GetHeaderName(); - var valueStr = value.GetRequestHeaderString(ServerOptions.Latin1RequestHeaders); + var valueStr = value.GetRequestHeaderStringNonNullCharacters(ServerOptions.Latin1RequestHeaders); RequestTrailers.Append(key, valueStr); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 7f24397d2b93..af631f644249 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -18,11 +18,6 @@ internal sealed partial class HttpRequestHeaders : HttpHeaders private readonly bool _useLatin1; private long _previousBits = 0; - public HttpRequestHeaders(bool reuseHeaderValues = true) - : this(reuseHeaderValues, useLatin1: false) - { - } - public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false) { _reuseHeaderValues = reuseHeaderValues; @@ -87,7 +82,7 @@ private void AppendContentLength(Span value) parsed < 0 || consumed != value.Length) { - BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetRequestHeaderString(_useLatin1)); + BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value.GetRequestHeaderStringNonNullCharacters(_useLatin1)); } _contentLength = parsed; diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs index 2e3b0d8293f1..04fd9711d711 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs @@ -120,7 +120,7 @@ public static unsafe string GetAsciiStringNonNullCharacters(this Span span fixed (char* output = asciiString) fixed (byte* buffer = span) { - // This version if AsciiUtilities returns null if there are any null (0 byte) characters + // StringUtilities.TryGetAsciiString returns null if there are any null (0 byte) characters // in the string if (!StringUtilities.TryGetAsciiString(buffer, output, span.Length)) { @@ -130,7 +130,7 @@ public static unsafe string GetAsciiStringNonNullCharacters(this Span span return asciiString; } - public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) + private static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) { if (span.IsEmpty) { @@ -166,7 +166,7 @@ public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this Span span) + private static unsafe string GetLatin1StringNonNullCharacters(this Span span) { if (span.IsEmpty) { @@ -189,17 +189,8 @@ public static unsafe string GetLatin1StringNonNullCharacters(this Span spa return resultString; } - public static unsafe string GetRequestHeaderString(this Span span, bool useLatin1) - { - if (useLatin1) - { - return GetLatin1StringNonNullCharacters(span); - } - else - { - return GetAsciiOrUTF8StringNonNullCharacters(span); - } - } + public static string GetRequestHeaderStringNonNullCharacters(this Span span, bool useLatin1) => + useLatin1 ? GetLatin1StringNonNullCharacters(span) : GetAsciiOrUTF8StringNonNullCharacters(span); public static string GetAsciiStringEscaped(this Span span, int maxChars) { diff --git a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs index 532e13781d99..4c4e37740047 100644 --- a/src/Servers/Kestrel/Core/test/UTF8Decoding.cs +++ b/src/Servers/Kestrel/Core/test/UTF8Decoding.cs @@ -17,7 +17,7 @@ public class UTF8DecodingTests [InlineData(new byte[] { 0xef, 0xbf, 0xbd })] // 3 bytes: Replacement character, highest UTF-8 character currently encoded in the UTF-8 code page private void FullUTF8RangeSupported(byte[] encodedBytes) { - var s = encodedBytes.AsSpan().GetAsciiOrUTF8StringNonNullCharacters(); + var s = encodedBytes.AsSpan().GetRequestHeaderStringNonNullCharacters(useLatin1: false); Assert.Equal(1, s.Length); } @@ -35,7 +35,7 @@ private void ExceptionThrownForZeroOrNonAscii(byte[] bytes) var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray(); Array.Copy(bytes, 0, byteRange, position, bytes.Length); - Assert.Throws(() => byteRange.AsSpan().GetAsciiOrUTF8StringNonNullCharacters()); + Assert.Throws(() => byteRange.AsSpan().GetRequestHeaderStringNonNullCharacters(useLatin1: false)); } } } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs index 9341da2b0534..cd6d19b75c91 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/BytesToStringBenchmark.cs @@ -67,7 +67,7 @@ public void Utf8BytesToString() { for (uint i = 0; i < Iterations; i++) { - HttpUtilities.GetAsciiOrUTF8StringNonNullCharacters(_utf8Bytes); + HttpUtilities.GetRequestHeaderStringNonNullCharacters(_utf8Bytes, useLatin1: false); } } diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index 66e6ada42b1a..b2f3292f3415 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -985,7 +985,7 @@ public unsafe void Append(Span name, Span value) }} // We didn't have a previous matching header value, or have already added a header, so get the string for this value. - var valueStr = value.GetRequestHeaderString(_useLatin1); + var valueStr = value.GetRequestHeaderStringNonNullCharacters(_useLatin1); if ((_bits & flag) == 0) {{ // We didn't already have a header set, so add a new one. @@ -1003,7 +1003,7 @@ public unsafe void Append(Span name, Span value) // The header was not one of the ""known"" headers. // Convert value to string first, because passing two spans causes 8 bytes stack zeroing in // this method with rep stosd, which is slower than necessary. - var valueStr = value.GetRequestHeaderString(_useLatin1); + var valueStr = value.GetRequestHeaderStringNonNullCharacters(_useLatin1); AppendUnknownHeaders(name, valueStr); }} }}" : "")} diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 5b875bffab16..fb756b511171 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -402,7 +402,7 @@ public override void Dispose() void IHttpHeadersHandler.OnHeader(Span name, Span value) { - _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(); + _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetRequestHeaderStringNonNullCharacters(useLatin1: false); } void IHttpHeadersHandler.OnHeadersComplete() { } From 15dc9ae22f253683a4a5ccbc60411c4edb66953d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 10 Jan 2020 17:03:55 -0800 Subject: [PATCH 5/7] Fix Vector.Widen logic to not fill first byte --- .../Internal/Infrastructure/StringUtilities.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs index fd1e3a0daea8..268d77a0e452 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/StringUtilities.cs @@ -203,16 +203,17 @@ public static unsafe bool TryGetLatin1String(byte* input, char* output, int coun // do/while as entry condition already checked do { - var vector = Unsafe.AsRef>(input); + // Use byte/ushort instead of signed equivalents to ensure it doesn't fill based on the high bit. + var vector = Unsafe.AsRef>(input); isValid &= CheckBytesNotNull(vector); Vector.Widen( vector, - out Unsafe.AsRef>(output), - out Unsafe.AsRef>(output + Vector.Count)); + out Unsafe.AsRef>(output), + out Unsafe.AsRef>(output + Vector.Count)); - input += Vector.Count; - output += Vector.Count; - } while (input <= end - Vector.Count); + input += Vector.Count; + output += Vector.Count; + } while (input <= end - Vector.Count); // Vector path done, loop back to do non-Vector // If is a exact multiple of vector size, bail now @@ -552,10 +553,10 @@ private static bool CheckBytesInAsciiRange(sbyte check) => check > 0; [MethodImpl(MethodImplOptions.AggressiveInlining)] // Needs a push - private static bool CheckBytesNotNull(Vector check) + private static bool CheckBytesNotNull(Vector check) { // Vectorized byte range check, signed byte != null - return !Vector.EqualsAny(check, Vector.Zero); + return !Vector.EqualsAny(check, Vector.Zero); } // Validate: bytes != 0 From 9a4d28be9f55d463cda85ac1b3ff025226b81a8f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 10 Jan 2020 17:56:33 -0800 Subject: [PATCH 6/7] Add tests --- ...t.AspNetCore.Server.Kestrel.Core.Manual.cs | 2 + .../Kestrel/Core/src/KestrelServerOptions.cs | 2 +- .../Core/test/HttpRequestHeadersTests.cs | 90 ++++++++++++++++++- .../test/KestrelConfigurationBuilderTests.cs | 21 +++++ .../InMemory.FunctionalTests/RequestTests.cs | 63 +++++++++++++ 5 files changed, 174 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs b/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs index de4e0a615ea3..4dec82338a3a 100644 --- a/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs +++ b/src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.Manual.cs @@ -40,6 +40,7 @@ public partial class KestrelServerOptions { internal System.Security.Cryptography.X509Certificates.X509Certificate2 DefaultCertificate { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } internal bool IsDevCertLoaded { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + internal bool Latin1RequestHeaders { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } internal System.Collections.Generic.List ListenOptions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } internal void ApplyDefaultCert(Microsoft.AspNetCore.Server.Kestrel.Https.HttpsConnectionAdapterOptions httpsOptions) { } internal void ApplyEndpointDefaults(Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions listenOptions) { } @@ -433,6 +434,7 @@ public ConfigurationReader(Microsoft.Extensions.Configuration.IConfiguration con public System.Collections.Generic.IDictionary Certificates { get { throw null; } } public Microsoft.AspNetCore.Server.Kestrel.Core.Internal.EndpointDefaults EndpointDefaults { get { throw null; } } public System.Collections.Generic.IEnumerable Endpoints { get { throw null; } } + public bool Latin1RequestHeaders { get { throw null; } } } internal partial class HttpConnectionContext { diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index a3c79dba377a..35a5b0bd49a9 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -93,7 +93,7 @@ public class KestrelServerOptions internal bool IsDevCertLoaded { get; set; } /// - /// Treat request headers as Latin-1 or ISO/IEC 8859-1 instead of UTF-8. Still reject ASCII control characters. + /// Treat request headers as Latin-1 or ISO/IEC 8859-1 instead of UTF-8. /// internal bool Latin1RequestHeaders { get; set; } diff --git a/src/Servers/Kestrel/Core/test/HttpRequestHeadersTests.cs b/src/Servers/Kestrel/Core/test/HttpRequestHeadersTests.cs index a7603b9190fd..d87f1f814fea 100644 --- a/src/Servers/Kestrel/Core/test/HttpRequestHeadersTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpRequestHeadersTests.cs @@ -318,6 +318,7 @@ public void AppendThrowsWhenHeaderNameContainsNonASCIICharacters() public void ValueReuseOnlyWhenAllowed(bool reuseValue, KnownHeader header) { const string HeaderValue = "Hello"; + var headers = new HttpRequestHeaders(reuseHeaderValues: reuseValue); for (var i = 0; i < 6; i++) @@ -336,14 +337,14 @@ public void ValueReuseOnlyWhenAllowed(bool reuseValue, KnownHeader header) Assert.Equal(values.PrevHeaderValue, values.NextHeaderValue); if (reuseValue) { - // When materalized string is reused previous and new should be the same object + // When materialized string is reused previous and new should be the same object Assert.Same(values.PrevHeaderValue, values.NextHeaderValue); } else { - // When materalized string is not reused previous and new should be the different objects + // When materialized string is not reused previous and new should be the different objects Assert.NotSame(values.PrevHeaderValue, values.NextHeaderValue); - } + } } } @@ -483,6 +484,89 @@ public void ValueReuseLatin1NotConfusedForUtf16AndStillRejected(bool reuseValue, } } + [Theory] + [MemberData(nameof(KnownRequestHeaders))] + public void Latin1ValuesAcceptedInLatin1ModeButNotReused(bool reuseValue, KnownHeader header) + { + var headers = new HttpRequestHeaders(reuseHeaderValues: reuseValue, useLatin1: true); + + var headerValue = new char[127]; // 64 + 32 + 16 + 8 + 4 + 2 + 1 + for (var i = 0; i < headerValue.Length; i++) + { + headerValue[i] = 'a'; + } + + for (var i = 0; i < headerValue.Length; i++) + { + // Set non-ascii Latin char that is valid Utf16 when widened; but not a valid utf8 -> utf16 conversion. + headerValue[i] = '\u00a3'; + + for (var mode = 0; mode <= 1; mode++) + { + string headerValueUtf16Latin1CrossOver; + if (mode == 0) + { + // Full length + headerValueUtf16Latin1CrossOver = new string(headerValue); + } + else + { + // Truncated length (to ensure different paths from changing lengths in matching) + headerValueUtf16Latin1CrossOver = new string(headerValue.AsSpan().Slice(0, i + 1)); + } + + headers.Reset(); + + var headerName = Encoding.ASCII.GetBytes(header.Name).AsSpan(); + var latinValueSpan = Encoding.GetEncoding("iso-8859-1").GetBytes(headerValueUtf16Latin1CrossOver).AsSpan(); + + Assert.False(latinValueSpan.SequenceEqual(Encoding.ASCII.GetBytes(headerValueUtf16Latin1CrossOver))); + + headers.Append(headerName, latinValueSpan); + headers.OnHeadersComplete(); + var parsedHeaderValue = ((IHeaderDictionary)headers)[header.Name].ToString(); + + Assert.Equal(headerValueUtf16Latin1CrossOver, parsedHeaderValue); + Assert.NotSame(headerValueUtf16Latin1CrossOver, parsedHeaderValue); + } + + // Reset back to Ascii + headerValue[i] = 'a'; + } + } + + [Theory] + [MemberData(nameof(KnownRequestHeaders))] + public void NullCharactersRejectedInUTF8AndLatin1Mode(bool useLatin1, KnownHeader header) + { + var headers = new HttpRequestHeaders(useLatin1: useLatin1); + + var valueArray = new char[127]; // 64 + 32 + 16 + 8 + 4 + 2 + 1 + for (var i = 0; i < valueArray.Length; i++) + { + valueArray[i] = 'a'; + } + + for (var i = 1; i < valueArray.Length; i++) + { + // Set non-ascii Latin char that is valid Utf16 when widened; but not a valid utf8 -> utf16 conversion. + valueArray[i] = '\0'; + string valueString = new string(valueArray); + + headers.Reset(); + + Assert.Throws(() => + { + var headerName = Encoding.ASCII.GetBytes(header.Name).AsSpan(); + var valueSpan = Encoding.ASCII.GetBytes(valueString).AsSpan(); + + headers.Append(headerName, valueSpan); + }); + + valueArray[i] = 'a'; + } + } + [Fact] public void ValueReuseNeverWhenUnknownHeader() { diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs index 5a47957cdf2a..23fb6115518e 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs @@ -456,6 +456,27 @@ private void EndpointConfigSectionCanSetProtocols(string input, HttpProtocols ex Assert.True(ran3); } + [Fact] + public void Latin1RequestHeadersReadFromConfig() + { + var options = CreateServerOptions(); + var config = new ConfigurationBuilder().AddInMemoryCollection().Build(); + + Assert.False(options.Latin1RequestHeaders); + options.Configure(config).Load(); + Assert.False(options.Latin1RequestHeaders); + + options = CreateServerOptions(); + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Latin1RequestHeaders", "true"), + }).Build(); + + Assert.False(options.Latin1RequestHeaders); + options.Configure(config).Load(); + Assert.True(options.Latin1RequestHeaders); + } + private static string GetCertificatePath() { var appData = Environment.GetEnvironmentVariable("APPDATA"); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index ed53a2fca855..c10d9d08a077 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -1653,6 +1653,69 @@ await connection.Receive( } } + [Fact] + public async Task Latin1HeaderValueAcceptedWhenLatin1OptionIsConfigured() + { + var testContext = new TestServiceContext(LoggerFactory); + + testContext.ServerOptions.Latin1RequestHeaders = true; + + await using (var server = new TestServer(context => + { + Assert.Equal("£", context.Request.Headers["X-Test"]); + return Task.CompletedTask; + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + // The StreamBackedTestConnection will encode £ using the "iso-8859-1" aka Latin1 encoding. + // It will be encoded as 0xA3 which isn't valid UTF-8. + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "X-Test: £", + "", + ""); + + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {testContext.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + } + } + + [Fact] + public async Task Latin1HeaderValueRejectedWhenLatin1OptionIsNotConfigured() + { + var testContext = new TestServiceContext(LoggerFactory); + + await using (var server = new TestServer(_ => Task.CompletedTask, testContext)) + { + using (var connection = server.CreateConnection()) + { + // The StreamBackedTestConnection will encode £ using the "iso-8859-1" aka Latin1 encoding. + // It will be encoded as 0xA3 which isn't valid UTF-8. + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "X-Test: £", + "", + ""); + + await connection.ReceiveEnd( + "HTTP/1.1 400 Bad Request", + "Connection: close", + $"Date: {testContext.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + } + } + public static TheoryData HostHeaderData => HttpParsingData.HostHeaderData; } } From 273f48967820c3701164bd40695cd4c70c759fc6 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 10 Jan 2020 18:43:06 -0800 Subject: [PATCH 7/7] Add HTTP/2 tests --- .../Http2/Http2StreamTests.cs | 60 +++++++++++++++++++ .../Http2/Http2TestBase.cs | 2 +- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 4937000db9e1..4b1bced2303c 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -4408,5 +4408,65 @@ await WaitForStreamErrorAsync(1, Http2ErrorCode.NO_ERROR, expectedErrorMessage: Assert.Single(_decodedHeaders); Assert.Equal("Custom Value", _decodedHeaders["CustomName"]); } + + [Fact] + public async Task HEADERS_Received_Latin1_AcceptedWhenLatin1OptionIsConfigured() + { + _serviceContext.ServerOptions.Latin1RequestHeaders = true; + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + // The HPackEncoder will encode £ as 0xA3 aka Latin1 encoding. + new KeyValuePair("X-Test", "£"), + }; + + await InitializeConnectionAsync(context => + { + Assert.Equal("£", context.Request.Headers["X-Test"]); + return Task.CompletedTask; + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM), + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + + _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this); + + Assert.Equal(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + Assert.Equal("0", _decodedHeaders["content-length"]); + } + + [Fact] + public async Task HEADERS_Received_Latin1_RejectedWhenLatin1OptionIsNotConfigured() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + // The HPackEncoder will encode £ as 0xA3 aka Latin1 encoding. + new KeyValuePair("X-Test", "£"), + }; + + await InitializeConnectionAsync(_noopApplication); + + await StartStreamAsync(1, headers, endStream: true); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: true, + expectedLastStreamId: 1, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.BadRequest_MalformedRequestInvalidHeaders); + } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index fb756b511171..d3d7e0e7ae45 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -402,7 +402,7 @@ public override void Dispose() void IHttpHeadersHandler.OnHeader(Span name, Span value) { - _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetRequestHeaderStringNonNullCharacters(useLatin1: false); + _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetRequestHeaderStringNonNullCharacters(useLatin1: _serviceContext.ServerOptions.Latin1RequestHeaders); } void IHttpHeadersHandler.OnHeadersComplete() { }