From ca2d931982806cb6d089e9254832181469eaa909 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Fri, 6 May 2022 16:14:52 -0700 Subject: [PATCH 01/22] Merge cookies into single string (fixes issue #26461) --- .../Core/src/Internal/Http2/Http2Stream.cs | 13 +++++++++ .../Http2/Http2ConnectionTests.cs | 29 ++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index c69f40bfa2df..96bcf8f2c4e3 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -11,6 +11,8 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Primitives; +using ASCIIEncoding = System.Text.ASCIIEncoding; +using HeaderNames = Microsoft.Net.Http.Headers.HeaderNames; using HttpCharacters = Microsoft.AspNetCore.Http.HttpCharacters; using HttpMethods = Microsoft.AspNetCore.Http.HttpMethods; @@ -201,6 +203,17 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio // Suppress pseudo headers from the public headers collection. HttpRequestHeaders.ClearPseudoRequestHeaders(); + // Cookies should be merged into a single string separated by "; " + StringValues cookies; + var containsCookies = HttpRequestHeaders.Remove(HeaderNames.Cookie, out cookies); + if (containsCookies) + { + var mergeCookies = string.Join("; ", cookies.ToArray()); + var cookiesMergedValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(mergeCookies)); + var cookiesHeaderValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(HeaderNames.Cookie)); + HttpRequestHeaders.Append(cookiesHeaderValueSpan, cookiesMergedValueSpan, false); + } + return true; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index f4dc806cacbb..af095d6d651d 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -2840,6 +2840,33 @@ public async Task HEADERS_Received_RequestLineLength_StreamError() await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } + [Fact] + public async Task HEADERS_CookiesMergedIntoOne() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.Cookie, "a=0"), + new KeyValuePair(HeaderNames.Cookie, "b=1"), + new KeyValuePair(HeaderNames.Cookie, "c=2"), + }; + + await InitializeConnectionAsync(_readHeadersApplication); + + await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM, headers); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 36, + withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM), + withStreamId: 1); + + Assert.Equal("a=0; b=1; c=2", _receivedHeaders["Cookie"]); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task PRIORITY_Received_StreamIdZero_ConnectionError() { @@ -5404,7 +5431,7 @@ public async Task StartConnection_SendNothing_NoError() await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } - public static TheoryData UpperCaseHeaderNameData +public static TheoryData UpperCaseHeaderNameData { get { From f3edf8f253d30aeb415328a6a342350be716ea71 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Fri, 6 May 2022 16:35:13 -0700 Subject: [PATCH 02/22] Implemented feature for http3 but test timesout --- .../Core/src/Internal/Http3/Http3Stream.cs | 13 ++++++ .../Http2/Http2ConnectionTests.cs | 2 +- .../Http3/Http3ConnectionTests.cs | 40 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 91bb4e65a4c2..32cb36c642b0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -18,6 +18,8 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; +using ASCIIEncoding = System.Text.ASCIIEncoding; +using HeaderNames = Microsoft.Net.Http.Headers.HeaderNames; using HttpCharacters = Microsoft.AspNetCore.Http.HttpCharacters; using HttpMethod = Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod; using HttpMethods = Microsoft.AspNetCore.Http.HttpMethods; @@ -907,6 +909,17 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio // Suppress pseudo headers from the public headers collection. HttpRequestHeaders.ClearPseudoRequestHeaders(); + // Cookies should be merged into a single string separated by "; " + StringValues cookies; + var containsCookies = HttpRequestHeaders.Remove(HeaderNames.Cookie, out cookies); + if (containsCookies) + { + var mergeCookies = string.Join("; ", cookies.ToArray()); + var cookiesMergedValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(mergeCookies)); + var cookiesHeaderValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(HeaderNames.Cookie)); + HttpRequestHeaders.Append(cookiesHeaderValueSpan, cookiesMergedValueSpan, false); + } + return true; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index af095d6d651d..f9adba635c4f 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -2862,7 +2862,7 @@ await ExpectAsync(Http2FrameType.HEADERS, withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM), withStreamId: 1); - Assert.Equal("a=0; b=1; c=2", _receivedHeaders["Cookie"]); + Assert.Equal("a=0; b=1; c=2", _receivedHeaders[HeaderNames.Cookie]); await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs index e7dd6999fe3e..412279f5ceff 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs @@ -130,6 +130,46 @@ await Http3Api.InitializeConnectionAsync(async context => await requestStream.ExpectReceiveEndOfStream(); } + [Fact] + public async Task HEADERS_CookiesMergedIntoOne() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.Cookie, "a=0"), + new KeyValuePair(HeaderNames.Cookie, "b=1"), + new KeyValuePair(HeaderNames.Cookie, "c=2"), + }; + + await Http3Api.InitializeConnectionAsync(async context => + { + var buffer = new byte[16 * 1024]; + var received = 0; + + while ((received = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length)) > 0) + { + await context.Response.Body.WriteAsync(buffer, 0, received); + } + }); + + await Http3Api.CreateControlStream(); + await Http3Api.GetInboundControlStream(); + + var requestStream = await Http3Api.CreateRequestStream(); + + await requestStream.SendHeadersAsync(headers); + var frame = await requestStream.ReceiveFrameAsync(); + + await requestStream.SendDataAsync(Encoding.ASCII.GetBytes("Hello world"), endStream: false); + var receivedHeaders = await requestStream.ExpectHeadersAsync(); + + Assert.Equal("a=0; b=1; c=2", receivedHeaders[HeaderNames.Cookie]); + + await requestStream.OnDisposedTask.DefaultTimeout(); + } + [Theory] [InlineData(0, 0)] [InlineData(1, 4)] From e6d6b673f9e05334d8a8e036783ccb9da0fce63d Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Mon, 9 May 2022 10:30:53 -0700 Subject: [PATCH 03/22] Fixed the test in Http3 --- .../Http3/Http3ConnectionTests.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs index 412279f5ceff..09e90ed7beb9 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs @@ -133,7 +133,7 @@ await Http3Api.InitializeConnectionAsync(async context => [Fact] public async Task HEADERS_CookiesMergedIntoOne() { - var headers = new[] + var requestHeaders = new[] { new KeyValuePair(HeaderNames.Method, "GET"), new KeyValuePair(HeaderNames.Path, "/"), @@ -143,11 +143,16 @@ public async Task HEADERS_CookiesMergedIntoOne() new KeyValuePair(HeaderNames.Cookie, "c=2"), }; + var receivedHeaders = ""; + await Http3Api.InitializeConnectionAsync(async context => { var buffer = new byte[16 * 1024]; var received = 0; + // verify that the cookies are all merged into a single string + receivedHeaders = context.Request.Headers[HeaderNames.Cookie]; + while ((received = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length)) > 0) { await context.Response.Body.WriteAsync(buffer, 0, received); @@ -156,18 +161,16 @@ await Http3Api.InitializeConnectionAsync(async context => await Http3Api.CreateControlStream(); await Http3Api.GetInboundControlStream(); - var requestStream = await Http3Api.CreateRequestStream(); - await requestStream.SendHeadersAsync(headers); - var frame = await requestStream.ReceiveFrameAsync(); - - await requestStream.SendDataAsync(Encoding.ASCII.GetBytes("Hello world"), endStream: false); - var receivedHeaders = await requestStream.ExpectHeadersAsync(); - Assert.Equal("a=0; b=1; c=2", receivedHeaders[HeaderNames.Cookie]); + await requestStream.SendHeadersAsync(requestHeaders, endStream: true); + var responseHeaders = await requestStream.ExpectHeadersAsync(); + await requestStream.ExpectReceiveEndOfStream(); await requestStream.OnDisposedTask.DefaultTimeout(); + + Assert.Equal("a=0; b=1; c=2", receivedHeaders); } [Theory] From 6ab232cbf749972ca58bc0465aa2fc5ebac2990d Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Mon, 9 May 2022 11:15:13 -0700 Subject: [PATCH 04/22] Improved design with a cast that avoids raw dictionary operations thereby improving performance and maintainability --- .../Core/src/Internal/Http2/Http2Stream.cs | 15 +++++---------- .../Core/src/Internal/Http3/Http3Stream.cs | 13 ++++--------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 96bcf8f2c4e3..2db5150ff676 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -11,8 +11,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Primitives; -using ASCIIEncoding = System.Text.ASCIIEncoding; -using HeaderNames = Microsoft.Net.Http.Headers.HeaderNames; using HttpCharacters = Microsoft.AspNetCore.Http.HttpCharacters; using HttpMethods = Microsoft.AspNetCore.Http.HttpMethods; @@ -204,14 +202,11 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio HttpRequestHeaders.ClearPseudoRequestHeaders(); // Cookies should be merged into a single string separated by "; " - StringValues cookies; - var containsCookies = HttpRequestHeaders.Remove(HeaderNames.Cookie, out cookies); - if (containsCookies) - { - var mergeCookies = string.Join("; ", cookies.ToArray()); - var cookiesMergedValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(mergeCookies)); - var cookiesHeaderValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(HeaderNames.Cookie)); - HttpRequestHeaders.Append(cookiesHeaderValueSpan, cookiesMergedValueSpan, false); + var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; + if (headers.Cookie.Count > 1) + { + var mergedCookies = string.Join("; ", headers.Cookie.ToArray()); + headers.Cookie = new StringValues(mergedCookies); } return true; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 32cb36c642b0..3743068e13e7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -18,8 +18,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; -using ASCIIEncoding = System.Text.ASCIIEncoding; -using HeaderNames = Microsoft.Net.Http.Headers.HeaderNames; using HttpCharacters = Microsoft.AspNetCore.Http.HttpCharacters; using HttpMethod = Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod; using HttpMethods = Microsoft.AspNetCore.Http.HttpMethods; @@ -910,14 +908,11 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio HttpRequestHeaders.ClearPseudoRequestHeaders(); // Cookies should be merged into a single string separated by "; " - StringValues cookies; - var containsCookies = HttpRequestHeaders.Remove(HeaderNames.Cookie, out cookies); - if (containsCookies) + var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; + if (headers.Cookie.Count > 1) { - var mergeCookies = string.Join("; ", cookies.ToArray()); - var cookiesMergedValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(mergeCookies)); - var cookiesHeaderValueSpan = new ReadOnlySpan(ASCIIEncoding.ASCII.GetBytes(HeaderNames.Cookie)); - HttpRequestHeaders.Append(cookiesHeaderValueSpan, cookiesMergedValueSpan, false); + var mergedCookies = string.Join("; ", headers.Cookie.ToArray()); + headers.Cookie = new StringValues(mergedCookies); } return true; From 5e064dae7ecdc3cd8e0168756e0a6ac35420b269 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Mon, 9 May 2022 11:38:44 -0700 Subject: [PATCH 05/22] Addressed PR comments --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs | 4 ++-- src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 4 ++-- .../InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 2db5150ff676..e556fe3eec81 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -202,11 +202,11 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio HttpRequestHeaders.ClearPseudoRequestHeaders(); // Cookies should be merged into a single string separated by "; " + // https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.5 var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; if (headers.Cookie.Count > 1) { - var mergedCookies = string.Join("; ", headers.Cookie.ToArray()); - headers.Cookie = new StringValues(mergedCookies); + headers.Cookie = string.Join("; ", headers.Cookie.ToArray()); } return true; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 3743068e13e7..ffaa51b11073 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -908,11 +908,11 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio HttpRequestHeaders.ClearPseudoRequestHeaders(); // Cookies should be merged into a single string separated by "; " + // https://datatracker.ietf.org/doc/html/draft-ietf-quic-http-34#section-4.1.1.2 var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; if (headers.Cookie.Count > 1) { - var mergedCookies = string.Join("; ", headers.Cookie.ToArray()); - headers.Cookie = new StringValues(mergedCookies); + headers.Cookie = string.Join("; ", headers.Cookie.ToArray()); } return true; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index f9adba635c4f..6778e02a8c67 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -5431,7 +5431,7 @@ public async Task StartConnection_SendNothing_NoError() await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } -public static TheoryData UpperCaseHeaderNameData + public static TheoryData UpperCaseHeaderNameData { get { From 547eb3966c6435a954c28a91a29f46540ee74553 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Mon, 9 May 2022 14:00:49 -0700 Subject: [PATCH 06/22] removed duplicate blank lines --- .../test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs index 09e90ed7beb9..1a9fe56f1dc7 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs @@ -163,7 +163,6 @@ await Http3Api.InitializeConnectionAsync(async context => await Http3Api.GetInboundControlStream(); var requestStream = await Http3Api.CreateRequestStream(); - await requestStream.SendHeadersAsync(requestHeaders, endStream: true); var responseHeaders = await requestStream.ExpectHeadersAsync(); From 30aab36d5ceadf85e957c3614312a02cbf1ec4d0 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 10:35:46 -0700 Subject: [PATCH 07/22] Moved cookie merging into generated HttpHeaders file and removed performance impacting cast --- .../Core/src/Internal/Http/HttpHeaders.Generated.cs | 11 ++++++++++- .../Core/src/Internal/Http/HttpRequestHeaders.cs | 2 +- .../Kestrel/Core/src/Internal/Http2/Http2Stream.cs | 6 +----- .../Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 6 +----- src/Servers/Kestrel/shared/KnownHeaders.cs | 9 +++++++++ 5 files changed, 22 insertions(+), 12 deletions(-) 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 febe7dc04a83..88ee59819525 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -7925,6 +7925,15 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec } } + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + public void MergeCookies() + { + if (_headers._Cookie.Count > 1) + { + _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); // TODO string.Join is less than ideal + } + } + [MethodImpl(MethodImplOptions.AggressiveOptimization)] public unsafe bool TryQPackAppend(int index, ReadOnlySpan value, bool checkForNewlineChars) { @@ -17394,4 +17403,4 @@ public bool MoveNext() } } } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 635a0b3276fc..c1ac7de56afa 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -132,7 +132,7 @@ private void SetValueUnknown(string key, StringValues value) [MethodImpl(MethodImplOptions.NoInlining)] private bool AddValueUnknown(string key, StringValues value) - { + { Unknown.Add(GetInternedHeaderName(key), value); // Return true, above will throw and exit for false return true; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index e556fe3eec81..ccdc21f9cf9a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -203,11 +203,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio // Cookies should be merged into a single string separated by "; " // https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.5 - var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; - if (headers.Cookie.Count > 1) - { - headers.Cookie = string.Join("; ", headers.Cookie.ToArray()); - } + HttpRequestHeaders.MergeCookies(); return true; } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index ffaa51b11073..503e4120b08f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -909,11 +909,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio // Cookies should be merged into a single string separated by "; " // https://datatracker.ietf.org/doc/html/draft-ietf-quic-http-34#section-4.1.1.2 - var headers = (AspNetCore.Http.IHeaderDictionary)HttpRequestHeaders; - if (headers.Cookie.Count > 1) - { - headers.Cookie = string.Join("; ", headers.Cookie.ToArray()); - } + HttpRequestHeaders.MergeCookies(); return true; } diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index 94981b7b8bd6..aecfb12ef493 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -1328,6 +1328,15 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec }} }} + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + public void MergeCookies() + {{ + if (_headers._Cookie.Count > 1) + {{ + _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); // TODO string.Join is less than ideal + }} + }} + [MethodImpl(MethodImplOptions.AggressiveOptimization)] public unsafe bool TryQPackAppend(int index, ReadOnlySpan value, bool checkForNewlineChars) {{ From 0ce8ac421926d4defa7d44c1a25211e12b5967fd Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 10:38:38 -0700 Subject: [PATCH 08/22] Removed redundant space --- .../Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index c1ac7de56afa..635a0b3276fc 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -132,7 +132,7 @@ private void SetValueUnknown(string key, StringValues value) [MethodImpl(MethodImplOptions.NoInlining)] private bool AddValueUnknown(string key, StringValues value) - { + { Unknown.Add(GetInternedHeaderName(key), value); // Return true, above will throw and exit for false return true; From 69182cda43b86ee75e5dbbbba253763b873c2dcd Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 13:12:34 -0700 Subject: [PATCH 09/22] Removed comment about string.Join performance --- .../Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs | 4 ++-- src/Servers/Kestrel/shared/KnownHeaders.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 88ee59819525..a622fee2d2c0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -7930,7 +7930,7 @@ public void MergeCookies() { if (_headers._Cookie.Count > 1) { - _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); // TODO string.Join is less than ideal + _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); } } @@ -17403,4 +17403,4 @@ public bool MoveNext() } } } -} +} \ No newline at end of file diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index aecfb12ef493..6a3343ad7b07 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -1333,7 +1333,7 @@ public void MergeCookies() {{ if (_headers._Cookie.Count > 1) {{ - _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); // TODO string.Join is less than ideal + _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); }} }} From c1febaecce1a5844d2d98ec87e97279d35ff7ac4 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 14:58:02 -0700 Subject: [PATCH 10/22] Implemented some optimizations but with little improvement --- .../Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs | 6 +++--- src/Servers/Kestrel/shared/KnownHeaders.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) 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 a622fee2d2c0..bf64b0e743df 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -7925,10 +7925,10 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec } } - [MethodImpl(MethodImplOptions.AggressiveOptimization)] + [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] public void MergeCookies() { - if (_headers._Cookie.Count > 1) + if ((_bits & 0x20000L) != 0 && _headers._Cookie.Count > 1) { _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); } @@ -17403,4 +17403,4 @@ public bool MoveNext() } } } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index 6a3343ad7b07..dba7ce93afcc 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -1328,10 +1328,10 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec }} }} - [MethodImpl(MethodImplOptions.AggressiveOptimization)] + [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] public void MergeCookies() {{ - if (_headers._Cookie.Count > 1) + if ((_bits & 0x20000L) != 0 && _headers._Cookie.Count > 1) {{ _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); }} From 312955b7a92a36937e21dc90b14886f2bf58b732 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 19:58:15 -0700 Subject: [PATCH 11/22] unhardcoded the bit comparison for checking if cookies are present --- .../Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs | 5 +++-- src/Servers/Kestrel/shared/KnownHeaders.cs | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) 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 bf64b0e743df..32a0e0e7fcdb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -383,6 +383,7 @@ internal partial class HttpRequestHeaders : IHeaderDictionary private HeaderReferences _headers; public bool HasConnection => (_bits & 0x2L) != 0; + public bool HasCookie => (_bits & 0x20000L) != 0; public bool HasTransferEncoding => (_bits & 0x20000000000L) != 0; public int HostCount => _headers._Host.Count; @@ -7928,7 +7929,7 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] public void MergeCookies() { - if ((_bits & 0x20000L) != 0 && _headers._Cookie.Count > 1) + if (HasCookie && _headers._Cookie.Count > 1) { _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); } @@ -17403,4 +17404,4 @@ public bool MoveNext() } } } -} +} \ No newline at end of file diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index dba7ce93afcc..09687d73964c 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -101,6 +101,7 @@ static KnownHeaders() }; var requestHeadersExistence = new[] { + HeaderNames.Cookie, HeaderNames.Connection, HeaderNames.TransferEncoding, }; @@ -1331,7 +1332,7 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] public void MergeCookies() {{ - if ((_bits & 0x20000L) != 0 && _headers._Cookie.Count > 1) + if (HasCookie && _headers._Cookie.Count > 1) {{ _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); }} From d009f4740d86fc39001911a9be5698fce32ab570 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Tue, 10 May 2022 20:03:02 -0700 Subject: [PATCH 12/22] Removed aggressive optimization flags --- .../Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs | 2 +- src/Servers/Kestrel/shared/KnownHeaders.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 32a0e0e7fcdb..d4efd248a82b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -7926,7 +7926,7 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec } } - [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void MergeCookies() { if (HasCookie && _headers._Cookie.Count > 1) diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index 09687d73964c..62ed25ff9ba3 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -1329,7 +1329,7 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec }} }} - [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void MergeCookies() {{ if (HasCookie && _headers._Cookie.Count > 1) From 678794345343a4ccb1b4b557ed2c29301dfdb6d7 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Wed, 11 May 2022 11:16:51 -0700 Subject: [PATCH 13/22] As per Stephen's suggestion, I moved the MergeCookies function out of the generated class. --- .../Core/src/Internal/Http/HttpHeaders.Generated.cs | 9 --------- .../Core/src/Internal/Http/HttpRequestHeaders.cs | 11 +++++++++++ src/Servers/Kestrel/shared/KnownHeaders.cs | 9 --------- 3 files changed, 11 insertions(+), 18 deletions(-) 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 d4efd248a82b..838b0463ab51 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -7926,15 +7926,6 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec } } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void MergeCookies() - { - if (HasCookie && _headers._Cookie.Count > 1) - { - _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); - } - } - [MethodImpl(MethodImplOptions.AggressiveOptimization)] public unsafe bool TryQPackAppend(int index, ReadOnlySpan value, bool checkForNewlineChars) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 635a0b3276fc..1e44a3d50f12 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -48,6 +48,17 @@ public void OnHeadersComplete() Clear(headersToClear); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void MergeCookies() + { + if (HasCookie && _headers._Cookie.Count > 1) + { + { + _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); + } + } + } + protected override void ClearFast() { if (!ReuseHeaderValues) diff --git a/src/Servers/Kestrel/shared/KnownHeaders.cs b/src/Servers/Kestrel/shared/KnownHeaders.cs index 62ed25ff9ba3..c04451b3f913 100644 --- a/src/Servers/Kestrel/shared/KnownHeaders.cs +++ b/src/Servers/Kestrel/shared/KnownHeaders.cs @@ -1329,15 +1329,6 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan value, bool chec }} }} - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void MergeCookies() - {{ - if (HasCookie && _headers._Cookie.Count > 1) - {{ - _headers._Cookie = string.Join(""; "", _headers._Cookie.ToArray()); - }} - }} - [MethodImpl(MethodImplOptions.AggressiveOptimization)] public unsafe bool TryQPackAppend(int index, ReadOnlySpan value, bool checkForNewlineChars) {{ From ec4a483327bb6076630a508287270d60f2cdd220 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Wed, 11 May 2022 11:22:04 -0700 Subject: [PATCH 14/22] removed unnecessary brackets. Not sure where they came form. --- .../Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs index 1e44a3d50f12..472f1c477107 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs @@ -53,9 +53,7 @@ public void MergeCookies() { if (HasCookie && _headers._Cookie.Count > 1) { - { - _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); - } + _headers._Cookie = string.Join("; ", _headers._Cookie.ToArray()); } } From 4702f408674ca7a4422074f133cdc3e796d6d742 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Wed, 11 May 2022 16:50:10 -0700 Subject: [PATCH 15/22] Fixed Benchmark (feat. Stephen) --- .../Http2/Http2ConnectionBenchmarkBase.cs | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 8c44390a9aeb..ba1c8345ec71 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -36,15 +36,17 @@ public abstract class Http2ConnectionBenchmarkBase private int _currentStreamId; private byte[] _headersBuffer; private DuplexPipe.DuplexPipePair _connectionPair; - private Http2Frame _httpFrame; private int _dataWritten; + private Task _requestProcessingTask; + + private Http2Frame _receiveHttpFrame = new(); + private Http2Frame _sendHttpFrame = new(); protected abstract Task ProcessRequest(HttpContext httpContext); public virtual void GlobalSetup() { _memoryPool = PinnedBlockMemoryPoolFactory.Create(); - _httpFrame = new Http2Frame(); var options = new PipeOptions(_memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); @@ -79,7 +81,7 @@ public virtual void GlobalSetup() _currentStreamId = 1; - _ = _connection.ProcessRequestsAsync(new DummyApplication(ProcessRequest, new MockHttpContextFactory())); + _requestProcessingTask = _connection.ProcessRequestsAsync(new DummyApplication(ProcessRequest, new MockHttpContextFactory())); _connectionPair.Application.Output.Write(Http2Connection.ClientPreface); _connectionPair.Application.Output.WriteSettings(new Http2PeerSettings @@ -89,12 +91,12 @@ public virtual void GlobalSetup() _connectionPair.Application.Output.FlushAsync().GetAwaiter().GetResult(); // Read past connection setup frames - ReceiveFrameAsync(_connectionPair.Application.Input, _httpFrame).GetAwaiter().GetResult(); - Debug.Assert(_httpFrame.Type == Http2FrameType.SETTINGS); - ReceiveFrameAsync(_connectionPair.Application.Input, _httpFrame).GetAwaiter().GetResult(); - Debug.Assert(_httpFrame.Type == Http2FrameType.WINDOW_UPDATE); - ReceiveFrameAsync(_connectionPair.Application.Input, _httpFrame).GetAwaiter().GetResult(); - Debug.Assert(_httpFrame.Type == Http2FrameType.SETTINGS); + ReceiveFrameAsync(_connectionPair.Application.Input).GetAwaiter().GetResult(); + Debug.Assert(_receiveHttpFrame.Type == Http2FrameType.SETTINGS); + ReceiveFrameAsync(_connectionPair.Application.Input).GetAwaiter().GetResult(); + Debug.Assert(_receiveHttpFrame.Type == Http2FrameType.WINDOW_UPDATE); + ReceiveFrameAsync(_connectionPair.Application.Input).GetAwaiter().GetResult(); + Debug.Assert(_receiveHttpFrame.Type == Http2FrameType.SETTINGS); } [Benchmark] @@ -102,32 +104,32 @@ public async Task MakeRequest() { _requestHeadersEnumerator.Initialize(_httpRequestHeaders); _requestHeadersEnumerator.MoveNext(); - _connectionPair.Application.Output.WriteStartStream(streamId: _currentStreamId, _hpackEncoder, _requestHeadersEnumerator, _headersBuffer, endStream: true, frame: _httpFrame); + _connectionPair.Application.Output.WriteStartStream(streamId: _currentStreamId, _hpackEncoder, _requestHeadersEnumerator, _headersBuffer, endStream: true, frame: _sendHttpFrame); await _connectionPair.Application.Output.FlushAsync(); while (true) { - await ReceiveFrameAsync(_connectionPair.Application.Input, _httpFrame); + await ReceiveFrameAsync(_connectionPair.Application.Input); - if (_httpFrame.StreamId != _currentStreamId && _httpFrame.StreamId != 0) + if (_receiveHttpFrame.StreamId != _currentStreamId && _receiveHttpFrame.StreamId != 0) { - throw new Exception($"Unexpected stream ID: {_httpFrame.StreamId}"); + throw new Exception($"Unexpected stream ID: {_receiveHttpFrame.StreamId}"); } - if (_httpFrame.Type == Http2FrameType.DATA) + if (_receiveHttpFrame.Type == Http2FrameType.DATA) { - _dataWritten += _httpFrame.DataPayloadLength; + _dataWritten += _receiveHttpFrame.DataPayloadLength; } if (_dataWritten > 1024 * 32) { - _connectionPair.Application.Output.WriteWindowUpdateAsync(streamId: 0, _dataWritten, _httpFrame); + _connectionPair.Application.Output.WriteWindowUpdateAsync(streamId: 0, _dataWritten, _sendHttpFrame); await _connectionPair.Application.Output.FlushAsync(); _dataWritten = 0; } - if ((_httpFrame.HeadersFlags & Http2HeadersFrameFlags.END_STREAM) == Http2HeadersFrameFlags.END_STREAM) + if ((_receiveHttpFrame.HeadersFlags & Http2HeadersFrameFlags.END_STREAM) == Http2HeadersFrameFlags.END_STREAM) { break; } @@ -136,7 +138,7 @@ public async Task MakeRequest() _currentStreamId += 2; } - internal async ValueTask ReceiveFrameAsync(PipeReader pipeReader, Http2Frame frame, uint maxFrameSize = Http2PeerSettings.DefaultMaxFrameSize) + internal async ValueTask ReceiveFrameAsync(PipeReader pipeReader, uint maxFrameSize = Http2PeerSettings.DefaultMaxFrameSize) { while (true) { @@ -147,7 +149,7 @@ internal async ValueTask ReceiveFrameAsync(PipeReader pipeReader, Http2Frame fra try { - if (Http2FrameReader.TryReadFrame(ref buffer, frame, maxFrameSize, out var framePayload)) + if (Http2FrameReader.TryReadFrame(ref buffer, _receiveHttpFrame, maxFrameSize, out var framePayload)) { consumed = examined = framePayload.End; return; @@ -170,9 +172,10 @@ internal async ValueTask ReceiveFrameAsync(PipeReader pipeReader, Http2Frame fra } [GlobalCleanup] - public void Dispose() + public async ValueTask DisposeAsync() { _connectionPair.Application.Output.Complete(); + await _requestProcessingTask; _memoryPool?.Dispose(); } } From 5e2e5094160542ac3328a388fe186469499451a8 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Wed, 11 May 2022 17:11:46 -0700 Subject: [PATCH 16/22] Added cookies params to the benchmark --- .../Http2/Http2ConnectionBenchmarkBase.cs | 12 ++++++++++++ .../Http2/Http2ConnectionEmptyBenchmark.cs | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index ba1c8345ec71..d46dbbebfc67 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -44,6 +44,9 @@ public abstract class Http2ConnectionBenchmarkBase protected abstract Task ProcessRequest(HttpContext httpContext); + [Params(0, 1, 3)] + public int numCookies { get; set; } + public virtual void GlobalSetup() { _memoryPool = PinnedBlockMemoryPoolFactory.Create(); @@ -58,6 +61,15 @@ public virtual void GlobalSetup() _httpRequestHeaders[HeaderNames.Scheme] = new StringValues("http"); _httpRequestHeaders[HeaderNames.Authority] = new StringValues("localhost:80"); + if (numCookies > 0) { + var cookies = new StringValues("0=1"); + for (int i = 1; i < numCookies; i++) + { + cookies.Append($"{i}={i + 1}"); + } + _httpRequestHeaders[HeaderNames.Cookie] = cookies; + } + _headersBuffer = new byte[1024 * 16]; _hpackEncoder = new DynamicHPackEncoder(); diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionEmptyBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionEmptyBenchmark.cs index 79b647597060..04d395f58ddc 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionEmptyBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionEmptyBenchmark.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; public class Http2ConnectionBenchmark : Http2ConnectionBenchmarkBase { - [Params(0, 128, 1024)] + [Params(0)] public int ResponseDataLength { get; set; } private string _responseData; From 3d6de08fc04f491d0717e122f7369b11f369c4d2 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Wed, 11 May 2022 19:38:52 -0700 Subject: [PATCH 17/22] Fixed formatting as per VS lightbulbs --- .../Http2/Http2ConnectionBenchmarkBase.cs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index d46dbbebfc67..9053baa2da03 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -1,25 +1,17 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; -using System.Buffers.Binary; using System.Diagnostics; -using System.IO; using System.IO.Pipelines; -using System.Linq; using System.Net.Http.HPack; -using System.Threading.Tasks; using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; -using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Http2HeadersEnumerator = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2HeadersEnumerator; @@ -39,13 +31,13 @@ public abstract class Http2ConnectionBenchmarkBase private int _dataWritten; private Task _requestProcessingTask; - private Http2Frame _receiveHttpFrame = new(); - private Http2Frame _sendHttpFrame = new(); + private readonly Http2Frame _receiveHttpFrame = new(); + private readonly Http2Frame _sendHttpFrame = new(); protected abstract Task ProcessRequest(HttpContext httpContext); [Params(0, 1, 3)] - public int numCookies { get; set; } + public int NumCookies { get; set; } public virtual void GlobalSetup() { @@ -61,15 +53,16 @@ public virtual void GlobalSetup() _httpRequestHeaders[HeaderNames.Scheme] = new StringValues("http"); _httpRequestHeaders[HeaderNames.Authority] = new StringValues("localhost:80"); - if (numCookies > 0) { + if (NumCookies > 0) + { var cookies = new StringValues("0=1"); - for (int i = 1; i < numCookies; i++) + for (var index = 1; index < NumCookies; index++) { - cookies.Append($"{i}={i + 1}"); + _ = cookies.Append($"{index}={index + 1}"); } _httpRequestHeaders[HeaderNames.Cookie] = cookies; } - + _headersBuffer = new byte[1024 * 16]; _hpackEncoder = new DynamicHPackEncoder(); From d0a3169f3869741a60d249d4074d7d51c076ef1f Mon Sep 17 00:00:00 2001 From: Daniel-Genkin-MS-2 <104801075+Daniel-Genkin-MS-2@users.noreply.github.com> Date: Thu, 12 May 2022 09:32:31 -0700 Subject: [PATCH 18/22] Store returned instance as per Chris' recommendation Co-authored-by: Chris Ross --- .../perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 9053baa2da03..ad70565b935f 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -58,7 +58,7 @@ public virtual void GlobalSetup() var cookies = new StringValues("0=1"); for (var index = 1; index < NumCookies; index++) { - _ = cookies.Append($"{index}={index + 1}"); + cookies = cookies.Append($"{index}={index + 1}"); } _httpRequestHeaders[HeaderNames.Cookie] = cookies; } From ed7452bd27b07ef3e29ab7ab9b3ddf7ff9e8a24b Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Thu, 12 May 2022 10:28:27 -0700 Subject: [PATCH 19/22] Fixed the build error --- .../perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index ad70565b935f..9008a5d963d7 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -58,7 +58,7 @@ public virtual void GlobalSetup() var cookies = new StringValues("0=1"); for (var index = 1; index < NumCookies; index++) { - cookies = cookies.Append($"{index}={index + 1}"); + cookies = (StringValues)cookies.Append($"{index}={index + 1}"); } _httpRequestHeaders[HeaderNames.Cookie] = cookies; } From 28e767da379219f4f925047b1f101e5db065a124 Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Thu, 12 May 2022 10:55:05 -0700 Subject: [PATCH 20/22] Stephen's suggestion to convert the IEnumerator to an array and implicitly convert to StringValues rather than via the cast --- .../perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 9008a5d963d7..60472278ed8b 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -58,7 +58,7 @@ public virtual void GlobalSetup() var cookies = new StringValues("0=1"); for (var index = 1; index < NumCookies; index++) { - cookies = (StringValues)cookies.Append($"{index}={index + 1}"); + cookies = cookies.Append($"{index}={index + 1}").ToArray(); } _httpRequestHeaders[HeaderNames.Cookie] = cookies; } From 3d4dc1585b654a3e430bd26feeb7a2deb67c97bd Mon Sep 17 00:00:00 2001 From: Daniel Genkin Date: Thu, 12 May 2022 15:00:25 -0700 Subject: [PATCH 21/22] Improved performance of the loop that builds cookies --- .../Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 60472278ed8b..03f188b99463 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -55,10 +55,10 @@ public virtual void GlobalSetup() if (NumCookies > 0) { - var cookies = new StringValues("0=1"); + var cookies = new string[NumCookies]; for (var index = 1; index < NumCookies; index++) { - cookies = cookies.Append($"{index}={index + 1}").ToArray(); + cookies[index] = $"{index}={index + 1}"; } _httpRequestHeaders[HeaderNames.Cookie] = cookies; } From 404d3eee259bae90eb216629bea83fed5dd21a05 Mon Sep 17 00:00:00 2001 From: Daniel-Genkin-MS-2 <104801075+Daniel-Genkin-MS-2@users.noreply.github.com> Date: Thu, 12 May 2022 15:09:05 -0700 Subject: [PATCH 22/22] Update src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs Co-authored-by: Stephen Halter --- .../perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 03f188b99463..5471aaf5cd8e 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -56,7 +56,7 @@ public virtual void GlobalSetup() if (NumCookies > 0) { var cookies = new string[NumCookies]; - for (var index = 1; index < NumCookies; index++) + for (var index = 0; index < NumCookies; index++) { cookies[index] = $"{index}={index + 1}"; }