From 40844544d9255da759c2594dfc65b9ff49a6a6de Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 May 2021 16:33:18 +1200 Subject: [PATCH 1/5] SkipLocalsInit on Kestrel methods with stackalloc --- src/Http/WebUtilities/src/FormPipeReader.cs | 3 ++- .../WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj | 1 + .../Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs | 1 + src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs | 1 + src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 2 ++ .../Core/src/Internal/Infrastructure/KestrelEventSource.cs | 1 + src/Shared/WebEncoders/WebEncoders.cs | 2 ++ src/Shared/runtime/Http2/Hpack/HPackEncoder.cs | 1 + 8 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Http/WebUtilities/src/FormPipeReader.cs b/src/Http/WebUtilities/src/FormPipeReader.cs index a902bdea349e..c28afb0a094c 100644 --- a/src/Http/WebUtilities/src/FormPipeReader.cs +++ b/src/Http/WebUtilities/src/FormPipeReader.cs @@ -332,6 +332,7 @@ private void ThrowValueTooLargeException() throw new InvalidDataException($"Form value length limit {ValueLengthLimit} exceeded."); } + [SkipLocalsInit] private string GetDecodedStringFromReadOnlySequence(in ReadOnlySequence ros) { if (ros.IsSingleSegment) @@ -341,7 +342,7 @@ private string GetDecodedStringFromReadOnlySequence(in ReadOnlySequence ro if (ros.Length < StackAllocThreshold) { - Span buffer = stackalloc byte[(int)ros.Length]; + Span buffer = stackalloc byte[StackAllocThreshold].Slice((int)ros.Length); ros.CopyTo(buffer); return GetDecodedString(buffer); } diff --git a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj index 95b496204d4e..3e3560224e2d 100644 --- a/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj +++ b/src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj @@ -5,6 +5,7 @@ $(DefaultNetCoreTargetFramework) true $(DefineConstants);WebEncoders_In_WebUtilities + true true aspnetcore false diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 0a3e43bd47e2..d0f8e816474b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -96,6 +96,7 @@ private void AppendContentLength(ReadOnlySpan value) } [MethodImpl(MethodImplOptions.NoInlining)] + [SkipLocalsInit] private void AppendContentLengthCustomEncoding(ReadOnlySpan value, Encoding? customEncoding) { if (_contentLength.HasValue) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 28dc06ff310a..f1285ba48727 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -352,6 +352,7 @@ private bool TryValidateAuthorityAndHost(out string hostText) return true; } + [SkipLocalsInit] private bool TryValidatePath(ReadOnlySpan pathSegment) { // Must start with a leading slash diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 0a6470ca6777..b201ca8ad09c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -7,6 +7,7 @@ using System.IO.Pipelines; using System.Net.Http; using System.Net.Http.QPack; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; @@ -739,6 +740,7 @@ private bool TryValidateAuthorityAndHost(out string hostText) return true; } + [SkipLocalsInit] private bool TryValidatePath(ReadOnlySpan pathSegment) { // Must start with a leading slash diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelEventSource.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelEventSource.cs index 8874eb529715..4c87cc2de792 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelEventSource.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelEventSource.cs @@ -301,6 +301,7 @@ protected override void OnEventCommand(EventCommandEventArgs command) } [NonEvent] + [SkipLocalsInit] private unsafe void WriteEvent(int eventId, string? arg1, string? arg2, string? arg3, string? arg4, string? arg5) { const int EventDataCount = 5; diff --git a/src/Shared/WebEncoders/WebEncoders.cs b/src/Shared/WebEncoders/WebEncoders.cs index ccad3bfc9379..2b69d4871df2 100644 --- a/src/Shared/WebEncoders/WebEncoders.cs +++ b/src/Shared/WebEncoders/WebEncoders.cs @@ -8,6 +8,7 @@ #endif using System.Diagnostics; using System.Globalization; +using System.Runtime.CompilerServices; using Microsoft.Extensions.WebEncoders.Sources; #if WebEncoders_In_WebUtilities @@ -343,6 +344,7 @@ public static int GetArraySizeRequiredToEncode(int count) /// /// The binary input to encode. /// The base64url-encoded form of . + [SkipLocalsInit] public static string Base64UrlEncode(ReadOnlySpan input) { if (input.IsEmpty) diff --git a/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs b/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs index 67a61c3c69f3..7ffe4f2778b6 100644 --- a/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs +++ b/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs @@ -4,6 +4,7 @@ #nullable enable using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Text; namespace System.Net.Http.HPack From f3384e8107e53d1120627dd71c2fdfd134ed6282 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 May 2021 16:38:17 +1200 Subject: [PATCH 2/5] Clean up --- src/Shared/runtime/Http2/Hpack/HPackEncoder.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs b/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs index 7ffe4f2778b6..67a61c3c69f3 100644 --- a/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs +++ b/src/Shared/runtime/Http2/Hpack/HPackEncoder.cs @@ -4,7 +4,6 @@ #nullable enable using System.Collections.Generic; using System.Diagnostics; -using System.Runtime.CompilerServices; using System.Text; namespace System.Net.Http.HPack From 734c21aa18fe4b2c2bb636d518f6cd06f947bcc8 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 May 2021 16:56:41 +1200 Subject: [PATCH 3/5] Fixes --- .../Microsoft.AspNetCore.DataProtection.Abstractions.csproj | 1 + src/Http/Routing/src/Matching/DfaMatcher.cs | 2 ++ src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj | 1 + .../src/Microsoft.AspNetCore.Http.Connections.csproj | 3 ++- 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj index 55e687fa18e4..c2983188e440 100644 --- a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj +++ b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj @@ -7,6 +7,7 @@ Microsoft.AspNetCore.DataProtection.IDataProtectionProvider Microsoft.AspNetCore.DataProtection.IDataProtector $(DefaultNetFxTargetFramework);netstandard2.0;$(DefaultNetCoreTargetFramework) true + true true aspnetcore;dataprotection enable diff --git a/src/Http/Routing/src/Matching/DfaMatcher.cs b/src/Http/Routing/src/Matching/DfaMatcher.cs index 9cc6ae2ab94f..106db18bc860 100644 --- a/src/Http/Routing/src/Matching/DfaMatcher.cs +++ b/src/Http/Routing/src/Matching/DfaMatcher.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Patterns; @@ -27,6 +28,7 @@ public DfaMatcher(ILogger logger, EndpointSelector selector, DfaStat _isDefaultEndpointSelector = selector is DefaultEndpointSelector; } + [SkipLocalsInit] public sealed override Task MatchAsync(HttpContext httpContext) { if (httpContext == null) diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index 36c0bacc92f1..10796dab2067 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -7,6 +7,7 @@ Microsoft.AspNetCore.Routing.Route Microsoft.AspNetCore.Routing.RouteCollection $(DefaultNetCoreTargetFramework) true + true true aspnetcore;routing false diff --git a/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj b/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj index 90e1daef0094..d4493eb60205 100644 --- a/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj +++ b/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj @@ -1,9 +1,10 @@ - + Components for providing real-time bi-directional communication across the Web. $(DefaultNetCoreTargetFramework) true + true false enable From 04f49f8a0a05b60db5486ff9347d8fb623956564 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 May 2021 21:24:22 +1200 Subject: [PATCH 4/5] Fix tests --- src/Http/WebUtilities/src/FormPipeReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/WebUtilities/src/FormPipeReader.cs b/src/Http/WebUtilities/src/FormPipeReader.cs index c28afb0a094c..92c340f973ae 100644 --- a/src/Http/WebUtilities/src/FormPipeReader.cs +++ b/src/Http/WebUtilities/src/FormPipeReader.cs @@ -342,7 +342,7 @@ private string GetDecodedStringFromReadOnlySequence(in ReadOnlySequence ro if (ros.Length < StackAllocThreshold) { - Span buffer = stackalloc byte[StackAllocThreshold].Slice((int)ros.Length); + Span buffer = stackalloc byte[StackAllocThreshold].Slice(0, (int)ros.Length); ros.CopyTo(buffer); return GetDecodedString(buffer); } From 006ed9a89ecf3dea158c4d719f1ddff2e94c2dd3 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 12 May 2021 21:24:12 +1200 Subject: [PATCH 5/5] PR feedback --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs | 2 +- src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 2 +- src/Shared/WebEncoders/WebEncoders.cs | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index f1285ba48727..0c7b8470ad6a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -356,7 +356,7 @@ private bool TryValidateAuthorityAndHost(out string hostText) private bool TryValidatePath(ReadOnlySpan pathSegment) { // Must start with a leading slash - if (pathSegment.Length == 0 || pathSegment[0] != '/') + if (pathSegment.IsEmpty || pathSegment[0] != '/') { ResetAndAbort(new ConnectionAbortedException(CoreStrings.FormatHttp2StreamErrorPathInvalid(RawTarget)), Http2ErrorCode.PROTOCOL_ERROR); return false; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index b201ca8ad09c..08f6033f8bfe 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -744,7 +744,7 @@ private bool TryValidateAuthorityAndHost(out string hostText) private bool TryValidatePath(ReadOnlySpan pathSegment) { // Must start with a leading slash - if (pathSegment.Length == 0 || pathSegment[0] != '/') + if (pathSegment.IsEmpty || pathSegment[0] != '/') { Abort(new ConnectionAbortedException(CoreStrings.FormatHttp3StreamErrorPathInvalid(RawTarget)), Http3ErrorCode.ProtocolError); return false; diff --git a/src/Shared/WebEncoders/WebEncoders.cs b/src/Shared/WebEncoders/WebEncoders.cs index 2b69d4871df2..86798884a070 100644 --- a/src/Shared/WebEncoders/WebEncoders.cs +++ b/src/Shared/WebEncoders/WebEncoders.cs @@ -347,6 +347,8 @@ public static int GetArraySizeRequiredToEncode(int count) [SkipLocalsInit] public static string Base64UrlEncode(ReadOnlySpan input) { + const int StackAllocThreshold = 128; + if (input.IsEmpty) { return string.Empty; @@ -355,8 +357,8 @@ public static string Base64UrlEncode(ReadOnlySpan input) int bufferSize = GetArraySizeRequiredToEncode(input.Length); char[]? bufferToReturnToPool = null; - Span buffer = bufferSize <= 128 - ? stackalloc char[bufferSize] + Span buffer = bufferSize <= StackAllocThreshold + ? stackalloc char[StackAllocThreshold] : bufferToReturnToPool = ArrayPool.Shared.Rent(bufferSize); var numBase64Chars = Base64UrlEncode(input, buffer);