From a10d9eef611a2902765181ac0703259d1bcf35a0 Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 20 Apr 2024 17:59:26 +0200 Subject: [PATCH 01/27] Initial implementation --- .../src/Internal/Http2/Http2FrameWriter.cs | 83 +++--- .../Core/test/Http2/Http2HPackEncoderTests.cs | 93 +++++-- .../Kestrel/shared/HPackHeaderWriter.cs | 41 ++- .../test/PipeWriterHttp2FrameExtensions.cs | 6 +- .../Http2/Http2StreamTests.cs | 243 ++++++++++++++++-- .../Http2/Http2TestBase.cs | 6 +- 6 files changed, 386 insertions(+), 86 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 802c68c4126f..c31defd1269a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -71,8 +71,8 @@ internal sealed class Http2FrameWriter // This is only set to true by tests. private readonly bool _scheduleInline; - private uint _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; - private byte[] _headerEncodingBuffer; + private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; + private readonly ArrayBufferWriter _headerEncodingBuffer; private long _unflushedBytes; private bool _completed; @@ -107,7 +107,7 @@ public Http2FrameWriter( _flusher = new TimingPipeFlusher(timeoutControl, serviceContext.Log); _flusher.Initialize(_outputWriter); _outgoingFrame = new Http2Frame(); - _headerEncodingBuffer = new byte[_maxFrameSize]; + _headerEncodingBuffer = new ArrayBufferWriter(_maxFrameSize); _scheduleInline = serviceContext.Scheduler == PipeScheduler.Inline; @@ -373,8 +373,9 @@ public void UpdateMaxFrameSize(uint maxFrameSize) { if (_maxFrameSize != maxFrameSize) { - _maxFrameSize = maxFrameSize; - _headerEncodingBuffer = new byte[_maxFrameSize]; + // Safe cast, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. + // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 + _maxFrameSize = (int)maxFrameSize; } } } @@ -509,8 +510,11 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht { _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); - var buffer = _headerEncodingBuffer.AsSpan(); + _headerEncodingBuffer.ResetWrittenCount(); + var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + Debug.Assert(done != HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); + _headerEncodingBuffer.Advance(payloadLength); FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. @@ -548,10 +552,18 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in try { - _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_STREAM, streamId); - var buffer = _headerEncodingBuffer.AsSpan(); - var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + HeaderWriteResult done = HeaderWriteResult.MoreHeaders; + int payloadLength; + do + { + _headersEnumerator.Initialize(headers); + _headerEncodingBuffer.ResetWrittenCount(); + var bufferSize = done == HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; + done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + } while (done == HeaderWriteResult.BufferTooSmall); + _headerEncodingBuffer.Advance(payloadLength); FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. @@ -566,32 +578,45 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, bool done) + private void SplitHeaderFramesToOutput(int streamId, HeaderWriteResult done, bool isFramePrepared) { - var buffer = _headerEncodingBuffer.AsSpan(); - _outgoingFrame.PayloadLength = payloadLength; - if (done) + var dataToFrame = _headerEncodingBuffer.WrittenSpan; + var shouldPrepareFrame = !isFramePrepared; + while (dataToFrame.Length > 0) { - _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; - } - - WriteHeaderUnsynchronized(); - _outputWriter.Write(buffer.Slice(0, payloadLength)); - - while (!done) - { - _outgoingFrame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); - - done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); - _outgoingFrame.PayloadLength = payloadLength; + if (shouldPrepareFrame) + { + _outgoingFrame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); + } + else + { + shouldPrepareFrame = true; + } - if (done) + var currentSize = dataToFrame.Length > _maxFrameSize ? _maxFrameSize : dataToFrame.Length; + _outgoingFrame.PayloadLength = currentSize; + if (done == HeaderWriteResult.Done && dataToFrame.Length == currentSize) { - _outgoingFrame.ContinuationFlags = Http2ContinuationFrameFlags.END_HEADERS; + _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; } WriteHeaderUnsynchronized(); - _outputWriter.Write(buffer.Slice(0, payloadLength)); + _outputWriter.Write(dataToFrame[..currentSize]); + dataToFrame = dataToFrame.Slice(currentSize); + } + } + + private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HeaderWriteResult done) + { + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + while (done != HeaderWriteResult.Done) + { + _headerEncodingBuffer.ResetWrittenCount(); + var bufferSize = done == HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; + done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + _headerEncodingBuffer.Advance(payloadLength); + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: false); } } @@ -1023,4 +1048,4 @@ private void EnqueueWaitingForMoreConnectionWindow(Http2OutputProducer producer) _http2Connection.Abort(new ConnectionAbortedException("HTTP/2 connection exceeded the outgoing flow control maximum queue size.")); } } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index e65d33eaf0fd..2832e60187ed 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -29,7 +29,7 @@ public void BeginEncodeHeaders_Status302_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -52,7 +52,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(5, length - 5).ToArray(); var hex = BitConverter.ToString(result); @@ -81,7 +81,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // First response enumerator.Initialize(headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -123,7 +123,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Second response enumerator.Initialize(headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -164,7 +164,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -225,7 +225,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // First response enumerator.Initialize((HttpResponseHeaders)headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -267,7 +267,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Second response enumerator.Initialize(headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -308,7 +308,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -366,7 +366,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(maxHeaderTableSize: Http2PeerSettings.DefaultHeaderTableSize); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); if (neverIndex) { @@ -392,7 +392,7 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Empty(GetHeaderEntries(hpackEncoder)); } @@ -482,11 +482,11 @@ public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair()); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(2, length); @@ -600,11 +600,68 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // Second request enumerator.Initialize(new Dictionary()); - Assert.True(HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); Assert.Equal(0, length); } + [Fact] + public void WithStatusCode_TooLargeHeader_ReturnsNotDone() + { + Span buffer = new byte[1024 * 16]; + + IHeaderDictionary headers = new HttpResponseHeaders(); + headers.Cookie = new string('a', buffer.Length + 1); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + } + + [Fact] + public void NoStatusCodeLargeHeader_ReturnsOversized() + { + Span buffer = new byte[1024 * 16]; + + IHeaderDictionary headers = new HttpResponseHeaders(); + headers.Cookie = new string('a', buffer.Length + 1); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + } + + [Fact] + public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() + { + Span buffer = new byte[1024 * 16]; + + IHeaderDictionary headers = new HttpResponseHeaders(); + headers.Cookie = new string('a', buffer.Length - 1); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + } + + [Fact] + public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() + { + Span buffer = new byte[1024 * 16]; + + IHeaderDictionary headers = new HttpResponseHeaders(); + headers.Accept = "application/json;"; + headers.Cookie = new string('a', buffer.Length - 1); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + } + private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) { var groupedHeaders = headers diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 22cf2256cd53..e992637bb0db 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -10,6 +10,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; #endif +internal enum HeaderWriteResult : byte +{ + // Not all headers written. + MoreHeaders = 0, + + // All headers written. + Done = 1, + + // Oversized header for the given buffer. + BufferTooSmall = 2, +} + // This file is used by Kestrel to write response headers and tests to write request headers. // To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs. internal static class HPackHeaderWriter @@ -17,7 +29,7 @@ internal static class HPackHeaderWriter /// /// Begin encoding headers in the first HEADERS frame. /// - public static bool BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { length = 0; @@ -35,12 +47,12 @@ public static bool BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackE if (!headersEnumerator.MoveNext()) { - return true; + return HeaderWriteResult.Done; } // We're ok with not throwing if no headers were encoded because we've already encoded the status. // There is a small chance that the header will encode if there is no other content in the next HEADERS frame. - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), throwIfNoneEncoded: false, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, out var headersLength); length += headersLength; return done; } @@ -48,7 +60,7 @@ public static bool BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackE /// /// Begin encoding headers in the first HEADERS frame. /// - public static bool BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { length = 0; @@ -60,10 +72,10 @@ public static bool BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2Hea if (!headersEnumerator.MoveNext()) { - return true; + return HeaderWriteResult.Done; } - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), throwIfNoneEncoded: true, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); length += headersLength; return done; } @@ -71,9 +83,9 @@ public static bool BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2Hea /// /// Continue encoding headers in the next HEADERS frame. The enumerator should already have a current value. /// - public static bool ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { - return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, throwIfNoneEncoded: true, out length); + return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, canRequestLargerBuffer: true, out length); } private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpackEncoder, Span buffer, out int length) @@ -91,7 +103,7 @@ private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpack } } - private static bool EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool throwIfNoneEncoded, out int length) + private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool canRequestLargerBuffer, out int length) { var currentLength = 0; do @@ -115,14 +127,15 @@ private static bool EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2Hea out var headerLength)) { // If the header wasn't written, and no headers have been written, then the header is too large. - // Throw an error to avoid an infinite loop of attempting to write large header. - if (currentLength == 0 && throwIfNoneEncoded) + // Request for a larger buffer to write large header. + if (currentLength == 0 && canRequestLargerBuffer) { - throw new HPackEncodingException(SR.net_http_hpack_encode_failure); + length = 0; + return HeaderWriteResult.BufferTooSmall; } length = currentLength; - return false; + return HeaderWriteResult.MoreHeaders; } currentLength += headerLength; @@ -130,7 +143,7 @@ private static bool EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2Hea while (headersEnumerator.MoveNext()); length = currentLength; - return true; + return HeaderWriteResult.Done; } private static HeaderEncodingHint ResolveHeaderEncodingHint(int staticTableId, string name) diff --git a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs index 84ead387bc89..c95cfd7d67a7 100644 --- a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs +++ b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs @@ -36,7 +36,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, out var length); frame.PayloadLength = length; - if (done) + if (done == Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) { frame.HeadersFlags = Http2HeadersFrameFlags.END_HEADERS; } @@ -49,14 +49,14 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami Http2FrameWriter.WriteHeader(frame, writer); writer.Write(buffer.Slice(0, length)); - while (!done) + while (done != Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) { frame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, out length); frame.PayloadLength = length; - if (done) + if (done == Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) { frame.ContinuationFlags = Http2ContinuationFrameFlags.END_HEADERS; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 1e5fb54fa0de..95282dd8ffb5 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2593,17 +2593,18 @@ await ExpectAsync(Http2FrameType.DATA, } [Fact] - public async Task ResponseTrailers_TooLong_Throws() + public async Task ResponseTrailers_TooLong_SplitsTrailersToContinuationFrames() { + var trailerValue = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); await InitializeConnectionAsync(async context => { await context.Response.WriteAsync("Hello World"); - context.Response.AppendTrailer("too_long", new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize)); + context.Response.AppendTrailer("too_long", trailerValue); }); await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + await ExpectAsync(Http2FrameType.HEADERS, withLength: 32, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); @@ -2613,18 +2614,24 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 1); - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); + var trailerFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 1); - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + var trailierContinuation = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 13, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); - _pair.Application.Output.Complete(); - await _connectionTask; + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); - var message = Assert.Single(LogMessages, m => m.Exception is HPackEncodingException); - Assert.Contains(SR.net_http_hpack_encode_failure, message.Exception.Message); + var buffer = new byte[trailerFrame.PayloadLength + trailierContinuation.PayloadLength]; + trailerFrame.PayloadSequence.CopyTo(buffer); + trailierContinuation.PayloadSequence.CopyTo(buffer.AsSpan(trailerFrame.PayloadLength)); + _hpackDecoder.Decode(buffer, endHeaders: true, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(trailerValue, _decodedHeaders["too_long"]); } [Fact] @@ -3183,28 +3190,226 @@ await ExpectAsync(Http2FrameType.DATA, } [Fact] - public async Task ResponseWithHeadersTooLarge_AbortsConnection() + public async Task ResponseWithHeaderValueTooLarge_SplitsHeaderToContinuationFrames() { - var appFinished = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await InitializeConnectionAsync(async context => + { + context.Response.Headers.ETag = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame3 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 5, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + + var temp = new byte[headersFrame.PayloadSequence.Length + headersFrame2.PayloadSequence.Length + headersFrame3.PayloadSequence.Length]; + headersFrame.PayloadSequence.CopyTo(temp.AsSpan()); + headersFrame2.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length)); + headersFrame3.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length + (int)headersFrame2.PayloadSequence.Length)); + + _hpackDecoder.Decode(temp, endHeaders: true, handler: this); + Assert.Equal((int)Http2PeerSettings.DefaultMaxFrameSize, _decodedHeaders[HeaderNames.ETag].Length); + } + + [Fact] + public async Task TooLargeHeaderFollowedByContinuationHeaders_Split() + { await InitializeConnectionAsync(async context => { - context.Response.Headers["too_long"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + context.Response.Headers.ETag = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + context.Response.Headers.TE = new string('a', 30); await context.Response.WriteAsync("Hello World"); }); await StartStreamAsync(1, _browserRequestHeaders, endStream: true); // Just the StatusCode gets written before aborting in the continuation frame - await ExpectAsync(Http2FrameType.HEADERS, + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame3 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 40, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + + var temp = new byte[headersFrame.PayloadSequence.Length + headersFrame2.PayloadSequence.Length + headersFrame3.PayloadSequence.Length]; + headersFrame.PayloadSequence.CopyTo(temp.AsSpan()); + headersFrame2.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length)); + headersFrame3.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length + (int)headersFrame2.PayloadSequence.Length)); + + _hpackDecoder.Decode(temp, endHeaders: true, handler: this); + Assert.Equal((int)Http2PeerSettings.DefaultMaxFrameSize, _decodedHeaders[HeaderNames.ETag].Length); + Assert.Equal(30, _decodedHeaders[HeaderNames.TE].Length); + } + + [Fact] + public async Task TwoTooLargeHeaderFollowedByContinuationHeaders_Split() + { + await InitializeConnectionAsync(async context => + { + context.Response.Headers.ETag = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + context.Response.Headers.TE = new string('b', (int)Http2PeerSettings.DefaultMaxFrameSize); + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + var frames = new Http2FrameWithPayload[5]; + frames[0] = await ExpectAsync(Http2FrameType.HEADERS, withLength: 32, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); + frames[1] = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + frames[2] = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 5, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + frames[3] = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + frames[4] = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 7, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + + var totalSize = frames.Sum(x => x.PayloadSequence.Length); + var temp = new byte[totalSize]; + var destinationIndex = 0; + for (var i = 0; i < frames.Length; i++) + { + frames[i].PayloadSequence.CopyTo(temp.AsSpan(destinationIndex)); + destinationIndex += (int)frames[i].PayloadSequence.Length; + } + _hpackDecoder.Decode(temp, endHeaders: true, handler: this); + Assert.Equal((int)Http2PeerSettings.DefaultMaxFrameSize, _decodedHeaders[HeaderNames.ETag].Length); + Assert.Equal((int)Http2PeerSettings.DefaultMaxFrameSize, _decodedHeaders[HeaderNames.TE].Length); + } + + [Fact] + public async Task ClientRequestedLargerFrame_HeadersSplitByRequestedSize() + { + _clientSettings.MaxFrameSize = 17000; + _serviceContext.ServerOptions.Limits.Http2.MaxFrameSize = 17001; + await InitializeConnectionAsync(async context => + { + context.Response.Headers.ETag = new string('a', 17002); + await context.Response.WriteAsync("Hello World"); + }, expectedSettingsCount: 5); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame1 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 17000, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 8, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + } + + [Fact] + public async Task ResponseWithMultipleHeaderValueTooLargeForFrame_SplitsHeaderToContinuationFrames() + { + await InitializeConnectionAsync(async context => + { + // This size makes it fit to a single header, but not next to the response status etc. + context.Response.Headers.ETag = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize - 20); + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16369, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + + var temp = new byte[headersFrame.PayloadSequence.Length + headersFrame2.PayloadSequence.Length]; + headersFrame.PayloadSequence.CopyTo(temp.AsSpan()); + headersFrame2.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length)); + + _hpackDecoder.Decode(temp, endHeaders: true, handler: this); + Assert.Equal((int)Http2PeerSettings.DefaultMaxFrameSize - 20, _decodedHeaders[HeaderNames.ETag].Length); + } + + [Fact] + public async Task ResponseWithHeaderNameTooLarge_SplitsHeaderToContinuationFrames() + { + var longHeaderName = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + var headerValue = "some value"; + await InitializeConnectionAsync(async context => + { + context.Response.Headers[longHeaderName] = headerValue; + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame3 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 15, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); - _pair.Application.Output.Complete(); + var temp = new byte[headersFrame.PayloadSequence.Length + headersFrame2.PayloadSequence.Length + headersFrame3.PayloadSequence.Length]; + headersFrame.PayloadSequence.CopyTo(temp.AsSpan()); + headersFrame2.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length)); + headersFrame3.PayloadSequence.CopyTo(temp.AsSpan((int)headersFrame.PayloadSequence.Length + (int)headersFrame2.PayloadSequence.Length)); - await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: int.MaxValue, Http2ErrorCode.INTERNAL_ERROR, - SR.net_http_hpack_encode_failure); + _hpackDecoder.Decode(temp, endHeaders: true, handler: this); + Assert.Equal(headerValue, _decodedHeaders[longHeaderName]); } [Fact] diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 70f47cf2e62b..0f837823b996 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -849,7 +849,7 @@ internal async Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done; + return done == HeaderWriteResult.Done; } internal Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags flags, IEnumerable> headers) @@ -919,7 +919,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done; + return done == HeaderWriteResult.Done; } internal async Task SendContinuationAsync(int streamId, Http2ContinuationFrameFlags flags, byte[] payload) @@ -947,7 +947,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done; + return done == HeaderWriteResult.Done; } internal Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) From 5cad996272b0207851473208e0d59e8929196b03 Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 25 Apr 2024 18:08:53 +0200 Subject: [PATCH 02/27] Review feedback --- .../src/Internal/Http2/Http2FrameWriter.cs | 24 +++++----- .../Core/test/Http2/Http2HPackEncoderTests.cs | 44 +++++++++---------- .../Kestrel/shared/HPackHeaderWriter.cs | 24 +++++----- .../test/PipeWriterHttp2FrameExtensions.cs | 6 +-- .../Http2/Http2TestBase.cs | 6 +-- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index c31defd1269a..ee78a548e077 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -511,9 +511,9 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; + var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); - Debug.Assert(done != HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); + Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); _headerEncodingBuffer.Advance(payloadLength); FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); } @@ -553,16 +553,16 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in try { _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_STREAM, streamId); - HeaderWriteResult done = HeaderWriteResult.MoreHeaders; + var done = HPackHeaderWriter.HeaderWriteResult.MoreHeaders; int payloadLength; do { _headersEnumerator.Initialize(headers); _headerEncodingBuffer.ResetWrittenCount(); - var bufferSize = done == HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; + var bufferSize = done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); - } while (done == HeaderWriteResult.BufferTooSmall); + } while (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall); _headerEncodingBuffer.Advance(payloadLength); FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); } @@ -578,7 +578,7 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void SplitHeaderFramesToOutput(int streamId, HeaderWriteResult done, bool isFramePrepared) + private void SplitHeaderFramesToOutput(int streamId, HPackHeaderWriter.HeaderWriteResult done, bool isFramePrepared) { var dataToFrame = _headerEncodingBuffer.WrittenSpan; var shouldPrepareFrame = !isFramePrepared; @@ -593,9 +593,9 @@ private void SplitHeaderFramesToOutput(int streamId, HeaderWriteResult done, boo shouldPrepareFrame = true; } - var currentSize = dataToFrame.Length > _maxFrameSize ? _maxFrameSize : dataToFrame.Length; + var currentSize = Math.Min(dataToFrame.Length, _maxFrameSize); _outgoingFrame.PayloadLength = currentSize; - if (done == HeaderWriteResult.Done && dataToFrame.Length == currentSize) + if (done == HPackHeaderWriter.HeaderWriteResult.Done && dataToFrame.Length == currentSize) { _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; } @@ -606,13 +606,13 @@ private void SplitHeaderFramesToOutput(int streamId, HeaderWriteResult done, boo } } - private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HeaderWriteResult done) + private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult done) { SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); - while (done != HeaderWriteResult.Done) + while (done != HPackHeaderWriter.HeaderWriteResult.Done) { _headerEncodingBuffer.ResetWrittenCount(); - var bufferSize = done == HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; + var bufferSize = done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); _headerEncodingBuffer.Advance(payloadLength); diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index 2832e60187ed..448f6c387d0a 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -29,7 +29,7 @@ public void BeginEncodeHeaders_Status302_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -52,7 +52,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(5, length - 5).ToArray(); var hex = BitConverter.ToString(result); @@ -81,7 +81,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // First response enumerator.Initialize(headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -123,7 +123,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Second response enumerator.Initialize(headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -164,7 +164,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -225,7 +225,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // First response enumerator.Initialize((HttpResponseHeaders)headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -267,7 +267,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Second response enumerator.Initialize(headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -308,7 +308,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -366,7 +366,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(maxHeaderTableSize: Http2PeerSettings.DefaultHeaderTableSize); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); if (neverIndex) { @@ -392,7 +392,7 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Empty(GetHeaderEntries(hpackEncoder)); } @@ -482,11 +482,11 @@ public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair()); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(2, length); @@ -600,7 +600,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // Second request enumerator.Initialize(new Dictionary()); - Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); Assert.Equal(0, length); } @@ -616,7 +616,7 @@ public void WithStatusCode_TooLargeHeader_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); } [Fact] @@ -630,7 +630,7 @@ public void NoStatusCodeLargeHeader_ReturnsOversized() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); } [Fact] @@ -644,7 +644,7 @@ public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); } [Fact] @@ -659,7 +659,7 @@ public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); } private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index e992637bb0db..71503772601b 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -10,22 +10,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; #endif -internal enum HeaderWriteResult : byte -{ - // Not all headers written. - MoreHeaders = 0, - - // All headers written. - Done = 1, - - // Oversized header for the given buffer. - BufferTooSmall = 2, -} - // This file is used by Kestrel to write response headers and tests to write request headers. // To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs. internal static class HPackHeaderWriter { + internal enum HeaderWriteResult : byte + { + // Not all headers written. + MoreHeaders = 0, + + // All headers written. + Done = 1, + + // Oversized header for the given buffer. + BufferTooSmall = 2, + } + /// /// Begin encoding headers in the first HEADERS frame. /// diff --git a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs index c95cfd7d67a7..52a168ba696b 100644 --- a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs +++ b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs @@ -36,7 +36,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, out var length); frame.PayloadLength = length; - if (done == Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) + if (done == HPackHeaderWriter.HeaderWriteResult.Done) { frame.HeadersFlags = Http2HeadersFrameFlags.END_HEADERS; } @@ -49,14 +49,14 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami Http2FrameWriter.WriteHeader(frame, writer); writer.Write(buffer.Slice(0, length)); - while (done != Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) + while (done != HPackHeaderWriter.HeaderWriteResult.Done) { frame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, out length); frame.PayloadLength = length; - if (done == Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult.Done) + if (done == HPackHeaderWriter.HeaderWriteResult.Done) { frame.ContinuationFlags = Http2ContinuationFrameFlags.END_HEADERS; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 0f837823b996..4542e01e0f2b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -849,7 +849,7 @@ internal async Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HeaderWriteResult.Done; + return done == HPackHeaderWriter.HeaderWriteResult.Done; } internal Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags flags, IEnumerable> headers) @@ -919,7 +919,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HeaderWriteResult.Done; + return done == HPackHeaderWriter.HeaderWriteResult.Done; } internal async Task SendContinuationAsync(int streamId, Http2ContinuationFrameFlags flags, byte[] payload) @@ -947,7 +947,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HeaderWriteResult.Done; + return done == HPackHeaderWriter.HeaderWriteResult.Done; } internal Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) From f53dd7cc11d1f1d93f1d893212f7f0b567be5169 Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 25 Apr 2024 21:02:06 +0200 Subject: [PATCH 03/27] Updating a comment --- src/Servers/Kestrel/shared/HPackHeaderWriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 71503772601b..2d3ba5a7442e 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -50,7 +50,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE return HeaderWriteResult.Done; } - // We're ok with not throwing if no headers were encoded because we've already encoded the status. + // We're ok with not increasing the buffer size if no headers were encoded because we've already encoded the status. // There is a small chance that the header will encode if there is no other content in the next HEADERS frame. var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, out var headersLength); length += headersLength; From 231c7165674a86b419f7736c0fab1e7da6ac6c5c Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 27 Apr 2024 20:01:26 +0200 Subject: [PATCH 04/27] Minor perf opt when all headers fit in a single header frame. --- .../Core/src/Internal/Http2/Http2FrameWriter.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index ee78a548e077..d12443ff0b6f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -514,8 +514,20 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); - _headerEncodingBuffer.Advance(payloadLength); - FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); + if (done == HPackHeaderWriter.HeaderWriteResult.Done) + { + // Fast path + _outgoingFrame.PayloadLength = payloadLength; + _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; + WriteHeaderUnsynchronized(); + _outputWriter.Write(buffer[0..payloadLength]); + } + else + { + // Slow path + _headerEncodingBuffer.Advance(payloadLength); + FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); + } } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. // Since we allow custom header encoders we don't know what type of exceptions to expect. From 887f5051e8a1e2055bfdf0e4d853f60dd45f16f6 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 1 May 2024 21:16:17 +0200 Subject: [PATCH 05/27] Adding MaxResponseHeadersLimit to H2 framewriter --- .../src/Internal/Http2/Http2FrameWriter.cs | 93 +++++++-- .../Kestrel/Core/src/KestrelServerLimits.cs | 34 ++++ .../Kestrel/Core/src/PublicAPI.Unshipped.txt | 2 + .../Core/test/Http2/Http2FrameWriterTests.cs | 20 +- .../Http2/Http2FrameWriterBenchmark.cs | 18 +- .../Http2/Http2StreamTests.cs | 189 ++++++++++++++++++ .../HttpClientHttp2InteropTests.cs | 13 +- 7 files changed, 348 insertions(+), 21 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index d12443ff0b6f..391e33f400ea 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -5,6 +5,7 @@ using System.Buffers.Binary; using System.Diagnostics; using System.IO.Pipelines; +using System.Net.Http; using System.Net.Http.HPack; using System.Threading.Channels; using Microsoft.AspNetCore.Connections; @@ -73,6 +74,8 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private readonly ArrayBufferWriter _headerEncodingBuffer; + private readonly int? _maxResponseHeadersTotalSize; + private int _currentResponseHeadersTotalSize; private long _unflushedBytes; private bool _completed; @@ -110,7 +113,7 @@ public Http2FrameWriter( _headerEncodingBuffer = new ArrayBufferWriter(_maxFrameSize); _scheduleInline = serviceContext.Scheduler == PipeScheduler.Inline; - + _maxResponseHeadersTotalSize = _http2Connection.Limits.MaxResponseHeadersTotalSize; _hpackEncoder = new DynamicHPackEncoder(serviceContext.ServerOptions.AllowResponseHeaderCompression); _maximumFlowControlQueueSize = AppContextMaximumFlowControlQueueSize is null @@ -508,15 +511,21 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht { try { + // In the case of the headers, there is always a status header to be returned, so BeginEncodeHeaders will not return BufferTooSmall. _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); + if (_maxResponseHeadersTotalSize.HasValue && payloadLength > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + _currentResponseHeadersTotalSize = payloadLength; if (done == HPackHeaderWriter.HeaderWriteResult.Done) { - // Fast path + // Fast path, only a single HEADER frame. _outgoingFrame.PayloadLength = payloadLength; _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; WriteHeaderUnsynchronized(); @@ -524,9 +533,10 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht } else { - // Slow path + // More headers sent in CONTINUATION frames. _headerEncodingBuffer.Advance(payloadLength); - FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + FinishWritingHeadersUnsynchronized(streamId); } } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. @@ -564,19 +574,46 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in try { + // In the case of the trailers, there is no status header to be written, so even the first call to BeginEncodeHeaders can return BufferTooSmall. _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_STREAM, streamId); - var done = HPackHeaderWriter.HeaderWriteResult.MoreHeaders; - int payloadLength; + var bufferSize = _headerEncodingBuffer.Capacity; + HPackHeaderWriter.HeaderWriteResult done; do { _headersEnumerator.Initialize(headers); _headerEncodingBuffer.ResetWrittenCount(); - var bufferSize = done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + if (done == HPackHeaderWriter.HeaderWriteResult.Done) + { + if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + payloadLength > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + _headerEncodingBuffer.Advance(payloadLength); + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + } + else if (done == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) + { + // More headers sent in CONTINUATION frames. + _currentResponseHeadersTotalSize += payloadLength; + if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + _headerEncodingBuffer.Advance(payloadLength); + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + FinishWritingHeadersUnsynchronized(streamId); + } + else + { + if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + bufferSize > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + bufferSize *= 2; + } } while (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall); - _headerEncodingBuffer.Advance(payloadLength); - FinishWritingHeadersUnsynchronized(streamId, payloadLength, done); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. // Since we allow custom header encoders we don't know what type of exceptions to expect. @@ -618,18 +655,36 @@ private void SplitHeaderFramesToOutput(int streamId, HPackHeaderWriter.HeaderWri } } - private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult done) + private void FinishWritingHeadersUnsynchronized(int streamId) { - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); - while (done != HPackHeaderWriter.HeaderWriteResult.Done) + HPackHeaderWriter.HeaderWriteResult done; + var bufferSize = _headerEncodingBuffer.Capacity; + do { _headerEncodingBuffer.ResetWrittenCount(); - var bufferSize = done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall ? _headerEncodingBuffer.Capacity * 2 : _headerEncodingBuffer.Capacity; var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; - done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); - _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: false); - } + done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + + if (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + { + if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + bufferSize > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + bufferSize *= 2; + } + else + { + // In case of Done or MoreHeaders: write to output. + _currentResponseHeadersTotalSize += payloadLength; + if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize > _maxResponseHeadersTotalSize.Value) + { + ThrowResponseHeadersLimitException(); + } + _headerEncodingBuffer.Advance(payloadLength); + SplitHeaderFramesToOutput(streamId, done, isFramePrepared: false); + } + } while (done != HPackHeaderWriter.HeaderWriteResult.Done); } /* Padding is not implemented @@ -1060,4 +1115,6 @@ private void EnqueueWaitingForMoreConnectionWindow(Http2OutputProducer producer) _http2Connection.Abort(new ConnectionAbortedException("HTTP/2 connection exceeded the outgoing flow control maximum queue size.")); } } + + private void ThrowResponseHeadersLimitException() => throw new HPackEncodingException(SR.Format(SR.net_http_headers_exceeded_length, _maxResponseHeadersTotalSize!)); } diff --git a/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs b/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs index ef6a9ea3264e..a09f214b4503 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs @@ -15,6 +15,9 @@ public class KestrelServerLimits // Matches the non-configurable default response buffer size for Kestrel in 1.0.0 private long? _maxResponseBufferSize = 64 * 1024; + // Matches the HttpClientHandler.MaxResponseHeadersLength's response header size. + private int? _maxResponseHeadersTotalSize = 64 * 1024; + // Matches the default client_max_body_size in nginx. // Also large enough that most requests should be under the limit. private long? _maxRequestBufferSize = 1024 * 1024; @@ -256,6 +259,27 @@ public long? MaxConcurrentUpgradedConnections } } + /// + /// Gets or sets the maximum size of the total response headers. When set to null, the response headers total size is unlimited. + /// Defaults to 65,536 bytes (64 KB). + /// + /// + /// When set to null, the size of the response buffer is unlimited. + /// When set to zero, no headers are allowed to be returned. + /// + public int? MaxResponseHeadersTotalSize + { + get => _maxResponseHeadersTotalSize; + set + { + if (value.HasValue && value.Value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.NonNegativeNumberOrNullRequired); + } + _maxResponseHeadersTotalSize = value; + } + } + internal void Serialize(Utf8JsonWriter writer) { writer.WriteString(nameof(KeepAliveTimeout), KeepAliveTimeout.ToString()); @@ -323,6 +347,16 @@ internal void Serialize(Utf8JsonWriter writer) writer.WriteString(nameof(MinResponseDataRate), MinResponseDataRate?.ToString()); writer.WriteString(nameof(RequestHeadersTimeout), RequestHeadersTimeout.ToString()); + writer.WritePropertyName(nameof(MaxResponseHeadersTotalSize)); + if (MaxResponseHeadersTotalSize is null) + { + writer.WriteNullValue(); + } + else + { + writer.WriteNumberValue(MaxResponseHeadersTotalSize.Value); + } + // HTTP2 writer.WritePropertyName(nameof(Http2)); writer.WriteStartObject(); diff --git a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..284fdbb55b33 100644 --- a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt +++ b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt @@ -1 +1,3 @@ #nullable enable +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerLimits.MaxResponseHeadersTotalSize.get -> int? +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerLimits.MaxResponseHeadersTotalSize.set -> void diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs index cafd8a98a0f2..fc7a14ace7e5 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs @@ -12,6 +12,9 @@ using Microsoft.AspNetCore.InternalTesting; using Moq; using Xunit; +using Microsoft.AspNetCore.Http.Features; +using Castle.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -55,8 +58,18 @@ public async Task WriteWindowUpdate_UnsetsReservedBit() private Http2FrameWriter CreateFrameWriter(Pipe pipe) { + var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); var serviceContext = TestContextFactory.CreateServiceContext(new KestrelServerOptions()); - return new Http2FrameWriter(pipe.Writer, null, null, 1, null, null, null, _dirtyMemoryPool, serviceContext); + var featureCollection = new FeatureCollection(); + featureCollection.Set(new TestConnectionMetricsContextFeature()); + var connectionContext = TestContextFactory.CreateHttpConnectionContext( + serviceContext: serviceContext, + connectionContext: null, + transport: pair.Transport, + connectionFeatures: featureCollection); + + var http2Connection = new Http2Connection(connectionContext); + return new Http2FrameWriter(pipe.Writer, null, http2Connection, 1, null, null, null, _dirtyMemoryPool, serviceContext); } [Fact] @@ -92,6 +105,11 @@ public async Task WriteHeader_UnsetsReservedBit() Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x00 }, payload.Skip(5).Take(4).ToArray()); } + + private sealed class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature + { + public ConnectionMetricsContext MetricsContext { get; } + } } public static class PipeReaderExtensions diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs index 9fe40252d54f..c9f32b9ddef2 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs @@ -11,6 +11,8 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; @@ -34,10 +36,19 @@ public void GlobalSetup() httpParser: new HttpParser(), dateHeaderValueManager: new DateHeaderValueManager(TimeProvider.System)); + var featureCollection = new FeatureCollection(); + featureCollection.Set(new TestConnectionMetricsContextFeature()); + var connectionContext = TestContextFactory.CreateHttpConnectionContext( + serviceContext: serviceContext, + connectionContext: null, + transport: new DuplexPipe(_pipe.Reader, _pipe.Writer), + connectionFeatures: featureCollection); + var http2Connection = new Http2Connection(connectionContext); + _frameWriter = new Http2FrameWriter( new NullPipeWriter(), connectionContext: null, - http2Connection: null, + http2Connection: http2Connection, maxStreamsPerConnection: 1, timeoutControl: null, minResponseDataRate: null, @@ -63,4 +74,9 @@ public void Dispose() _pipe.Writer.Complete(); _memoryPool?.Dispose(); } + + private sealed class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature + { + public ConnectionMetricsContext MetricsContext { get; } + } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 95282dd8ffb5..589fa6ced364 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -5554,4 +5554,193 @@ await ExpectAsync(Http2FrameType.RST_STREAM, Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[InternalHeaderNames.Status]); } + + [Fact] + public async Task Headers_LargerMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 1; + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } + + [Fact] + public async Task HeadersContinuation_LargerMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = (int)Http2PeerSettings.DefaultMaxFrameSize * 2; + await InitializeConnectionAsync(async context => + { + context.Response.Headers["My"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + context.Response.Headers["My2"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + var headersFrame3 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 7, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } + + [Fact] + public async Task HeadersContinuation_BufferGrowsOverMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = (int)Http2PeerSettings.DefaultMaxFrameSize * 2; + await InitializeConnectionAsync(async context => + { + context.Response.Headers["My"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize * 2); + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } + + [Fact] + public async Task TrailersWhenDone_LargerMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("too_long", new string('a', 100)); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + // Just the StatusCode gets written before aborting in the continuation frame + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } + + [Fact] + public async Task TrailersWhenMoreHeader_LargerMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("My", new string('a', 100)); + context.Response.AppendTrailer("My2", new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize)); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } + + [Fact] + public async Task TrailersWhenBufferTooSmall_LargerMaxResponseHeadersTotalSize_AbortsConnection() + { + _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("My", new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize + 1)); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + var goAway = await ExpectAsync(Http2FrameType.GOAWAY, + withLength: 8, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); + + _pair.Application.Output.Complete(); + await _connectionTask; + } } diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs index d9166a9d824c..d61ee992f772 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs @@ -1381,7 +1381,18 @@ public async Task Settings_MaxHeaderListSize_Client(string scheme) var hostBuilder = new HostBuilder() .ConfigureWebHost(webHostBuilder => { - ConfigureKestrel(webHostBuilder, scheme); + webHostBuilder.UseKestrel(options => + { + options.Limits.MaxResponseHeadersTotalSize = null; + options.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http2; + if (scheme == "https") + { + listenOptions.UseHttps(TestResources.GetTestCertificate()); + } + }); + }); webHostBuilder.ConfigureServices(AddTestLogging) .Configure(app => app.Run(context => { From b91a34c1e4a6a0463d16592de9ca5cc2c5dcda5d Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 2 May 2024 08:53:17 +0200 Subject: [PATCH 06/27] Protect against overflow --- .../Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 391e33f400ea..cd03411790fb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -74,8 +74,8 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private readonly ArrayBufferWriter _headerEncodingBuffer; - private readonly int? _maxResponseHeadersTotalSize; - private int _currentResponseHeadersTotalSize; + private readonly long? _maxResponseHeadersTotalSize; + private long _currentResponseHeadersTotalSize; private long _unflushedBytes; private bool _completed; From a655a380ac19e51aa97ab55419cad205438e6b89 Mon Sep 17 00:00:00 2001 From: ladeak Date: Fri, 3 May 2024 23:30:10 +0200 Subject: [PATCH 07/27] Validating the header length in HPackHeaderWriter --- .../src/Internal/Http2/Http2FrameWriter.cs | 38 ++---------- .../Core/test/Http2/Http2FrameWriterTests.cs | 11 +--- .../Core/test/Http2/Http2HPackEncoderTests.cs | 60 ++++++++++++------- .../Http2/HPackHeaderWriterBenchmark.cs | 13 ++-- .../Http2/Http2FrameWriterBenchmark.cs | 6 +- .../Kestrel/shared/HPackHeaderWriter.cs | 45 +++++++++++--- .../test/PipeWriterHttp2FrameExtensions.cs | 5 +- .../Http2/Http2TestBase.cs | 18 ++++-- .../HttpClientHttp2InteropTests.cs | 41 +++++++------ 9 files changed, 125 insertions(+), 112 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index cd03411790fb..825c30bbe066 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -5,7 +5,6 @@ using System.Buffers.Binary; using System.Diagnostics; using System.IO.Pipelines; -using System.Net.Http; using System.Net.Http.HPack; using System.Threading.Channels; using Microsoft.AspNetCore.Connections; @@ -515,14 +514,10 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); + _currentResponseHeadersTotalSize = 0; var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); - if (_maxResponseHeadersTotalSize.HasValue && payloadLength > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } - _currentResponseHeadersTotalSize = payloadLength; if (done == HPackHeaderWriter.HeaderWriteResult.Done) { // Fast path, only a single HEADER frame. @@ -583,34 +578,21 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in _headersEnumerator.Initialize(headers); _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.Done) { - if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + payloadLength > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } _headerEncodingBuffer.Advance(payloadLength); SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); } else if (done == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) { // More headers sent in CONTINUATION frames. - _currentResponseHeadersTotalSize += payloadLength; - if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } _headerEncodingBuffer.Advance(payloadLength); SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); FinishWritingHeadersUnsynchronized(streamId); } else { - if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + bufferSize > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } bufferSize *= 2; } } while (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall); @@ -663,24 +645,14 @@ private void FinishWritingHeadersUnsynchronized(int streamId) { _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; - done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); - + done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { - if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize + bufferSize > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } bufferSize *= 2; } else { // In case of Done or MoreHeaders: write to output. - _currentResponseHeadersTotalSize += payloadLength; - if (_maxResponseHeadersTotalSize.HasValue && _currentResponseHeadersTotalSize > _maxResponseHeadersTotalSize.Value) - { - ThrowResponseHeadersLimitException(); - } _headerEncodingBuffer.Advance(payloadLength); SplitHeaderFramesToOutput(streamId, done, isFramePrepared: false); } @@ -1115,6 +1087,4 @@ private void EnqueueWaitingForMoreConnectionWindow(Http2OutputProducer producer) _http2Connection.Abort(new ConnectionAbortedException("HTTP/2 connection exceeded the outgoing flow control maximum queue size.")); } } - - private void ThrowResponseHeadersLimitException() => throw new HPackEncodingException(SR.Format(SR.net_http_headers_exceeded_length, _maxResponseHeadersTotalSize!)); } diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs index fc7a14ace7e5..ec0e4dff251f 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs @@ -1,20 +1,13 @@ // 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.IO.Pipelines; -using System.Linq; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Microsoft.AspNetCore.InternalTesting; using Moq; -using Xunit; -using Microsoft.AspNetCore.Http.Features; -using Castle.Core; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index 448f6c387d0a..28efc4324de8 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -23,13 +23,14 @@ public class Http2HPackEncoderTests public void BeginEncodeHeaders_Status302_NewIndexValue() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = new HttpResponseHeaders(); var enumerator = new Http2HeadersEnumerator(); enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -38,12 +39,14 @@ public void BeginEncodeHeaders_Status302_NewIndexValue() var statusHeader = GetHeaderEntry(hpackEncoder, 0); Assert.Equal(":status", statusHeader.Name); Assert.Equal("302", statusHeader.Value); + Assert.Equal(0, accumulatedLength); } [Fact] public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(); headers.CacheControl = "private"; @@ -52,7 +55,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); var result = buffer.Slice(5, length - 5).ToArray(); var hex = BitConverter.ToString(result); @@ -61,6 +64,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() var statusHeader = GetHeaderEntry(hpackEncoder, 0); Assert.Equal("Cache-Control", statusHeader.Name); Assert.Equal("private", statusHeader.Value); + Assert.Equal(9, accumulatedLength); // Cache-Control has a static table Id. } [Fact] @@ -69,6 +73,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Test follows example https://tools.ietf.org/html/rfc7541#appendix-C.5 Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(); headers.CacheControl = "private"; @@ -81,7 +86,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // First response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -123,7 +128,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -164,7 +169,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -213,6 +218,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Test follows example https://tools.ietf.org/html/rfc7541#appendix-C.5 Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(_ => Encoding.UTF8); headers.CacheControl = "你好e"; @@ -225,7 +231,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // First response enumerator.Initialize((HttpResponseHeaders)headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -267,7 +273,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -308,7 +314,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -358,6 +364,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName, bool neverIndex) { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = new HttpResponseHeaders(); headers.Append(headerName, "1"); @@ -366,7 +373,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(maxHeaderTableSize: Http2PeerSettings.DefaultHeaderTableSize); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out _)); if (neverIndex) { @@ -384,6 +391,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEntry() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var headers = new HttpResponseHeaders(); headers.Append("x-Custom", new string('!', (int)Http2PeerSettings.DefaultHeaderTableSize)); @@ -392,7 +400,7 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); Assert.Empty(GetHeaderEntries(hpackEncoder)); } @@ -477,16 +485,16 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair[] headers, byte[] expectedPayload, int? statusCode) { var hpackEncoder = new DynamicHPackEncoder(); - + long accumulatedLength = 0; var payload = new byte[1024]; var length = 0; if (statusCode.HasValue) { - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(statusCode.Value, hpackEncoder, GetHeadersEnumerator(headers), payload, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(statusCode.Value, hpackEncoder, GetHeadersEnumerator(headers), payload, ref accumulatedLength, null, out length)); } else { - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, GetHeadersEnumerator(headers), payload, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, GetHeadersEnumerator(headers), payload, ref accumulatedLength, null, out length)); } Assert.Equal(expectedPayload.Length, length); @@ -503,6 +511,7 @@ public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair buffer = new byte[1024 * 16]; + long accumulatedLength = 0; var hpackEncoder = new DynamicHPackEncoder(); hpackEncoder.UpdateMaxHeaderTableSize(100); @@ -586,7 +596,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // First request enumerator.Initialize(new Dictionary()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); Assert.Equal(2, length); @@ -600,7 +610,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // Second request enumerator.Initialize(new Dictionary()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); Assert.Equal(0, length); } @@ -609,6 +619,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() public void WithStatusCode_TooLargeHeader_ReturnsNotDone() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length + 1); @@ -616,13 +627,14 @@ public void WithStatusCode_TooLargeHeader_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); } [Fact] public void NoStatusCodeLargeHeader_ReturnsOversized() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length + 1); @@ -630,13 +642,14 @@ public void NoStatusCodeLargeHeader_ReturnsOversized() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); } [Fact] public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length - 1); @@ -644,13 +657,14 @@ public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); } [Fact] public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() { Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Accept = "application/json;"; @@ -659,7 +673,7 @@ public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); } private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs index cff79ef3dd96..83fc5abb7707 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs @@ -1,16 +1,12 @@ // 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.Linq; using System.Net.Http.HPack; using System.Text; -using System.Threading.Tasks; using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; @@ -21,6 +17,7 @@ public class HPackHeaderWriterBenchmark private HttpResponseHeaders _knownResponseHeaders; private HttpResponseHeaders _unknownResponseHeaders; private byte[] _buffer; + private long _accumulatedHeaderLength; [GlobalSetup] public void GlobalSetup() @@ -56,7 +53,7 @@ public void GlobalSetup() public void BeginEncodeHeaders_KnownHeaders() { _http2HeadersEnumerator.Initialize(_knownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); } [Benchmark] @@ -64,14 +61,14 @@ public void BeginEncodeHeaders_KnownHeaders_CustomEncoding() { _knownResponseHeaders.EncodingSelector = _ => Encoding.UTF8; _http2HeadersEnumerator.Initialize(_knownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); } [Benchmark] public void BeginEncodeHeaders_UnknownHeaders() { _http2HeadersEnumerator.Initialize(_unknownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); } [Benchmark] @@ -79,6 +76,6 @@ public void BeginEncodeHeaders_UnknownHeaders_CustomEncoding() { _knownResponseHeaders.EncodingSelector = _ => Encoding.UTF8; _http2HeadersEnumerator.Initialize(_unknownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); } } diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs index c9f32b9ddef2..3b639b56872b 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs @@ -4,14 +4,12 @@ using System.Buffers; using System.IO.Pipelines; using BenchmarkDotNet.Attributes; - using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; -using Microsoft.AspNetCore.InternalTesting; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 2d3ba5a7442e..28cb7c177ef2 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -1,8 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#nullable enable using System.Net.Http; using System.Net.Http.HPack; +using System.Text; #if !(IS_TESTS || IS_BENCHMARKS) namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; @@ -29,7 +31,7 @@ internal enum HeaderWriteResult : byte /// /// Begin encoding headers in the first HEADERS frame. /// - public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) { length = 0; @@ -52,7 +54,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE // We're ok with not increasing the buffer size if no headers were encoded because we've already encoded the status. // There is a small chance that the header will encode if there is no other content in the next HEADERS frame. - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, ref accumulatedLength, maxLength, out var headersLength); length += headersLength; return done; } @@ -60,7 +62,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE /// /// Begin encoding headers in the first HEADERS frame. /// - public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) { length = 0; @@ -75,7 +77,7 @@ public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnco return HeaderWriteResult.Done; } - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, ref accumulatedLength, maxLength, out var headersLength); length += headersLength; return done; } @@ -83,9 +85,9 @@ public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnco /// /// Continue encoding headers in the next HEADERS frame. The enumerator should already have a current value. /// - public static HeaderWriteResult ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) { - return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, canRequestLargerBuffer: true, out length); + return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, canRequestLargerBuffer: true, ref accumulatedLength, maxLength, out length); } private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpackEncoder, Span buffer, out int length) @@ -103,7 +105,7 @@ private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpack } } - private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool canRequestLargerBuffer, out int length) + private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool canRequestLargerBuffer, ref long accumulatedLength, long? maxLength, out int length) { var currentLength = 0; do @@ -130,22 +132,51 @@ private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEnco // Request for a larger buffer to write large header. if (currentLength == 0 && canRequestLargerBuffer) { + // Estimate the encoded header length (without compression) to check if it fits the max length. + // This stops the BufferTooSmall responses to run away with allocating larger and larger buffers. + // The header is probably not indexed by the static or dynamic tables, otherwise it woudld an empty buffer, + // hence calculating a header length. + CheckRequiredHeaderSize(accumulatedLength + currentLength, maxLength, name, value, valueEncoding); length = 0; return HeaderWriteResult.BufferTooSmall; } + if (maxLength.HasValue && accumulatedLength > maxLength) + { + ThrowResponseHeadersLimitException(maxLength.Value); + } length = currentLength; return HeaderWriteResult.MoreHeaders; } currentLength += headerLength; + accumulatedLength += headerLength; } while (headersEnumerator.MoveNext()); + if (maxLength.HasValue && accumulatedLength > maxLength) + { + ThrowResponseHeadersLimitException(maxLength.Value); + } length = currentLength; return HeaderWriteResult.Done; } + private static void CheckRequiredHeaderSize(long accumulatedLength, long? maxLength, string name, string value, Encoding? valueEncoding) + { + if (!maxLength.HasValue) + { + return; + } + var length = HeaderField.GetLength(name.Length, valueEncoding?.GetByteCount(value) ?? value.Length); + if (length + accumulatedLength > maxLength) + { + ThrowResponseHeadersLimitException(maxLength.Value); + } + } + + private static void ThrowResponseHeadersLimitException(long maxLength) => throw new HPackEncodingException(SR.Format(SR.net_http_headers_exceeded_length, maxLength!)); + private static HeaderEncodingHint ResolveHeaderEncodingHint(int staticTableId, string name) { HeaderEncodingHint hint; diff --git a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs index 52a168ba696b..37735bd8851c 100644 --- a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs +++ b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs @@ -33,7 +33,8 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami frame.PrepareHeaders(Http2HeadersFrameFlags.NONE, streamId); var buffer = headerEncodingBuffer.AsSpan(); - var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, out var length); + var currentLength = 0L; + var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, ref currentLength, null, out var length); frame.PayloadLength = length; if (done == HPackHeaderWriter.HeaderWriteResult.Done) @@ -53,7 +54,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami { frame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); - done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, out length); + done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, ref currentLength, null, out length); frame.PayloadLength = length; if (done == HPackHeaderWriter.HeaderWriteResult.Done) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 4542e01e0f2b..216aea416323 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -633,8 +633,9 @@ protected Task SendHeadersWithPaddingAsync(int streamId, IEnumerable SendHeadersAsync(int streamId, Http2HeadersFrameFlags { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); + long accumulatedHeaderLength = 0; frame.PrepareHeaders(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, out var length); + var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, ref accumulatedHeaderLength, null, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); @@ -910,10 +914,11 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); + long accumulatedHeaderLength = 0; frame.PrepareContinuation(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, out var length); + var done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, ref accumulatedHeaderLength, null, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); @@ -938,10 +943,11 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); + long accumulatedHeaderLength = 0; frame.PrepareContinuation(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, GetHeadersEnumerator(headers), buffer.Span, out var length); + var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, GetHeadersEnumerator(headers), buffer.Span, ref accumulatedHeaderLength, null, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs index d61ee992f772..01142eb5258a 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs @@ -1381,18 +1381,7 @@ public async Task Settings_MaxHeaderListSize_Client(string scheme) var hostBuilder = new HostBuilder() .ConfigureWebHost(webHostBuilder => { - webHostBuilder.UseKestrel(options => - { - options.Limits.MaxResponseHeadersTotalSize = null; - options.Listen(IPAddress.Loopback, 0, listenOptions => - { - listenOptions.Protocols = HttpProtocols.Http2; - if (scheme == "https") - { - listenOptions.UseHttps(TestResources.GetTestCertificate()); - } - }); - }); + ConfigureKestrel(webHostBuilder, scheme, headerLimit: null); webHostBuilder.ConfigureServices(AddTestLogging) .Configure(app => app.Run(context => { @@ -1718,14 +1707,28 @@ private static void ConfigureKestrel(IWebHostBuilder webHostBuilder, string sche { webHostBuilder.UseKestrel(options => { - options.Listen(IPAddress.Loopback, 0, listenOptions => + ConfigureListeneOptions(scheme, options); + }); + } + + private static void ConfigureKestrel(IWebHostBuilder webHostBuilder, string scheme, int? headerLimit) + { + webHostBuilder.UseKestrel(options => + { + options.Limits.MaxResponseHeadersTotalSize = headerLimit; + ConfigureListeneOptions(scheme, options); + }); + } + + private static void ConfigureListeneOptions(string scheme, KestrelServerOptions options) + { + options.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http2; + if (scheme == "https") { - listenOptions.Protocols = HttpProtocols.Http2; - if (scheme == "https") - { - listenOptions.UseHttps(TestResources.GetTestCertificate()); - } - }); + listenOptions.UseHttps(TestResources.GetTestCertificate()); + } }); } From f31c8c832398d9245a7acac4bd0b7202727ab1d8 Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 4 May 2024 14:05:40 +0200 Subject: [PATCH 08/27] Removing a test file commented out and adding more tests. --- .../Core/test/HPackHeaderWriterTests.cs | 199 ------------------ .../Core/test/Http2/Http2HPackEncoderTests.cs | 107 +++++++++- 2 files changed, 104 insertions(+), 202 deletions(-) delete mode 100644 src/Servers/Kestrel/Core/test/HPackHeaderWriterTests.cs diff --git a/src/Servers/Kestrel/Core/test/HPackHeaderWriterTests.cs b/src/Servers/Kestrel/Core/test/HPackHeaderWriterTests.cs deleted file mode 100644 index cc6d85be4950..000000000000 --- a/src/Servers/Kestrel/Core/test/HPackHeaderWriterTests.cs +++ /dev/null @@ -1,199 +0,0 @@ -// 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.Collections.Generic; -//using System.Linq; -//using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -//using Microsoft.Extensions.Primitives; -//using Xunit; - -//namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests -//{ -// public class HPackHeaderWriterTests -// { -// public static TheoryData[], byte[], int?> SinglePayloadData -// { -// get -// { -// var data = new TheoryData[], byte[], int?>(); - -// // Lowercase header name letters only -// data.Add( -// new[] -// { -// new KeyValuePair("CustomHeader", "CustomValue"), -// }, -// new byte[] -// { -// // 0 12 c u s t o m -// 0x00, 0x0c, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, -// // h e a d e r 11 C -// 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x0b, 0x43, -// // u s t o m V a l -// 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x56, 0x61, 0x6c, -// // u e -// 0x75, 0x65 -// }, -// null); -// // Lowercase header name letters only -// data.Add( -// new[] -// { -// new KeyValuePair("CustomHeader!#$%&'*+-.^_`|~", "CustomValue"), -// }, -// new byte[] -// { -// // 0 27 c u s t o m -// 0x00, 0x1b, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, -// // h e a d e r ! # -// 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x21, 0x23, -// // $ % & ' * + - . -// 0x24, 0x25, 0x26, 0x27, 0x2a, 0x2b, 0x2d, 0x2e, -// // ^ _ ` | ~ 11 C u -// 0x5e, 0x5f, 0x60, 0x7c, 0x7e, 0x0b, 0x43, 0x75, -// // s t o m V a l u -// 0x73, 0x74, 0x6f, 0x6d, 0x56, 0x61, 0x6c, 0x75, -// // e -// 0x65 -// }, -// null); -// // Single Payload -// data.Add( -// new[] -// { -// new KeyValuePair("date", "Mon, 24 Jul 2017 19:22:30 GMT"), -// new KeyValuePair("content-type", "text/html; charset=utf-8"), -// new KeyValuePair("server", "Kestrel") -// }, -// new byte[] -// { -// 0x88, 0x00, 0x04, 0x64, 0x61, 0x74, 0x65, 0x1d, -// 0x4d, 0x6f, 0x6e, 0x2c, 0x20, 0x32, 0x34, 0x20, -// 0x4a, 0x75, 0x6c, 0x20, 0x32, 0x30, 0x31, 0x37, -// 0x20, 0x31, 0x39, 0x3a, 0x32, 0x32, 0x3a, 0x33, -// 0x30, 0x20, 0x47, 0x4d, 0x54, 0x00, 0x0c, 0x63, -// 0x6f, 0x6e, 0x74, 0x65, 0x6e, 0x74, 0x2d, 0x74, -// 0x79, 0x70, 0x65, 0x18, 0x74, 0x65, 0x78, 0x74, -// 0x2f, 0x68, 0x74, 0x6d, 0x6c, 0x3b, 0x20, 0x63, -// 0x68, 0x61, 0x72, 0x73, 0x65, 0x74, 0x3d, 0x75, -// 0x74, 0x66, 0x2d, 0x38, 0x00, 0x06, 0x73, 0x65, -// 0x72, 0x76, 0x65, 0x72, 0x07, 0x4b, 0x65, 0x73, -// 0x74, 0x72, 0x65, 0x6c -// }, -// 200); - -// return data; -// } -// } - -// [Theory] -// [MemberData(nameof(SinglePayloadData))] -// public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair[] headers, byte[] expectedPayload, int? statusCode) -// { -// var payload = new byte[1024]; -// var length = 0; -// if (statusCode.HasValue) -// { -// Assert.True(HPackHeaderWriter.BeginEncodeHeaders(statusCode.Value, GetHeadersEnumerator(headers), payload, out length)); -// } -// else -// { -// Assert.True(HPackHeaderWriter.BeginEncodeHeaders(GetHeadersEnumerator(headers), payload, out length)); -// } -// Assert.Equal(expectedPayload.Length, length); - -// for (var i = 0; i < length; i++) -// { -// Assert.True(expectedPayload[i] == payload[i], $"{expectedPayload[i]} != {payload[i]} at {i} (len {length})"); -// } - -// Assert.Equal(expectedPayload, new ArraySegment(payload, 0, length)); -// } - -// [Theory] -// [InlineData(true)] -// [InlineData(false)] -// public void EncodesHeadersInMultiplePayloadsWhenSpaceNotAvailable(bool exactSize) -// { -// var statusCode = 200; -// var headers = new[] -// { -// new KeyValuePair("date", "Mon, 24 Jul 2017 19:22:30 GMT"), -// new KeyValuePair("content-type", "text/html; charset=utf-8"), -// new KeyValuePair("server", "Kestrel") -// }; - -// var expectedStatusCodePayload = new byte[] -// { -// 0x88 -// }; - -// var expectedDateHeaderPayload = new byte[] -// { -// 0x00, 0x04, 0x64, 0x61, 0x74, 0x65, 0x1d, 0x4d, -// 0x6f, 0x6e, 0x2c, 0x20, 0x32, 0x34, 0x20, 0x4a, -// 0x75, 0x6c, 0x20, 0x32, 0x30, 0x31, 0x37, 0x20, -// 0x31, 0x39, 0x3a, 0x32, 0x32, 0x3a, 0x33, 0x30, -// 0x20, 0x47, 0x4d, 0x54 -// }; - -// var expectedContentTypeHeaderPayload = new byte[] -// { -// 0x00, 0x0c, 0x63, 0x6f, 0x6e, 0x74, 0x65, 0x6e, -// 0x74, 0x2d, 0x74, 0x79, 0x70, 0x65, 0x18, 0x74, -// 0x65, 0x78, 0x74, 0x2f, 0x68, 0x74, 0x6d, 0x6c, -// 0x3b, 0x20, 0x63, 0x68, 0x61, 0x72, 0x73, 0x65, -// 0x74, 0x3d, 0x75, 0x74, 0x66, 0x2d, 0x38 -// }; - -// var expectedServerHeaderPayload = new byte[] -// { -// 0x00, 0x06, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, -// 0x07, 0x4b, 0x65, 0x73, 0x74, 0x72, 0x65, 0x6c -// }; - -// Span payload = new byte[1024]; -// var offset = 0; -// var headerEnumerator = GetHeadersEnumerator(headers); - -// // When !exactSize, slices are one byte short of fitting the next header -// var sliceLength = expectedStatusCodePayload.Length + (exactSize ? 0 : expectedDateHeaderPayload.Length - 1); -// Assert.False(HPackHeaderWriter.BeginEncodeHeaders(statusCode, headerEnumerator, payload.Slice(offset, sliceLength), out var length)); -// Assert.Equal(expectedStatusCodePayload.Length, length); -// Assert.Equal(expectedStatusCodePayload, payload.Slice(0, length).ToArray()); - -// offset += length; - -// sliceLength = expectedDateHeaderPayload.Length + (exactSize ? 0 : expectedContentTypeHeaderPayload.Length - 1); -// Assert.False(HPackHeaderWriter.ContinueEncodeHeaders(headerEnumerator, payload.Slice(offset, sliceLength), out length)); -// Assert.Equal(expectedDateHeaderPayload.Length, length); -// Assert.Equal(expectedDateHeaderPayload, payload.Slice(offset, length).ToArray()); - -// offset += length; - -// sliceLength = expectedContentTypeHeaderPayload.Length + (exactSize ? 0 : expectedServerHeaderPayload.Length - 1); -// Assert.False(HPackHeaderWriter.ContinueEncodeHeaders(headerEnumerator, payload.Slice(offset, sliceLength), out length)); -// Assert.Equal(expectedContentTypeHeaderPayload.Length, length); -// Assert.Equal(expectedContentTypeHeaderPayload, payload.Slice(offset, length).ToArray()); - -// offset += length; - -// sliceLength = expectedServerHeaderPayload.Length; -// Assert.True(HPackHeaderWriter.ContinueEncodeHeaders(headerEnumerator, payload.Slice(offset, sliceLength), out length)); -// Assert.Equal(expectedServerHeaderPayload.Length, length); -// Assert.Equal(expectedServerHeaderPayload, payload.Slice(offset, length).ToArray()); -// } - -// private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) -// { -// var groupedHeaders = headers -// .GroupBy(k => k.Key) -// .ToDictionary(g => g.Key, g => new StringValues(g.Select(gg => gg.Value).ToArray())); - -// var enumerator = new Http2HeadersEnumerator(); -// enumerator.Initialize(groupedHeaders); -// return enumerator; -// } -// } -//} diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index 28efc4324de8..27e43d66de7b 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -67,6 +67,44 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() Assert.Equal(9, accumulatedLength); // Cache-Control has a static table Id. } + [Fact] + public void BeginEncodeHeaders_CustomHeader_AccumulatedBytesIncreased() + { + Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; + + var headers = (IHeaderDictionary)new HttpResponseHeaders(); + headers["My"] = "hello world"; + + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + + Assert.Equal(21, length); + Assert.Equal(16, accumulatedLength); // Excludes status. + } + + [Fact] + public void NoStatusBeginEncodeHeaders_CustomHeader_AccumulatedLengthEqualsPayloadLength() + { + Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; + + var headers = (IHeaderDictionary)new HttpResponseHeaders(); + headers["My"] = "hello world"; + + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + + Assert.Equal(16, length); + Assert.Equal(16, accumulatedLength); + } + [Fact] public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() { @@ -616,7 +654,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() } [Fact] - public void WithStatusCode_TooLargeHeader_ReturnsNotDone() + public void WithStatusCode_TooLargeHeader_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; long accumulatedLength = 0; @@ -628,6 +666,8 @@ public void WithStatusCode_TooLargeHeader_ReturnsNotDone() var hpackEncoder = new DynamicHPackEncoder(); Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(1, length); + Assert.Equal(0, accumulatedLength); // Status exluded } [Fact] @@ -643,10 +683,12 @@ public void NoStatusCodeLargeHeader_ReturnsOversized() var hpackEncoder = new DynamicHPackEncoder(); Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(0, length); + Assert.Equal(0, accumulatedLength); } [Fact] - public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() + public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; long accumulatedLength = 0; @@ -658,10 +700,12 @@ public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() var hpackEncoder = new DynamicHPackEncoder(); Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(1, length); + Assert.Equal(0, accumulatedLength); // No status } [Fact] - public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() + public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; long accumulatedLength = 0; @@ -674,6 +718,63 @@ public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsNotDone() var hpackEncoder = new DynamicHPackEncoder(); Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(26, length); + Assert.Equal(26, accumulatedLength); + } + + [Fact] + public void MaxHeaderTotalLength_ThrowsBeforeDone() + { + int headerLength = 10; + IHeaderDictionary headers = new HttpResponseHeaders(); + headers["My"] = new string('a', headerLength); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Throws(() => + { + Span buffer = new byte[1024 * 16]; + long accumulatedLength = 0; + HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, headerLength - 1, out var length); + }); + } + + [Fact] + public void MaxHeaderTotalLength_ThrowsBeforeMoreHeaders() + { + int headerLength = 1000; + IHeaderDictionary headers = new HttpResponseHeaders(); + headers["My"] = new string('a', headerLength); + headers["My2"] = new string('a', headerLength); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Throws(() => + { + Span buffer = new byte[1024]; + long accumulatedLength = 0; + HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, headerLength + 1, out var length); + }); + } + + [Fact] + public void MaxHeaderTotalLength_ThrowsBeforeBufferTooSmall() + { + int headerLength = 1200; + IHeaderDictionary headers = new HttpResponseHeaders(); + headers["My"] = new string('a', headerLength); + var enumerator = new Http2HeadersEnumerator(); + enumerator.Initialize(headers); + + var hpackEncoder = new DynamicHPackEncoder(); + Assert.Throws(() => + { + Span buffer = new byte[1024]; + long accumulatedLength = 0; + HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, 1, out var length); + }); } private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) From 6ab7271919c68f40a7a0f20a79eb286cc7ce2fdd Mon Sep 17 00:00:00 2001 From: ladeak Date: Mon, 13 May 2024 23:33:24 +0200 Subject: [PATCH 09/27] Review feedback --- .../src/Internal/Http2/Http2FrameWriter.cs | 22 +++++++++---------- .../Kestrel/shared/HPackHeaderWriter.cs | 10 ++++++--- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 825c30bbe066..73ddbfb7c3d7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -517,7 +517,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _currentResponseHeadersTotalSize = 0; var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); - Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, beucase this always writes the status."); + Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); if (done == HPackHeaderWriter.HeaderWriteResult.Done) { // Fast path, only a single HEADER frame. @@ -530,7 +530,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht { // More headers sent in CONTINUATION frames. _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); FinishWritingHeadersUnsynchronized(streamId); } } @@ -582,13 +582,13 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in if (done == HPackHeaderWriter.HeaderWriteResult.Done) { _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + SplitHeaderFramesToOutput(streamId, endOfHeaders: true, isFramePrepared: true); } else if (done == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) { // More headers sent in CONTINUATION frames. _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: true); + SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); FinishWritingHeadersUnsynchronized(streamId); } else @@ -609,7 +609,7 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void SplitHeaderFramesToOutput(int streamId, HPackHeaderWriter.HeaderWriteResult done, bool isFramePrepared) + private void SplitHeaderFramesToOutput(int streamId, bool endOfHeaders, bool isFramePrepared) { var dataToFrame = _headerEncodingBuffer.WrittenSpan; var shouldPrepareFrame = !isFramePrepared; @@ -619,14 +619,12 @@ private void SplitHeaderFramesToOutput(int streamId, HPackHeaderWriter.HeaderWri { _outgoingFrame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); } - else - { - shouldPrepareFrame = true; - } + // Should prepare continuation frames. + shouldPrepareFrame = true; var currentSize = Math.Min(dataToFrame.Length, _maxFrameSize); _outgoingFrame.PayloadLength = currentSize; - if (done == HPackHeaderWriter.HeaderWriteResult.Done && dataToFrame.Length == currentSize) + if (endOfHeaders && dataToFrame.Length == currentSize) { _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; } @@ -644,7 +642,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId) do { _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { @@ -654,7 +652,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId) { // In case of Done or MoreHeaders: write to output. _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, done, isFramePrepared: false); + SplitHeaderFramesToOutput(streamId, endOfHeaders: done == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); } } while (done != HPackHeaderWriter.HeaderWriteResult.Done); } diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 28cb7c177ef2..99d5ae918a97 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -52,7 +52,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE return HeaderWriteResult.Done; } - // We're ok with not increasing the buffer size if no headers were encoded because we've already encoded the status. + // Since we've already encoded the status, we know we didn't start with an empty buffer. We don't need to increase it immediately because // There is a small chance that the header will encode if there is no other content in the next HEADERS frame. var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, ref accumulatedLength, maxLength, out var headersLength); length += headersLength; @@ -168,10 +168,14 @@ private static void CheckRequiredHeaderSize(long accumulatedLength, long? maxLen { return; } + var maxLengthValue = maxLength.GetValueOrDefault(); + + // The default encoding is Latin1, hence we can use the value.Length. HPack encoder uses the same + // calculation for the header value length. var length = HeaderField.GetLength(name.Length, valueEncoding?.GetByteCount(value) ?? value.Length); - if (length + accumulatedLength > maxLength) + if (length + accumulatedLength > maxLengthValue) { - ThrowResponseHeadersLimitException(maxLength.Value); + ThrowResponseHeadersLimitException(maxLengthValue); } } From e21620a44e1589a5f70b03635d47e49680fb146c Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 15 May 2024 21:49:52 +0200 Subject: [PATCH 10/27] Removing header limits related code because it is up to the service owner to not to ddos itself --- .../src/Internal/Http2/Http2Connection.cs | 6 +- .../src/Internal/Http2/Http2FrameWriter.cs | 16 +- .../Kestrel/Core/src/KestrelServerLimits.cs | 34 --- .../Kestrel/Core/src/PublicAPI.Unshipped.txt | 2 - .../Core/test/Http2/Http2FrameWriterTests.cs | 19 +- .../Core/test/Http2/Http2HPackEncoderTests.cs | 164 ++------------ .../Http2/HPackHeaderWriterBenchmark.cs | 9 +- .../Http2/Http2FrameWriterBenchmark.cs | 18 +- .../Kestrel/shared/HPackHeaderWriter.cs | 50 +---- .../test/PipeWriterHttp2FrameExtensions.cs | 5 +- .../Http2/Http2StreamTests.cs | 200 +----------------- .../Http2/Http2TestBase.cs | 20 +- .../HttpClientHttp2InteropTests.cs | 30 +-- 13 files changed, 62 insertions(+), 511 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 44fd7ad794b8..890cf2aed347 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -938,7 +938,9 @@ private Task ProcessSettingsFrameAsync(in ReadOnlySequence payload) if (_clientSettings.MaxFrameSize != previousMaxFrameSize) { // Don't let the client choose an arbitrarily large size, this will be used for response buffers. - _frameWriter.UpdateMaxFrameSize(Math.Min(_clientSettings.MaxFrameSize, _serverSettings.MaxFrameSize)); + // Safe cast, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. + // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 + _frameWriter.UpdateMaxFrameSize((int)Math.Min(_clientSettings.MaxFrameSize, _serverSettings.MaxFrameSize)); } // This difference can be negative. @@ -1829,4 +1831,4 @@ private static class GracefulCloseInitiator public const int Server = 1; public const int Client = 2; } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 73ddbfb7c3d7..c4c2719f60fe 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -73,8 +73,6 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private readonly ArrayBufferWriter _headerEncodingBuffer; - private readonly long? _maxResponseHeadersTotalSize; - private long _currentResponseHeadersTotalSize; private long _unflushedBytes; private bool _completed; @@ -112,7 +110,6 @@ public Http2FrameWriter( _headerEncodingBuffer = new ArrayBufferWriter(_maxFrameSize); _scheduleInline = serviceContext.Scheduler == PipeScheduler.Inline; - _maxResponseHeadersTotalSize = _http2Connection.Limits.MaxResponseHeadersTotalSize; _hpackEncoder = new DynamicHPackEncoder(serviceContext.ServerOptions.AllowResponseHeaderCompression); _maximumFlowControlQueueSize = AppContextMaximumFlowControlQueueSize is null @@ -369,15 +366,13 @@ public void UpdateMaxHeaderTableSize(uint maxHeaderTableSize) } } - public void UpdateMaxFrameSize(uint maxFrameSize) + public void UpdateMaxFrameSize(int maxFrameSize) { lock (_writeLock) { if (_maxFrameSize != maxFrameSize) { - // Safe cast, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. - // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 - _maxFrameSize = (int)maxFrameSize; + _maxFrameSize = maxFrameSize; } } } @@ -514,9 +509,8 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); - _currentResponseHeadersTotalSize = 0; var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); + var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); if (done == HPackHeaderWriter.HeaderWriteResult.Done) { @@ -578,7 +572,7 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in _headersEnumerator.Initialize(headers); _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); + done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.Done) { _headerEncodingBuffer.Advance(payloadLength); @@ -643,7 +637,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId) { _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; - done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, ref _currentResponseHeadersTotalSize, _maxResponseHeadersTotalSize, out var payloadLength); + done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { bufferSize *= 2; diff --git a/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs b/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs index a09f214b4503..ef6a9ea3264e 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerLimits.cs @@ -15,9 +15,6 @@ public class KestrelServerLimits // Matches the non-configurable default response buffer size for Kestrel in 1.0.0 private long? _maxResponseBufferSize = 64 * 1024; - // Matches the HttpClientHandler.MaxResponseHeadersLength's response header size. - private int? _maxResponseHeadersTotalSize = 64 * 1024; - // Matches the default client_max_body_size in nginx. // Also large enough that most requests should be under the limit. private long? _maxRequestBufferSize = 1024 * 1024; @@ -259,27 +256,6 @@ public long? MaxConcurrentUpgradedConnections } } - /// - /// Gets or sets the maximum size of the total response headers. When set to null, the response headers total size is unlimited. - /// Defaults to 65,536 bytes (64 KB). - /// - /// - /// When set to null, the size of the response buffer is unlimited. - /// When set to zero, no headers are allowed to be returned. - /// - public int? MaxResponseHeadersTotalSize - { - get => _maxResponseHeadersTotalSize; - set - { - if (value.HasValue && value.Value < 0) - { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.NonNegativeNumberOrNullRequired); - } - _maxResponseHeadersTotalSize = value; - } - } - internal void Serialize(Utf8JsonWriter writer) { writer.WriteString(nameof(KeepAliveTimeout), KeepAliveTimeout.ToString()); @@ -347,16 +323,6 @@ internal void Serialize(Utf8JsonWriter writer) writer.WriteString(nameof(MinResponseDataRate), MinResponseDataRate?.ToString()); writer.WriteString(nameof(RequestHeadersTimeout), RequestHeadersTimeout.ToString()); - writer.WritePropertyName(nameof(MaxResponseHeadersTotalSize)); - if (MaxResponseHeadersTotalSize is null) - { - writer.WriteNullValue(); - } - else - { - writer.WriteNumberValue(MaxResponseHeadersTotalSize.Value); - } - // HTTP2 writer.WritePropertyName(nameof(Http2)); writer.WriteStartObject(); diff --git a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt index 284fdbb55b33..7dc5c58110bf 100644 --- a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt +++ b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt @@ -1,3 +1 @@ #nullable enable -Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerLimits.MaxResponseHeadersTotalSize.get -> int? -Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerLimits.MaxResponseHeadersTotalSize.set -> void diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs index ec0e4dff251f..f3887d7a0abe 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs @@ -3,10 +3,8 @@ using System.Buffers; using System.IO.Pipelines; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Moq; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -51,18 +49,8 @@ public async Task WriteWindowUpdate_UnsetsReservedBit() private Http2FrameWriter CreateFrameWriter(Pipe pipe) { - var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); var serviceContext = TestContextFactory.CreateServiceContext(new KestrelServerOptions()); - var featureCollection = new FeatureCollection(); - featureCollection.Set(new TestConnectionMetricsContextFeature()); - var connectionContext = TestContextFactory.CreateHttpConnectionContext( - serviceContext: serviceContext, - connectionContext: null, - transport: pair.Transport, - connectionFeatures: featureCollection); - - var http2Connection = new Http2Connection(connectionContext); - return new Http2FrameWriter(pipe.Writer, null, http2Connection, 1, null, null, null, _dirtyMemoryPool, serviceContext); + return new Http2FrameWriter(pipe.Writer, null, null, 1, null, null, null, _dirtyMemoryPool, serviceContext); } [Fact] @@ -98,11 +86,6 @@ public async Task WriteHeader_UnsetsReservedBit() Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x00 }, payload.Skip(5).Take(4).ToArray()); } - - private sealed class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature - { - public ConnectionMetricsContext MetricsContext { get; } - } } public static class PipeReaderExtensions diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index 27e43d66de7b..c6a9e849f7b4 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -1,19 +1,12 @@ // 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.Collections.Generic; -using System.Linq; using System.Net.Http.HPack; using System.Text; -using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.Extensions.Primitives; -using Microsoft.Net.Http.Headers; - -using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -23,14 +16,13 @@ public class Http2HPackEncoderTests public void BeginEncodeHeaders_Status302_NewIndexValue() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = new HttpResponseHeaders(); var enumerator = new Http2HeadersEnumerator(); enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -39,14 +31,12 @@ public void BeginEncodeHeaders_Status302_NewIndexValue() var statusHeader = GetHeaderEntry(hpackEncoder, 0); Assert.Equal(":status", statusHeader.Name); Assert.Equal("302", statusHeader.Value); - Assert.Equal(0, accumulatedLength); } [Fact] public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(); headers.CacheControl = "private"; @@ -55,7 +45,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(5, length - 5).ToArray(); var hex = BitConverter.ToString(result); @@ -64,54 +54,13 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() var statusHeader = GetHeaderEntry(hpackEncoder, 0); Assert.Equal("Cache-Control", statusHeader.Name); Assert.Equal("private", statusHeader.Value); - Assert.Equal(9, accumulatedLength); // Cache-Control has a static table Id. - } - - [Fact] - public void BeginEncodeHeaders_CustomHeader_AccumulatedBytesIncreased() - { - Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; - - var headers = (IHeaderDictionary)new HttpResponseHeaders(); - headers["My"] = "hello world"; - - var enumerator = new Http2HeadersEnumerator(); - enumerator.Initialize(headers); - - var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); - - Assert.Equal(21, length); - Assert.Equal(16, accumulatedLength); // Excludes status. - } - - [Fact] - public void NoStatusBeginEncodeHeaders_CustomHeader_AccumulatedLengthEqualsPayloadLength() - { - Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; - - var headers = (IHeaderDictionary)new HttpResponseHeaders(); - headers["My"] = "hello world"; - - var enumerator = new Http2HeadersEnumerator(); - enumerator.Initialize(headers); - - var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); - - Assert.Equal(16, length); - Assert.Equal(16, accumulatedLength); } [Fact] public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() { // Test follows example https://tools.ietf.org/html/rfc7541#appendix-C.5 - Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(); headers.CacheControl = "private"; @@ -124,7 +73,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // First response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -166,7 +115,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -207,7 +156,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -256,7 +205,6 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Test follows example https://tools.ietf.org/html/rfc7541#appendix-C.5 Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = (IHeaderDictionary)new HttpResponseHeaders(_ => Encoding.UTF8); headers.CacheControl = "你好e"; @@ -269,7 +217,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // First response enumerator.Initialize((HttpResponseHeaders)headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -311,7 +259,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -352,7 +300,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -402,7 +350,6 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName, bool neverIndex) { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = new HttpResponseHeaders(); headers.Append(headerName, "1"); @@ -411,7 +358,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(maxHeaderTableSize: Http2PeerSettings.DefaultHeaderTableSize); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out _)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); if (neverIndex) { @@ -429,7 +376,6 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEntry() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var headers = new HttpResponseHeaders(); headers.Append("x-Custom", new string('!', (int)Http2PeerSettings.DefaultHeaderTableSize)); @@ -438,7 +384,7 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Empty(GetHeaderEntries(hpackEncoder)); } @@ -523,16 +469,15 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair[] headers, byte[] expectedPayload, int? statusCode) { var hpackEncoder = new DynamicHPackEncoder(); - long accumulatedLength = 0; var payload = new byte[1024]; var length = 0; if (statusCode.HasValue) { - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(statusCode.Value, hpackEncoder, GetHeadersEnumerator(headers), payload, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(statusCode.Value, hpackEncoder, GetHeadersEnumerator(headers), payload, out length)); } else { - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, GetHeadersEnumerator(headers), payload, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, GetHeadersEnumerator(headers), payload, out length)); } Assert.Equal(expectedPayload.Length, length); @@ -549,7 +494,6 @@ public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair buffer = new byte[1024 * 16]; - long accumulatedLength = 0; var hpackEncoder = new DynamicHPackEncoder(); hpackEncoder.UpdateMaxHeaderTableSize(100); @@ -634,7 +577,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // First request enumerator.Initialize(new Dictionary()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(2, length); @@ -648,7 +591,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // Second request enumerator.Initialize(new Dictionary()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); Assert.Equal(0, length); } @@ -657,7 +600,6 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() public void WithStatusCode_TooLargeHeader_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length + 1); @@ -665,16 +607,14 @@ public void WithStatusCode_TooLargeHeader_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(1, length); - Assert.Equal(0, accumulatedLength); // Status exluded } [Fact] public void NoStatusCodeLargeHeader_ReturnsOversized() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length + 1); @@ -682,16 +622,14 @@ public void NoStatusCodeLargeHeader_ReturnsOversized() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(0, length); - Assert.Equal(0, accumulatedLength); } [Fact] public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Cookie = new string('a', buffer.Length - 1); @@ -699,16 +637,14 @@ public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(1, length); - Assert.Equal(0, accumulatedLength); // No status } [Fact] public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() { Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; IHeaderDictionary headers = new HttpResponseHeaders(); headers.Accept = "application/json;"; @@ -717,64 +653,8 @@ public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, null, out var length)); + Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(26, length); - Assert.Equal(26, accumulatedLength); - } - - [Fact] - public void MaxHeaderTotalLength_ThrowsBeforeDone() - { - int headerLength = 10; - IHeaderDictionary headers = new HttpResponseHeaders(); - headers["My"] = new string('a', headerLength); - var enumerator = new Http2HeadersEnumerator(); - enumerator.Initialize(headers); - - var hpackEncoder = new DynamicHPackEncoder(); - Assert.Throws(() => - { - Span buffer = new byte[1024 * 16]; - long accumulatedLength = 0; - HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, headerLength - 1, out var length); - }); - } - - [Fact] - public void MaxHeaderTotalLength_ThrowsBeforeMoreHeaders() - { - int headerLength = 1000; - IHeaderDictionary headers = new HttpResponseHeaders(); - headers["My"] = new string('a', headerLength); - headers["My2"] = new string('a', headerLength); - var enumerator = new Http2HeadersEnumerator(); - enumerator.Initialize(headers); - - var hpackEncoder = new DynamicHPackEncoder(); - Assert.Throws(() => - { - Span buffer = new byte[1024]; - long accumulatedLength = 0; - HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, headerLength + 1, out var length); - }); - } - - [Fact] - public void MaxHeaderTotalLength_ThrowsBeforeBufferTooSmall() - { - int headerLength = 1200; - IHeaderDictionary headers = new HttpResponseHeaders(); - headers["My"] = new string('a', headerLength); - var enumerator = new Http2HeadersEnumerator(); - enumerator.Initialize(headers); - - var hpackEncoder = new DynamicHPackEncoder(); - Assert.Throws(() => - { - Span buffer = new byte[1024]; - long accumulatedLength = 0; - HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, ref accumulatedLength, 1, out var length); - }); } private static Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs index 83fc5abb7707..cff2aa9bed9d 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/HPackHeaderWriterBenchmark.cs @@ -17,7 +17,6 @@ public class HPackHeaderWriterBenchmark private HttpResponseHeaders _knownResponseHeaders; private HttpResponseHeaders _unknownResponseHeaders; private byte[] _buffer; - private long _accumulatedHeaderLength; [GlobalSetup] public void GlobalSetup() @@ -53,7 +52,7 @@ public void GlobalSetup() public void BeginEncodeHeaders_KnownHeaders() { _http2HeadersEnumerator.Initialize(_knownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); } [Benchmark] @@ -61,14 +60,14 @@ public void BeginEncodeHeaders_KnownHeaders_CustomEncoding() { _knownResponseHeaders.EncodingSelector = _ => Encoding.UTF8; _http2HeadersEnumerator.Initialize(_knownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); } [Benchmark] public void BeginEncodeHeaders_UnknownHeaders() { _http2HeadersEnumerator.Initialize(_unknownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); } [Benchmark] @@ -76,6 +75,6 @@ public void BeginEncodeHeaders_UnknownHeaders_CustomEncoding() { _knownResponseHeaders.EncodingSelector = _ => Encoding.UTF8; _http2HeadersEnumerator.Initialize(_unknownResponseHeaders); - HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, ref _accumulatedHeaderLength, null, out _); + HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _http2HeadersEnumerator, _buffer, out _); } } diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs index 3b639b56872b..7b934d2101c4 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs @@ -5,12 +5,10 @@ using System.IO.Pipelines; using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; @@ -34,19 +32,10 @@ public void GlobalSetup() httpParser: new HttpParser(), dateHeaderValueManager: new DateHeaderValueManager(TimeProvider.System)); - var featureCollection = new FeatureCollection(); - featureCollection.Set(new TestConnectionMetricsContextFeature()); - var connectionContext = TestContextFactory.CreateHttpConnectionContext( - serviceContext: serviceContext, - connectionContext: null, - transport: new DuplexPipe(_pipe.Reader, _pipe.Writer), - connectionFeatures: featureCollection); - var http2Connection = new Http2Connection(connectionContext); - _frameWriter = new Http2FrameWriter( new NullPipeWriter(), connectionContext: null, - http2Connection: http2Connection, + http2Connection: null, maxStreamsPerConnection: 1, timeoutControl: null, minResponseDataRate: null, @@ -72,9 +61,4 @@ public void Dispose() _pipe.Writer.Complete(); _memoryPool?.Dispose(); } - - private sealed class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature - { - public ConnectionMetricsContext MetricsContext { get; } - } } diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 99d5ae918a97..2c5cff09b610 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -4,7 +4,6 @@ #nullable enable using System.Net.Http; using System.Net.Http.HPack; -using System.Text; #if !(IS_TESTS || IS_BENCHMARKS) namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; @@ -31,7 +30,7 @@ internal enum HeaderWriteResult : byte /// /// Begin encoding headers in the first HEADERS frame. /// - public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) + public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { length = 0; @@ -54,7 +53,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE // Since we've already encoded the status, we know we didn't start with an empty buffer. We don't need to increase it immediately because // There is a small chance that the header will encode if there is no other content in the next HEADERS frame. - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, ref accumulatedLength, maxLength, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: false, out var headersLength); length += headersLength; return done; } @@ -62,7 +61,7 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE /// /// Begin encoding headers in the first HEADERS frame. /// - public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) + public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { length = 0; @@ -77,7 +76,7 @@ public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnco return HeaderWriteResult.Done; } - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, ref accumulatedLength, maxLength, out var headersLength); + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); length += headersLength; return done; } @@ -85,9 +84,9 @@ public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnco /// /// Continue encoding headers in the next HEADERS frame. The enumerator should already have a current value. /// - public static HeaderWriteResult ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, ref long accumulatedLength, long? maxLength, out int length) + public static HeaderWriteResult ContinueEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) { - return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, canRequestLargerBuffer: true, ref accumulatedLength, maxLength, out length); + return EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer, canRequestLargerBuffer: true, out length); } private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpackEncoder, Span buffer, out int length) @@ -105,7 +104,7 @@ private static bool EncodeStatusHeader(int statusCode, DynamicHPackEncoder hpack } } - private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool canRequestLargerBuffer, ref long accumulatedLength, long? maxLength, out int length) + private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool canRequestLargerBuffer, out int length) { var currentLength = 0; do @@ -132,55 +131,20 @@ private static HeaderWriteResult EncodeHeadersCore(DynamicHPackEncoder hpackEnco // Request for a larger buffer to write large header. if (currentLength == 0 && canRequestLargerBuffer) { - // Estimate the encoded header length (without compression) to check if it fits the max length. - // This stops the BufferTooSmall responses to run away with allocating larger and larger buffers. - // The header is probably not indexed by the static or dynamic tables, otherwise it woudld an empty buffer, - // hence calculating a header length. - CheckRequiredHeaderSize(accumulatedLength + currentLength, maxLength, name, value, valueEncoding); length = 0; return HeaderWriteResult.BufferTooSmall; } - - if (maxLength.HasValue && accumulatedLength > maxLength) - { - ThrowResponseHeadersLimitException(maxLength.Value); - } length = currentLength; return HeaderWriteResult.MoreHeaders; } currentLength += headerLength; - accumulatedLength += headerLength; } while (headersEnumerator.MoveNext()); - - if (maxLength.HasValue && accumulatedLength > maxLength) - { - ThrowResponseHeadersLimitException(maxLength.Value); - } length = currentLength; return HeaderWriteResult.Done; } - private static void CheckRequiredHeaderSize(long accumulatedLength, long? maxLength, string name, string value, Encoding? valueEncoding) - { - if (!maxLength.HasValue) - { - return; - } - var maxLengthValue = maxLength.GetValueOrDefault(); - - // The default encoding is Latin1, hence we can use the value.Length. HPack encoder uses the same - // calculation for the header value length. - var length = HeaderField.GetLength(name.Length, valueEncoding?.GetByteCount(value) ?? value.Length); - if (length + accumulatedLength > maxLengthValue) - { - ThrowResponseHeadersLimitException(maxLengthValue); - } - } - - private static void ThrowResponseHeadersLimitException(long maxLength) => throw new HPackEncodingException(SR.Format(SR.net_http_headers_exceeded_length, maxLength!)); - private static HeaderEncodingHint ResolveHeaderEncodingHint(int staticTableId, string name) { HeaderEncodingHint hint; diff --git a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs index 37735bd8851c..52a168ba696b 100644 --- a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs +++ b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs @@ -33,8 +33,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami frame.PrepareHeaders(Http2HeadersFrameFlags.NONE, streamId); var buffer = headerEncodingBuffer.AsSpan(); - var currentLength = 0L; - var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, ref currentLength, null, out var length); + var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, out var length); frame.PayloadLength = length; if (done == HPackHeaderWriter.HeaderWriteResult.Done) @@ -54,7 +53,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami { frame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); - done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, ref currentLength, null, out length); + done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, out length); frame.PayloadLength = length; if (done == HPackHeaderWriter.HeaderWriteResult.Done) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 589fa6ced364..f77a54738372 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -1,27 +1,18 @@ // 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.Collections.Generic; using System.Globalization; -using System.IO; -using System.Linq; -using System.Net.Http; -using System.Net.Http.HPack; using System.Runtime.ExceptionServices; using System.Text; -using System.Threading; -using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.InternalTesting; using Microsoft.Extensions.Logging; using Microsoft.Net.Http.Headers; -using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -5554,193 +5545,4 @@ await ExpectAsync(Http2FrameType.RST_STREAM, Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[InternalHeaderNames.Status]); } - - [Fact] - public async Task Headers_LargerMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 1; - await InitializeConnectionAsync(async context => - { - await context.Response.WriteAsync("Hello World"); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - // Just the StatusCode gets written before aborting in the continuation frame - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } - - [Fact] - public async Task HeadersContinuation_LargerMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = (int)Http2PeerSettings.DefaultMaxFrameSize * 2; - await InitializeConnectionAsync(async context => - { - context.Response.Headers["My"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); - context.Response.Headers["My2"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); - await context.Response.WriteAsync("Hello World"); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - // Just the StatusCode gets written before aborting in the continuation frame - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 32, - withFlags: (byte)Http2HeadersFrameFlags.NONE, - withStreamId: 1); - var headersFrame2 = await ExpectAsync(Http2FrameType.CONTINUATION, - withLength: 16384, - withFlags: (byte)Http2HeadersFrameFlags.NONE, - withStreamId: 1); - var headersFrame3 = await ExpectAsync(Http2FrameType.CONTINUATION, - withLength: 7, - withFlags: (byte)Http2HeadersFrameFlags.NONE, - withStreamId: 1); - - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } - - [Fact] - public async Task HeadersContinuation_BufferGrowsOverMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = (int)Http2PeerSettings.DefaultMaxFrameSize * 2; - await InitializeConnectionAsync(async context => - { - context.Response.Headers["My"] = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize * 2); - await context.Response.WriteAsync("Hello World"); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - // Just the StatusCode gets written before aborting in the continuation frame - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 32, - withFlags: (byte)Http2HeadersFrameFlags.NONE, - withStreamId: 1); - - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } - - [Fact] - public async Task TrailersWhenDone_LargerMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; - await InitializeConnectionAsync(async context => - { - await context.Response.WriteAsync("Hello World"); - context.Response.AppendTrailer("too_long", new string('a', 100)); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - await ExpectAsync(Http2FrameType.HEADERS, - withLength: 32, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - - await ExpectAsync(Http2FrameType.DATA, - withLength: 11, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - // Just the StatusCode gets written before aborting in the continuation frame - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } - - [Fact] - public async Task TrailersWhenMoreHeader_LargerMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; - await InitializeConnectionAsync(async context => - { - await context.Response.WriteAsync("Hello World"); - context.Response.AppendTrailer("My", new string('a', 100)); - context.Response.AppendTrailer("My2", new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize)); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 32, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 11, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } - - [Fact] - public async Task TrailersWhenBufferTooSmall_LargerMaxResponseHeadersTotalSize_AbortsConnection() - { - _serviceContext.ServerOptions.Limits.MaxResponseHeadersTotalSize = 100; - await InitializeConnectionAsync(async context => - { - await context.Response.WriteAsync("Hello World"); - context.Response.AppendTrailer("My", new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize + 1)); - }); - - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 32, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 11, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - var goAway = await ExpectAsync(Http2FrameType.GOAWAY, - withLength: 8, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 0); - - VerifyGoAway(goAway, int.MaxValue, Http2ErrorCode.INTERNAL_ERROR); - - _pair.Application.Output.Complete(); - await _connectionTask; - } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 216aea416323..1a5b95034e90 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -14,12 +14,12 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Microsoft.AspNetCore.InternalTesting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Microsoft.Extensions.Time.Testing; @@ -633,9 +633,8 @@ protected Task SendHeadersWithPaddingAsync(int streamId, IEnumerable SendHeadersAsync(int streamId, Http2HeadersFrameFlags { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); - long accumulatedHeaderLength = 0; frame.PrepareHeaders(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, ref accumulatedHeaderLength, null, out var length); + var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); @@ -914,11 +910,10 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); - long accumulatedHeaderLength = 0; frame.PrepareContinuation(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, ref accumulatedHeaderLength, null, out var length); + var done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, headersEnumerator, buffer.Span, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); @@ -943,11 +938,10 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF { var outputWriter = _pair.Application.Output; var frame = new Http2Frame(); - long accumulatedHeaderLength = 0; frame.PrepareContinuation(flags, streamId); var buffer = _headerEncodingBuffer.AsMemory(); - var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, GetHeadersEnumerator(headers), buffer.Span, ref accumulatedHeaderLength, null, out var length); + var done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, GetHeadersEnumerator(headers), buffer.Span, out var length); frame.PayloadLength = length; Http2FrameWriter.WriteHeader(frame, outputWriter); diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs index 01142eb5258a..d9166a9d824c 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs @@ -1381,7 +1381,7 @@ public async Task Settings_MaxHeaderListSize_Client(string scheme) var hostBuilder = new HostBuilder() .ConfigureWebHost(webHostBuilder => { - ConfigureKestrel(webHostBuilder, scheme, headerLimit: null); + ConfigureKestrel(webHostBuilder, scheme); webHostBuilder.ConfigureServices(AddTestLogging) .Configure(app => app.Run(context => { @@ -1707,28 +1707,14 @@ private static void ConfigureKestrel(IWebHostBuilder webHostBuilder, string sche { webHostBuilder.UseKestrel(options => { - ConfigureListeneOptions(scheme, options); - }); - } - - private static void ConfigureKestrel(IWebHostBuilder webHostBuilder, string scheme, int? headerLimit) - { - webHostBuilder.UseKestrel(options => - { - options.Limits.MaxResponseHeadersTotalSize = headerLimit; - ConfigureListeneOptions(scheme, options); - }); - } - - private static void ConfigureListeneOptions(string scheme, KestrelServerOptions options) - { - options.Listen(IPAddress.Loopback, 0, listenOptions => - { - listenOptions.Protocols = HttpProtocols.Http2; - if (scheme == "https") + options.Listen(IPAddress.Loopback, 0, listenOptions => { - listenOptions.UseHttps(TestResources.GetTestCertificate()); - } + listenOptions.Protocols = HttpProtocols.Http2; + if (scheme == "https") + { + listenOptions.UseHttps(TestResources.GetTestCertificate()); + } + }); }); } From 9720c3a5f848c2fe8b5f350f41da220ab1efe1db Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 16 May 2024 22:03:20 +0200 Subject: [PATCH 11/27] A test for 1MB header value --- .../Http2/Http2StreamTests.cs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index f77a54738372..4afbca7c7be6 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -3403,6 +3403,42 @@ await InitializeConnectionAsync(async context => Assert.Equal(headerValue, _decodedHeaders[longHeaderName]); } + [Fact] + public async Task ResponseHeader_OneMegaByte_SplitsHeaderToContinuationFrames() + { + int frameSize = (int)Http2PeerSettings.DefaultMaxFrameSize; + int count = 64; + var headerValue = new string('a', frameSize * count); // 1 MB value + await InitializeConnectionAsync(async context => + { + context.Response.Headers["my"] = headerValue; + await context.Response.WriteAsync("Hello World"); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Just the StatusCode gets written before aborting in the continuation frame + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + for (int i = 0; i < count; i++) + { + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + } + + // One more frame because of the header name + size of header value + size header name + 2 * H encoding + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 8, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + } + [Fact] public async Task WriteAsync_PreCancelledCancellationToken_DoesNotAbort() { From f8f8ff972aaf2815e0c7547090aa4f4e6c3d66f9 Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 18 May 2024 11:18:13 +0200 Subject: [PATCH 12/27] Review comments - removing an unneeded if condition --- .../Core/src/Internal/Http2/Http2FrameWriter.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index c4c2719f60fe..d9e30a841e1c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -370,10 +370,7 @@ public void UpdateMaxFrameSize(int maxFrameSize) { lock (_writeLock) { - if (_maxFrameSize != maxFrameSize) - { - _maxFrameSize = maxFrameSize; - } + _maxFrameSize = maxFrameSize; } } @@ -509,7 +506,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[0.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. + var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); if (done == HPackHeaderWriter.HeaderWriteResult.Done) @@ -518,11 +515,12 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _outgoingFrame.PayloadLength = payloadLength; _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; WriteHeaderUnsynchronized(); - _outputWriter.Write(buffer[0..payloadLength]); + _outputWriter.Write(buffer[..payloadLength]); } else { - // More headers sent in CONTINUATION frames. + // Sending the current HEADERS frame to output and the remaining headers + // are processed by FinishWritingHeadersUnsynchronized using CONTINUATION frames. _headerEncodingBuffer.Advance(payloadLength); SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); FinishWritingHeadersUnsynchronized(streamId); @@ -571,7 +569,7 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in { _headersEnumerator.Initialize(headers); _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[0..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); if (done == HPackHeaderWriter.HeaderWriteResult.Done) { @@ -580,7 +578,8 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } else if (done == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) { - // More headers sent in CONTINUATION frames. + // Sending the current HEADERS frame to output and the remaining headers + // are processed by FinishWritingHeadersUnsynchronized using CONTINUATION frames. _headerEncodingBuffer.Advance(payloadLength); SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); FinishWritingHeadersUnsynchronized(streamId); From 994b27d72ba31c1785d2b7ab39ee0800e10050a7 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 22 May 2024 00:17:13 +0200 Subject: [PATCH 13/27] Sketch --- .../src/Internal/Http2/Http2FrameWriter.cs | 100 +++++++++--------- .../Kestrel/shared/HPackHeaderWriter.cs | 18 ++++ 2 files changed, 66 insertions(+), 52 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index d9e30a841e1c..561a1c175683 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -507,24 +507,9 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); _headerEncodingBuffer.ResetWrittenCount(); var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - var done = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); - Debug.Assert(done != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); - if (done == HPackHeaderWriter.HeaderWriteResult.Done) - { - // Fast path, only a single HEADER frame. - _outgoingFrame.PayloadLength = payloadLength; - _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; - WriteHeaderUnsynchronized(); - _outputWriter.Write(buffer[..payloadLength]); - } - else - { - // Sending the current HEADERS frame to output and the remaining headers - // are processed by FinishWritingHeadersUnsynchronized using CONTINUATION frames. - _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); - FinishWritingHeadersUnsynchronized(streamId); - } + var writeResult = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + Debug.Assert(writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); + FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. // Since we allow custom header encoders we don't know what type of exceptions to expect. @@ -564,31 +549,11 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in // In the case of the trailers, there is no status header to be written, so even the first call to BeginEncodeHeaders can return BufferTooSmall. _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_STREAM, streamId); var bufferSize = _headerEncodingBuffer.Capacity; - HPackHeaderWriter.HeaderWriteResult done; - do - { - _headersEnumerator.Initialize(headers); - _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - done = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); - if (done == HPackHeaderWriter.HeaderWriteResult.Done) - { - _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, endOfHeaders: true, isFramePrepared: true); - } - else if (done == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) - { - // Sending the current HEADERS frame to output and the remaining headers - // are processed by FinishWritingHeadersUnsynchronized using CONTINUATION frames. - _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, endOfHeaders: false, isFramePrepared: true); - FinishWritingHeadersUnsynchronized(streamId); - } - else - { - bufferSize *= 2; - } - } while (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall); + _headersEnumerator.Initialize(headers); + _headerEncodingBuffer.ResetWrittenCount(); + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. + var writeResult = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. // Since we allow custom header encoders we don't know what type of exceptions to expect. @@ -602,8 +567,9 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void SplitHeaderFramesToOutput(int streamId, bool endOfHeaders, bool isFramePrepared) + private void SplitHeaderFramesToOutput(int streamId, int payloadLength, bool endOfHeaders, bool isFramePrepared) { + _headerEncodingBuffer.Advance(payloadLength); var dataToFrame = _headerEncodingBuffer.WrittenSpan; var shouldPrepareFrame = !isFramePrepared; while (dataToFrame.Length > 0) @@ -628,26 +594,56 @@ private void SplitHeaderFramesToOutput(int streamId, bool endOfHeaders, bool isF } } - private void FinishWritingHeadersUnsynchronized(int streamId) + private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult writeResult) { - HPackHeaderWriter.HeaderWriteResult done; + if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done && payloadLength <= _maxFrameSize) + { + // Fast path, only a single HEADER frame. + _headerEncodingBuffer.Advance(payloadLength); + _outgoingFrame.PayloadLength = payloadLength; + _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; + WriteHeaderUnsynchronized(); + _outputWriter.Write(_headerEncodingBuffer.WrittenSpan); + return; + } + else if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) + { + SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: true, isFramePrepared: true); + return; + } + + // TODO: It seems like, if this grows, it stays at the new size for the lifetime of the connection - maybe we want to allocate a new buffer instead? var bufferSize = _headerEncodingBuffer.Capacity; - do + + // This may happen in case of the TRAILERS after the initial encode operation. + while (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { + Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); _headerEncodingBuffer.ResetWrittenCount(); + bufferSize *= 2; var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; - done = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); - if (done == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + } + + // Either Done or MoreFrames (this handles writeResult with BufferTooSmall or MoreHeaders). + SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); + + // HEADERS and zero or more CONTINUATIONS sent - all subsequent frames are (unprepared) CONTINUATIONs + while (writeResult != HPackHeaderWriter.HeaderWriteResult.Done) + { + _headerEncodingBuffer.ResetWrittenCount(); + var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; + writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + if (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { bufferSize *= 2; } else { // In case of Done or MoreHeaders: write to output. - _headerEncodingBuffer.Advance(payloadLength); - SplitHeaderFramesToOutput(streamId, endOfHeaders: done == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); + SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); } - } while (done != HPackHeaderWriter.HeaderWriteResult.Done); + } } /* Padding is not implemented diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 2c5cff09b610..68fa304d5057 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -81,6 +81,24 @@ public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnco return done; } + /// + /// Begin encoding headers in the first HEADERS frame without stepping the iterator. + /// + public static HeaderWriteResult RetryBeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + { + length = 0; + + if (!hpackEncoder.EnsureDynamicTableSizeUpdate(buffer, out var sizeUpdateLength)) + { + throw new HPackEncodingException(SR.net_http_hpack_encode_failure); + } + length += sizeUpdateLength; + + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); + length += headersLength; + return done; + } + /// /// Continue encoding headers in the next HEADERS frame. The enumerator should already have a current value. /// From a77a93009600d383ae852a681e936a17230b4aa8 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 22 May 2024 22:18:42 +0200 Subject: [PATCH 14/27] using arraypool instead of arraybufferwriter --- .../src/Internal/Http2/Http2FrameWriter.cs | 78 ++++++++++--------- .../Http2/Http2StreamTests.cs | 55 ++++++++++++- 2 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 561a1c175683..60722a9de9c7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -72,7 +72,7 @@ internal sealed class Http2FrameWriter private readonly bool _scheduleInline; private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; - private readonly ArrayBufferWriter _headerEncodingBuffer; + private byte[] _headerEncodingBuffer; private long _unflushedBytes; private bool _completed; @@ -107,7 +107,7 @@ public Http2FrameWriter( _flusher = new TimingPipeFlusher(timeoutControl, serviceContext.Log); _flusher.Initialize(_outputWriter); _outgoingFrame = new Http2Frame(); - _headerEncodingBuffer = new ArrayBufferWriter(_maxFrameSize); + _headerEncodingBuffer = new byte[_maxFrameSize]; _scheduleInline = serviceContext.Scheduler == PipeScheduler.Inline; _hpackEncoder = new DynamicHPackEncoder(serviceContext.ServerOptions.AllowResponseHeaderCompression); @@ -370,7 +370,11 @@ public void UpdateMaxFrameSize(int maxFrameSize) { lock (_writeLock) { - _maxFrameSize = maxFrameSize; + if (_maxFrameSize != maxFrameSize) + { + _maxFrameSize = maxFrameSize; + _headerEncodingBuffer = new byte[_maxFrameSize]; + } } } @@ -505,9 +509,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht // In the case of the headers, there is always a status header to be returned, so BeginEncodeHeaders will not return BufferTooSmall. _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); - _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(_maxFrameSize)[.._maxFrameSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - var writeResult = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + var writeResult = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, _headerEncodingBuffer, out var payloadLength); Debug.Assert(writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } @@ -548,11 +550,8 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in { // In the case of the trailers, there is no status header to be written, so even the first call to BeginEncodeHeaders can return BufferTooSmall. _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_STREAM, streamId); - var bufferSize = _headerEncodingBuffer.Capacity; _headersEnumerator.Initialize(headers); - _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; // GetSpan might return more data that can result in a less deterministic behavior on the way headers are split into frames. - var writeResult = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out var payloadLength); + var writeResult = HPackHeaderWriter.BeginEncodeHeaders(_hpackEncoder, _headersEnumerator, _headerEncodingBuffer, out var payloadLength); FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. @@ -567,10 +566,8 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void SplitHeaderFramesToOutput(int streamId, int payloadLength, bool endOfHeaders, bool isFramePrepared) + private void SplitHeaderFramesToOutput(int streamId, ReadOnlySpan dataToFrame, bool endOfHeaders, bool isFramePrepared) { - _headerEncodingBuffer.Advance(payloadLength); - var dataToFrame = _headerEncodingBuffer.WrittenSpan; var shouldPrepareFrame = !isFramePrepared; while (dataToFrame.Length > 0) { @@ -596,44 +593,49 @@ private void SplitHeaderFramesToOutput(int streamId, int payloadLength, bool end private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult writeResult) { - if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done && payloadLength <= _maxFrameSize) + Debug.Assert(payloadLength <= _maxFrameSize, "The initial payload lengths is written to _headerEncodingBuffer with size of _maxFrameSize"); + var bufferSize = _maxFrameSize; + if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) { // Fast path, only a single HEADER frame. - _headerEncodingBuffer.Advance(payloadLength); _outgoingFrame.PayloadLength = payloadLength; _outgoingFrame.HeadersFlags |= Http2HeadersFrameFlags.END_HEADERS; WriteHeaderUnsynchronized(); - _outputWriter.Write(_headerEncodingBuffer.WrittenSpan); + _outputWriter.Write(_headerEncodingBuffer.AsSpan(0, payloadLength)); return; } - else if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) + else if (writeResult == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) { - SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: true, isFramePrepared: true); - return; + _outgoingFrame.PayloadLength = payloadLength; + WriteHeaderUnsynchronized(); + _outputWriter.Write(_headerEncodingBuffer.AsSpan(0, payloadLength)); } - - // TODO: It seems like, if this grows, it stays at the new size for the lifetime of the connection - maybe we want to allocate a new buffer instead? - var bufferSize = _headerEncodingBuffer.Capacity; - - // This may happen in case of the TRAILERS after the initial encode operation. - while (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + else { - Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); - _headerEncodingBuffer.ResetWrittenCount(); - bufferSize *= 2; - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; - writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + // This may happen in case of the TRAILERS after the initial encode operation. + while (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + { + Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); + bufferSize *= 2; + var headerBuffer = ArrayPool.Shared.Rent(bufferSize); + var buffer = headerBuffer.AsSpan(0, bufferSize); + writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + if (writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + { + SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); + } + ArrayPool.Shared.Return(headerBuffer); + } } - // Either Done or MoreFrames (this handles writeResult with BufferTooSmall or MoreHeaders). - SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); + bufferSize = _maxFrameSize; // HEADERS and zero or more CONTINUATIONS sent - all subsequent frames are (unprepared) CONTINUATIONs while (writeResult != HPackHeaderWriter.HeaderWriteResult.Done) { - _headerEncodingBuffer.ResetWrittenCount(); - var buffer = _headerEncodingBuffer.GetSpan(bufferSize)[..bufferSize]; - writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); + byte[]? headerBuffer = null; + Span buffer = bufferSize == _maxFrameSize ? _headerEncodingBuffer : headerBuffer = ArrayPool.Shared.Rent(bufferSize); + writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer[..bufferSize], out payloadLength); if (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { bufferSize *= 2; @@ -641,7 +643,11 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, else { // In case of Done or MoreHeaders: write to output. - SplitHeaderFramesToOutput(streamId, payloadLength, endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); + SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); + } + if (headerBuffer != null) + { + ArrayPool.Shared.Return(headerBuffer); } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 4afbca7c7be6..d1b3086e4f3a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2584,7 +2584,7 @@ await ExpectAsync(Http2FrameType.DATA, } [Fact] - public async Task ResponseTrailers_TooLong_SplitsTrailersToContinuationFrames() + public async Task ResponseTrailers_SingleLong_SplitsTrailersToContinuationFrames() { var trailerValue = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); await InitializeConnectionAsync(async context => @@ -2625,6 +2625,59 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal(trailerValue, _decodedHeaders["too_long"]); } + [Fact] + public async Task ResponseTrailers_ShortThenLongThenShort_SplitsTrailers() + { + var trailerValue = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + string shortValue = "testValue"; + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("short", shortValue); + context.Response.AppendTrailer("long", trailerValue); + context.Response.AppendTrailer("short2", shortValue); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + var trailerFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 17, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 1); + + var trailierContinuation1 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + var trailierContinuation2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 27, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); + + var buffer = new byte[trailerFrame.PayloadLength + trailierContinuation1.PayloadLength + trailierContinuation2.PayloadLength]; + trailerFrame.PayloadSequence.CopyTo(buffer); + trailierContinuation1.PayloadSequence.CopyTo(buffer.AsSpan(trailerFrame.PayloadLength)); + trailierContinuation2.PayloadSequence.CopyTo(buffer.AsSpan(trailerFrame.PayloadLength + trailierContinuation1.PayloadLength)); + _hpackDecoder.Decode(buffer, endHeaders: true, handler: this); + Assert.Equal(3, _decodedHeaders.Count); + Assert.Equal(trailerValue, _decodedHeaders["long"]); + Assert.Equal(shortValue, _decodedHeaders["short"]); + Assert.Equal(shortValue, _decodedHeaders["short2"]); + } + [Fact] public async Task ResponseTrailers_WithLargeUnflushedData_DataExceedsFlowControlAvailableAndNotSentWithTrailers() { From 4f14b0c732f84c0e377e465ff00e0d1c27baa9f9 Mon Sep 17 00:00:00 2001 From: ladeak Date: Fri, 24 May 2024 20:48:03 +0200 Subject: [PATCH 15/27] Re-using the rented buffers --- .../src/Internal/Http2/Http2FrameWriter.cs | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 60722a9de9c7..c54a4d5b15d6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -595,6 +595,8 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, { Debug.Assert(payloadLength <= _maxFrameSize, "The initial payload lengths is written to _headerEncodingBuffer with size of _maxFrameSize"); var bufferSize = _maxFrameSize; + byte[]? largeHeaderBuffer = null; + Span buffer; if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) { // Fast path, only a single HEADER frame. @@ -617,38 +619,46 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, { Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); bufferSize *= 2; - var headerBuffer = ArrayPool.Shared.Rent(bufferSize); - var buffer = headerBuffer.AsSpan(0, bufferSize); + largeHeaderBuffer = ArrayPool.Shared.Rent(bufferSize); + buffer = largeHeaderBuffer.AsSpan(0, bufferSize); writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); if (writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); } - ArrayPool.Shared.Return(headerBuffer); + ArrayPool.Shared.Return(largeHeaderBuffer); + } + if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) + { + return; } } - bufferSize = _maxFrameSize; - // HEADERS and zero or more CONTINUATIONS sent - all subsequent frames are (unprepared) CONTINUATIONs + bufferSize = _maxFrameSize; + buffer = _headerEncodingBuffer; while (writeResult != HPackHeaderWriter.HeaderWriteResult.Done) { - byte[]? headerBuffer = null; - Span buffer = bufferSize == _maxFrameSize ? _headerEncodingBuffer : headerBuffer = ArrayPool.Shared.Rent(bufferSize); writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer[..bufferSize], out payloadLength); if (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { + if (largeHeaderBuffer != null) + { + ArrayPool.Shared.Return(largeHeaderBuffer); + } bufferSize *= 2; + largeHeaderBuffer = ArrayPool.Shared.Rent(bufferSize); + buffer = largeHeaderBuffer.AsSpan(0, bufferSize); } else { // In case of Done or MoreHeaders: write to output. SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); } - if (headerBuffer != null) - { - ArrayPool.Shared.Return(headerBuffer); - } + } + if (largeHeaderBuffer != null) + { + ArrayPool.Shared.Return(largeHeaderBuffer); } } From 46be03f34a089caacceaa1b593992258425cfdfd Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 30 May 2024 10:48:35 +0200 Subject: [PATCH 16/27] Preserving the size of the temporary larger buffer within the framewriter. This way if the same large header is written multiple times over the lifespan of a connection, it does not need to go through the loops of increasing the headers repetitively. --- .../src/Internal/Http2/Http2FrameWriter.cs | 30 ++-- .../Core/test/Http2/Http2FrameWriterTests.cs | 7 + .../Http2/Http2StreamTests.cs | 132 ++++++++++++++++++ 3 files changed, 160 insertions(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index c54a4d5b15d6..783e3715bbf2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -29,6 +29,8 @@ internal sealed class Http2FrameWriter /// TODO (https://github.com/dotnet/aspnetcore/issues/51309): eliminate this limit. private const string MaximumFlowControlQueueSizeProperty = "Microsoft.AspNetCore.Server.Kestrel.Http2.MaxConnectionFlowControlQueueSize"; + private const int HeaderBufferSizeMultiplier = 2; + private static readonly int? AppContextMaximumFlowControlQueueSize = GetAppContextMaximumFlowControlQueueSize(); private static int? GetAppContextMaximumFlowControlQueueSize() @@ -73,6 +75,7 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private byte[] _headerEncodingBuffer; + private int _headersEncodingLargeBufferSize = Http2PeerSettings.MinAllowedMaxFrameSize * HeaderBufferSizeMultiplier; private long _unflushedBytes; private bool _completed; @@ -372,6 +375,12 @@ public void UpdateMaxFrameSize(int maxFrameSize) { if (_maxFrameSize != maxFrameSize) { + // Safe multiply, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. + // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 + if (_maxFrameSize * HeaderBufferSizeMultiplier == _headersEncodingLargeBufferSize) + { + _headersEncodingLargeBufferSize = maxFrameSize * HeaderBufferSizeMultiplier; + } _maxFrameSize = maxFrameSize; _headerEncodingBuffer = new byte[_maxFrameSize]; } @@ -594,7 +603,6 @@ private void SplitHeaderFramesToOutput(int streamId, ReadOnlySpan dataToFr private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult writeResult) { Debug.Assert(payloadLength <= _maxFrameSize, "The initial payload lengths is written to _headerEncodingBuffer with size of _maxFrameSize"); - var bufferSize = _maxFrameSize; byte[]? largeHeaderBuffer = null; Span buffer; if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) @@ -615,18 +623,23 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, else { // This may happen in case of the TRAILERS after the initial encode operation. + // The _maxFrameSize sized _headerEncodingBuffer was too small. while (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); - bufferSize *= 2; - largeHeaderBuffer = ArrayPool.Shared.Rent(bufferSize); - buffer = largeHeaderBuffer.AsSpan(0, bufferSize); + largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); + buffer = largeHeaderBuffer.AsSpan(0, _headersEncodingLargeBufferSize); writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); if (writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); } + else + { + _headersEncodingLargeBufferSize *= HeaderBufferSizeMultiplier; + } ArrayPool.Shared.Return(largeHeaderBuffer); + largeHeaderBuffer = null; } if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) { @@ -635,20 +648,19 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, } // HEADERS and zero or more CONTINUATIONS sent - all subsequent frames are (unprepared) CONTINUATIONs - bufferSize = _maxFrameSize; buffer = _headerEncodingBuffer; while (writeResult != HPackHeaderWriter.HeaderWriteResult.Done) { - writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer[..bufferSize], out payloadLength); + writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); if (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) { if (largeHeaderBuffer != null) { ArrayPool.Shared.Return(largeHeaderBuffer); + _headersEncodingLargeBufferSize *= HeaderBufferSizeMultiplier; } - bufferSize *= 2; - largeHeaderBuffer = ArrayPool.Shared.Rent(bufferSize); - buffer = largeHeaderBuffer.AsSpan(0, bufferSize); + largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); + buffer = largeHeaderBuffer.AsSpan(0, _headersEncodingLargeBufferSize); } else { diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs index f3887d7a0abe..ea2ec9ba5609 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2FrameWriterTests.cs @@ -86,6 +86,13 @@ public async Task WriteHeader_UnsetsReservedBit() Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x00 }, payload.Skip(5).Take(4).ToArray()); } + + [Fact] + public void UpdateMaxFrameSize_To_ProtocolMaximum() + { + var sut = CreateFrameWriter(new Pipe()); + sut.UpdateMaxFrameSize((int)Math.Pow(2, 24) - 1); + } } public static class PipeReaderExtensions diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index d1b3086e4f3a..73658853848f 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2625,6 +2625,52 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal(trailerValue, _decodedHeaders["too_long"]); } + [Fact] + public async Task ResponseTrailers_DoubleLong_SplitsTrailersToContinuationFrames() + { + var trailerValue = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("too_long", trailerValue); + context.Response.AppendTrailer("too_long2", trailerValue); + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 13, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 14, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); + } + [Fact] public async Task ResponseTrailers_ShortThenLongThenShort_SplitsTrailers() { @@ -2678,6 +2724,92 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal(shortValue, _decodedHeaders["short2"]); } + [Fact] + public async Task LongResponseHeader_FollowedBy_LongResponseTrailer_SplitsTrailersToContinuationFrames() + { + var value = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + await InitializeConnectionAsync(async context => + { + context.Response.Headers["too_long_header"] = value; + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("too_long_trailer", value); + }); + + // Stream 1 + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + // Response headers + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 20, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + // Data + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + // Trailers + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 21, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + // Stream 3 + await StartStreamAsync(3, _browserRequestHeaders, endStream: true); + + // Response headers + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 2, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 3); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 3); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 20, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + + // Data + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + + // Trailers + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 3); + + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 21, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + + await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false).DefaultTimeout(); + } + [Fact] public async Task ResponseTrailers_WithLargeUnflushedData_DataExceedsFlowControlAvailableAndNotSentWithTrailers() { From 9857dcb3bf552cdf850970e1f9430b2a3449b372 Mon Sep 17 00:00:00 2001 From: ladeak Date: Fri, 31 May 2024 20:47:51 +0200 Subject: [PATCH 17/27] Adjusting the logic of UpdateMaxFrameSize for setting a new _headersEncodingLargeBufferSize --- .../Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 783e3715bbf2..33bedb4ea75f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -377,10 +377,7 @@ public void UpdateMaxFrameSize(int maxFrameSize) { // Safe multiply, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 - if (_maxFrameSize * HeaderBufferSizeMultiplier == _headersEncodingLargeBufferSize) - { - _headersEncodingLargeBufferSize = maxFrameSize * HeaderBufferSizeMultiplier; - } + _headersEncodingLargeBufferSize = Math.Max(_headersEncodingLargeBufferSize, maxFrameSize * HeaderBufferSizeMultiplier); _maxFrameSize = maxFrameSize; _headerEncodingBuffer = new byte[_maxFrameSize]; } From 95c9553228ce7246c0a6ecc9ae2fcff28a6bc92d Mon Sep 17 00:00:00 2001 From: ladeak Date: Fri, 31 May 2024 21:02:46 +0200 Subject: [PATCH 18/27] Adjusting the logic of _headersEncodingLargeBufferSize to avoid 0 values. --- .../Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 33bedb4ea75f..b0fe953a44fb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -633,7 +633,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, } else { - _headersEncodingLargeBufferSize *= HeaderBufferSizeMultiplier; + _headersEncodingLargeBufferSize = int.Max(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier, _maxFrameSize); } ArrayPool.Shared.Return(largeHeaderBuffer); largeHeaderBuffer = null; @@ -654,7 +654,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, if (largeHeaderBuffer != null) { ArrayPool.Shared.Return(largeHeaderBuffer); - _headersEncodingLargeBufferSize *= HeaderBufferSizeMultiplier; + _headersEncodingLargeBufferSize = int.Max(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier, _maxFrameSize); } largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); buffer = largeHeaderBuffer.AsSpan(0, _headersEncodingLargeBufferSize); From c4b93d674013aac6850a08a4923510d87c5e54d8 Mon Sep 17 00:00:00 2001 From: ladeak Date: Fri, 31 May 2024 21:53:48 +0200 Subject: [PATCH 19/27] Extenting Benachmark with headers sized 0, 10KB and 20KB --- .../Http2/Http2FrameWriterBenchmark.cs | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs index 7b934d2101c4..03589ed07b77 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2FrameWriterBenchmark.cs @@ -4,11 +4,13 @@ using System.Buffers; using System.IO.Pipelines; using BenchmarkDotNet.Attributes; + using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; +using Microsoft.AspNetCore.InternalTesting; namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks; @@ -42,11 +44,26 @@ public void GlobalSetup() "TestConnectionId", _memoryPool, serviceContext); + } + + private int _largeHeaderSize; - _responseHeaders = new HttpResponseHeaders(); - var headers = (IHeaderDictionary)_responseHeaders; - headers.ContentType = "application/json"; - headers.ContentLength = 1024; + [Params(0, 10, 20)] + public int LargeHeaderSize + { + get => _largeHeaderSize; + set + { + _largeHeaderSize = value; + _responseHeaders = new HttpResponseHeaders(); + var headers = (IHeaderDictionary)_responseHeaders; + headers.ContentType = "application/json"; + headers.ContentLength = 1024; + if (value > 0) + { + headers.Add("my", new string('a', value * 1024)); + } + } } [Benchmark] From 3fe0a7324abf80f39ec20cc6be1f628627a92480 Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 1 Jun 2024 09:32:13 +0200 Subject: [PATCH 20/27] Remove int.Max on header size increase and using checked arithmetic instead. --- .../Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index b0fe953a44fb..ab0da704897a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -377,7 +377,7 @@ public void UpdateMaxFrameSize(int maxFrameSize) { // Safe multiply, MaxFrameSize is limited to 2^24-1 bytes by the protocol and by Http2PeerSettings. // Ref: https://datatracker.ietf.org/doc/html/rfc7540#section-4.2 - _headersEncodingLargeBufferSize = Math.Max(_headersEncodingLargeBufferSize, maxFrameSize * HeaderBufferSizeMultiplier); + _headersEncodingLargeBufferSize = int.Max(_headersEncodingLargeBufferSize, maxFrameSize * HeaderBufferSizeMultiplier); _maxFrameSize = maxFrameSize; _headerEncodingBuffer = new byte[_maxFrameSize]; } @@ -633,7 +633,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, } else { - _headersEncodingLargeBufferSize = int.Max(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier, _maxFrameSize); + _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } ArrayPool.Shared.Return(largeHeaderBuffer); largeHeaderBuffer = null; @@ -654,7 +654,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, if (largeHeaderBuffer != null) { ArrayPool.Shared.Return(largeHeaderBuffer); - _headersEncodingLargeBufferSize = int.Max(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier, _maxFrameSize); + _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); buffer = largeHeaderBuffer.AsSpan(0, _headersEncodingLargeBufferSize); From 0dfc97bef8c66fc0e6cbf159c4933eec6f28eef2 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 3 Jun 2024 13:24:02 +0800 Subject: [PATCH 21/27] Tests --- .../Http2/Http2StreamTests.cs | 198 ++++++++++++++++-- .../Http2/Http2RequestTests.cs | 40 ++++ .../Interop.FunctionalTests/HttpHelpers.cs | 7 +- 3 files changed, 221 insertions(+), 24 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 73658853848f..8628af4117d9 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2625,6 +2625,99 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal(trailerValue, _decodedHeaders["too_long"]); } + [Fact] + public async Task ResponseTrailers_ShortHeadersBeforeSingleLong_MultipleRequests_ShortHeadersInDynamicTable() + { + var trailerValue = new string('a', (int)Http2PeerSettings.DefaultMaxFrameSize); + await InitializeConnectionAsync(async context => + { + await context.Response.WriteAsync("Hello World"); + context.Response.AppendTrailer("a-key", "a-value"); + context.Response.AppendTrailer("b-key", "b-value"); + context.Response.AppendTrailer("too_long", trailerValue); + }); + + // Request 1 + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 32, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + var request1TrailerFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 30, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 1); + + var request1TrailierContinuation1 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + + var request1TrailierContinuation2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 13, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + _hpackDecoder.Decode(request1TrailerFrame.PayloadSequence, endHeaders: false, handler: this); + Assert.Equal("a-value", _decodedHeaders["a-key"]); + Assert.Equal("b-value", _decodedHeaders["b-key"]); + + _decodedHeaders.Clear(); + _hpackDecoder.Decode(request1TrailierContinuation1.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + _hpackDecoder.Decode(request1TrailierContinuation2.PayloadSequence, endHeaders: true, handler: this); + Assert.Equal(trailerValue, _decodedHeaders["too_long"]); + + // Request 2 + await StartStreamAsync(3, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 2, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + + var request2TrailerFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 2, + withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, + withStreamId: 3); + + var request2TrailierContinuation1 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 16384, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 3); + + var request2TrailierContinuation2 = await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: 13, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + + _hpackDecoder.Decode(request2TrailerFrame.PayloadSequence, endHeaders: false, handler: this); + Assert.Equal("a-value", _decodedHeaders["a-key"]); + Assert.Equal("b-value", _decodedHeaders["b-key"]); + + _decodedHeaders.Clear(); + _hpackDecoder.Decode(request2TrailierContinuation1.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + _hpackDecoder.Decode(request2TrailierContinuation2.PayloadSequence, endHeaders: true, handler: this); + Assert.Equal(trailerValue, _decodedHeaders["too_long"]); + + await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false).DefaultTimeout(); + } + [Fact] public async Task ResponseTrailers_DoubleLong_SplitsTrailersToContinuationFrames() { @@ -2648,26 +2741,39 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 1); - await ExpectAsync(Http2FrameType.HEADERS, + var frame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 13, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Equal(trailerValue, _decodedHeaders["too_long"]); + _decodedHeaders.Clear(); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 14, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: true, handler: this); + Assert.Equal(trailerValue, _decodedHeaders["too_long2"]); + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); } @@ -2701,27 +2807,29 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, withStreamId: 1); + _hpackDecoder.Decode(trailerFrame.PayloadSequence, endHeaders: false, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(shortValue, _decodedHeaders["short"]); + _decodedHeaders.Clear(); + var trailierContinuation1 = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); + _hpackDecoder.Decode(trailierContinuation1.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + var trailierContinuation2 = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 27, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); - - var buffer = new byte[trailerFrame.PayloadLength + trailierContinuation1.PayloadLength + trailierContinuation2.PayloadLength]; - trailerFrame.PayloadSequence.CopyTo(buffer); - trailierContinuation1.PayloadSequence.CopyTo(buffer.AsSpan(trailerFrame.PayloadLength)); - trailierContinuation2.PayloadSequence.CopyTo(buffer.AsSpan(trailerFrame.PayloadLength + trailierContinuation1.PayloadLength)); - _hpackDecoder.Decode(buffer, endHeaders: true, handler: this); - Assert.Equal(3, _decodedHeaders.Count); + _hpackDecoder.Decode(trailierContinuation2.PayloadSequence, endHeaders: true, handler: this); Assert.Equal(trailerValue, _decodedHeaders["long"]); - Assert.Equal(shortValue, _decodedHeaders["short"]); Assert.Equal(shortValue, _decodedHeaders["short2"]); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false).DefaultTimeout(); } [Fact] @@ -2739,21 +2847,35 @@ await InitializeConnectionAsync(async context => await StartStreamAsync(1, _browserRequestHeaders, endStream: true); // Response headers - await ExpectAsync(Http2FrameType.HEADERS, + var frame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 32, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Equal(2, _decodedHeaders.Count); + Assert.Equal("200", _decodedHeaders[":status"]); + Assert.Equal("Sat, 01 Jan 2000 00:00:00 GMT", _decodedHeaders["date"]); + _decodedHeaders.Clear(); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 20, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: true, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(value, _decodedHeaders["too_long_header"]); + _decodedHeaders.Clear(); + // Data await ExpectAsync(Http2FrameType.DATA, withLength: 11, @@ -2761,35 +2883,57 @@ await ExpectAsync(Http2FrameType.DATA, withStreamId: 1); // Trailers - await ExpectAsync(Http2FrameType.HEADERS, + frame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, withStreamId: 1); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 21, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: true, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(value, _decodedHeaders["too_long_trailer"]); + _decodedHeaders.Clear(); + // Stream 3 await StartStreamAsync(3, _browserRequestHeaders, endStream: true); // Response headers - await ExpectAsync(Http2FrameType.HEADERS, + frame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 2, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 3); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Equal(2, _decodedHeaders.Count); + Assert.Equal("200", _decodedHeaders[":status"]); + Assert.Equal("Sat, 01 Jan 2000 00:00:00 GMT", _decodedHeaders["date"]); + _decodedHeaders.Clear(); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.NONE, withStreamId: 3); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 20, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 3); + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: true, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(value, _decodedHeaders["too_long_header"]); + _decodedHeaders.Clear(); + // Data await ExpectAsync(Http2FrameType.DATA, withLength: 11, @@ -2797,16 +2941,24 @@ await ExpectAsync(Http2FrameType.DATA, withStreamId: 3); // Trailers - await ExpectAsync(Http2FrameType.HEADERS, + frame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 16384, withFlags: (byte)Http2HeadersFrameFlags.END_STREAM, withStreamId: 3); - await ExpectAsync(Http2FrameType.CONTINUATION, + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: false, handler: this); + Assert.Empty(_decodedHeaders); + + frame = await ExpectAsync(Http2FrameType.CONTINUATION, withLength: 21, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 3); + _hpackDecoder.Decode(frame.PayloadSequence, endHeaders: true, handler: this); + Assert.Single(_decodedHeaders); + Assert.Equal(value, _decodedHeaders["too_long_trailer"]); + _decodedHeaders.Clear(); + await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false).DefaultTimeout(); } diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs index 2d58d859b81b..86af821432b6 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs @@ -89,6 +89,46 @@ public async Task GET_Metrics_HttpProtocolAndTlsSet() } } + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task GET_LargeResponseHeader_Success(bool largeValue, bool largeKey) + { + // Arrange + var longKey = "key-" + new string('$', largeKey ? 128 * 1024 : 1); + var longValue = "value-" + new string('!', largeValue ? 128 * 1024 : 1); + var builder = CreateHostBuilder( + c => + { + c.Response.Headers["test"] = "abc"; + c.Response.Headers[longKey] = longValue; + return Task.CompletedTask; + }, + protocol: HttpProtocols.Http2, + plaintext: true); + + using (var host = builder.Build()) + { + await host.StartAsync(); + var client = HttpHelpers.CreateClient(maxResponseHeadersLength: 1024); + + // Act + var request1 = new HttpRequestMessage(HttpMethod.Get, $"http://127.0.0.1:{host.GetPort()}/"); + request1.Version = HttpVersion.Version20; + request1.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + + var response = await client.SendAsync(request1, CancellationToken.None); + response.EnsureSuccessStatusCode(); + + // Assert + Assert.Equal("abc", response.Headers.GetValues("test").Single()); + Assert.Equal(longValue, response.Headers.GetValues(longKey).Single()); + + await host.StopAsync(); + } + } + [Fact] public async Task GET_NoTLS_Http11RequestToHttp2Endpoint_400Result() { diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs index cc6f7bacb4f2..92bb131c4ed4 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs @@ -35,7 +35,7 @@ public static HttpProtocolException GetProtocolException(this Exception ex) throw new Exception($"Couldn't find {nameof(HttpProtocolException)}. Original error: {ex}"); } - public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, TimeSpan? expect100ContinueTimeout = null, bool includeClientCert = false) + public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, TimeSpan? expect100ContinueTimeout = null, bool includeClientCert = false, int? maxResponseHeadersLength = null) { var handler = new SocketsHttpHandler(); handler.SslOptions = new System.Net.Security.SslClientAuthenticationOptions @@ -55,6 +55,11 @@ public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, Time handler.PooledConnectionIdleTimeout = idleTimeout.Value; } + if (maxResponseHeadersLength != null) + { + handler.MaxResponseHeadersLength = maxResponseHeadersLength.Value; + } + return new HttpMessageInvoker(handler); } From 79be13bbfb5e635fdc0d026e130611b8f3004d1a Mon Sep 17 00:00:00 2001 From: ladeak Date: Mon, 3 Jun 2024 09:56:02 +0200 Subject: [PATCH 22/27] Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs Co-authored-by: James Newton-King --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index ab0da704897a..b033c6084728 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -516,7 +516,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); var writeResult = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, _headerEncodingBuffer, out var payloadLength); - Debug.Assert(writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "Oversized frames should not be returned, because this always writes the status."); + Debug.Assert(writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "This always writes the status as the first header, and it should never be an over the buffer size."); FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. From 8773062db5035ffb6815fbfe7f95231cda42e050 Mon Sep 17 00:00:00 2001 From: ladeak Date: Mon, 3 Jun 2024 10:21:44 +0200 Subject: [PATCH 23/27] Review feedback: HeaderWriteResult not an inner type, adding comments on size increase --- .../src/Internal/Http2/Http2FrameWriter.cs | 27 +++++++----- .../Core/test/Http2/Http2HPackEncoderTests.cs | 44 +++++++++---------- .../Kestrel/shared/HPackHeaderWriter.cs | 25 +++++------ .../test/PipeWriterHttp2FrameExtensions.cs | 7 +-- .../Http2/Http2TestBase.cs | 6 +-- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index b033c6084728..7b2947e54bff 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -516,7 +516,7 @@ private void WriteResponseHeadersUnsynchronized(int streamId, int statusCode, Ht _headersEnumerator.Initialize(headers); _outgoingFrame.PrepareHeaders(headerFrameFlags, streamId); var writeResult = HPackHeaderWriter.BeginEncodeHeaders(statusCode, _hpackEncoder, _headersEnumerator, _headerEncodingBuffer, out var payloadLength); - Debug.Assert(writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, "This always writes the status as the first header, and it should never be an over the buffer size."); + Debug.Assert(writeResult != HeaderWriteResult.BufferTooSmall, "This always writes the status as the first header, and it should never be an over the buffer size."); FinishWritingHeadersUnsynchronized(streamId, payloadLength, writeResult); } // Any exception from the HPack encoder can leave the dynamic table in a corrupt state. @@ -597,12 +597,12 @@ private void SplitHeaderFramesToOutput(int streamId, ReadOnlySpan dataToFr } } - private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HPackHeaderWriter.HeaderWriteResult writeResult) + private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, HeaderWriteResult writeResult) { Debug.Assert(payloadLength <= _maxFrameSize, "The initial payload lengths is written to _headerEncodingBuffer with size of _maxFrameSize"); byte[]? largeHeaderBuffer = null; Span buffer; - if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) + if (writeResult == HeaderWriteResult.Done) { // Fast path, only a single HEADER frame. _outgoingFrame.PayloadLength = payloadLength; @@ -611,7 +611,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, _outputWriter.Write(_headerEncodingBuffer.AsSpan(0, payloadLength)); return; } - else if (writeResult == HPackHeaderWriter.HeaderWriteResult.MoreHeaders) + else if (writeResult == HeaderWriteResult.MoreHeaders) { _outgoingFrame.PayloadLength = payloadLength; WriteHeaderUnsynchronized(); @@ -621,24 +621,26 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, { // This may happen in case of the TRAILERS after the initial encode operation. // The _maxFrameSize sized _headerEncodingBuffer was too small. - while (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + while (writeResult == HeaderWriteResult.BufferTooSmall) { Debug.Assert(payloadLength == 0, "Payload written even though buffer is too small"); largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); buffer = largeHeaderBuffer.AsSpan(0, _headersEncodingLargeBufferSize); writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); - if (writeResult != HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + if (writeResult != HeaderWriteResult.BufferTooSmall) { - SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: true); + SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: true); } else { + // The size is backed in a local field and not reset for further streams on the connections. + // When a large header is repeatedly returned on a connection, the buffer size won't need to be repeatedly increased. _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } ArrayPool.Shared.Return(largeHeaderBuffer); largeHeaderBuffer = null; } - if (writeResult == HPackHeaderWriter.HeaderWriteResult.Done) + if (writeResult == HeaderWriteResult.Done) { return; } @@ -646,14 +648,17 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, // HEADERS and zero or more CONTINUATIONS sent - all subsequent frames are (unprepared) CONTINUATIONs buffer = _headerEncodingBuffer; - while (writeResult != HPackHeaderWriter.HeaderWriteResult.Done) + while (writeResult != HeaderWriteResult.Done) { writeResult = HPackHeaderWriter.ContinueEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); - if (writeResult == HPackHeaderWriter.HeaderWriteResult.BufferTooSmall) + if (writeResult == HeaderWriteResult.BufferTooSmall) { if (largeHeaderBuffer != null) { ArrayPool.Shared.Return(largeHeaderBuffer); + + // The size is backed in a local field and not reset for further streams on the connections. + // When a large header is repeatedly returned on a connection, the buffer size won't need to be repeatedly increased. _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); @@ -662,7 +667,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, else { // In case of Done or MoreHeaders: write to output. - SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HPackHeaderWriter.HeaderWriteResult.Done, isFramePrepared: false); + SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: false); } } if (largeHeaderBuffer != null) diff --git a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs index c6a9e849f7b4..2edf4628129e 100644 --- a/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs +++ b/src/Servers/Kestrel/Core/test/Http2/Http2HPackEncoderTests.cs @@ -22,7 +22,7 @@ public void BeginEncodeHeaders_Status302_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -45,7 +45,7 @@ public void BeginEncodeHeaders_CacheControlPrivate_NewIndexValue() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(5, length - 5).ToArray(); var hex = BitConverter.ToString(result); @@ -73,7 +73,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // First response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -115,7 +115,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -156,7 +156,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeExceeded_EvictionsToFit() headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -217,7 +217,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // First response enumerator.Initialize((HttpResponseHeaders)headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(302, hpackEncoder, enumerator, buffer, out var length)); var result = buffer.Slice(0, length).ToArray(); var hex = BitConverter.ToString(result); @@ -259,7 +259,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction // Second response enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(307, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -300,7 +300,7 @@ public void BeginEncodeHeadersCustomEncoding_MaxHeaderTableSizeExceeded_Eviction headers.SetCookie = "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1"; enumerator.Initialize(headers); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out length)); result = buffer.Slice(0, length).ToArray(); hex = BitConverter.ToString(result); @@ -358,7 +358,7 @@ public void BeginEncodeHeaders_ExcludedHeaders_NotAddedToTable(string headerName enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(maxHeaderTableSize: Http2PeerSettings.DefaultHeaderTableSize); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out _)); if (neverIndex) { @@ -384,7 +384,7 @@ public void BeginEncodeHeaders_HeaderExceedHeaderTableSize_NoIndexAndNoHeaderEnt enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Empty(GetHeaderEntries(hpackEncoder)); } @@ -473,11 +473,11 @@ public void EncodesHeadersInSinglePayloadWhenSpaceAvailable(KeyValuePair()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(2, length); @@ -591,7 +591,7 @@ public void BeginEncodeHeaders_MaxHeaderTableSizeUpdated_SizeUpdateInHeaders() // Second request enumerator.Initialize(new Dictionary()); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); + Assert.Equal(HeaderWriteResult.Done, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out length)); Assert.Equal(0, length); } @@ -607,7 +607,7 @@ public void WithStatusCode_TooLargeHeader_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(1, length); } @@ -622,7 +622,7 @@ public void NoStatusCodeLargeHeader_ReturnsOversized() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.BufferTooSmall, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(0, length); } @@ -637,7 +637,7 @@ public void WithStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(200, hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(1, length); } @@ -653,7 +653,7 @@ public void NoStatusCode_JustFittingHeaderNoSpace_ReturnsMoreHeaders() enumerator.Initialize(headers); var hpackEncoder = new DynamicHPackEncoder(); - Assert.Equal(HPackHeaderWriter.HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); + Assert.Equal(HeaderWriteResult.MoreHeaders, HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, enumerator, buffer, out var length)); Assert.Equal(26, length); } diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 68fa304d5057..c13dd110d90b 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -#nullable enable using System.Net.Http; using System.Net.Http.HPack; @@ -11,22 +10,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; #endif -// This file is used by Kestrel to write response headers and tests to write request headers. -// To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs. -internal static class HPackHeaderWriter +internal enum HeaderWriteResult : byte { - internal enum HeaderWriteResult : byte - { - // Not all headers written. - MoreHeaders = 0, + // Not all headers written. + MoreHeaders = 0, - // All headers written. - Done = 1, + // All headers written. + Done = 1, - // Oversized header for the given buffer. - BufferTooSmall = 2, - } + // Oversized header for the given buffer. + BufferTooSmall = 2, +} +// This file is used by Kestrel to write response headers and tests to write request headers. +// To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs. +internal static class HPackHeaderWriter +{ /// /// Begin encoding headers in the first HEADERS frame. /// diff --git a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs index 52a168ba696b..4480a06271a4 100644 --- a/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs +++ b/src/Servers/Kestrel/shared/test/PipeWriterHttp2FrameExtensions.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Http2HeadersEnumerator = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2HeadersEnumerator; using HPackHeaderWriter = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HPackHeaderWriter; +using HeaderWriteResult = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HeaderWriteResult; namespace Microsoft.AspNetCore.InternalTesting; @@ -36,7 +37,7 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami var done = HPackHeaderWriter.BeginEncodeHeaders(hpackEncoder, headers, buffer, out var length); frame.PayloadLength = length; - if (done == HPackHeaderWriter.HeaderWriteResult.Done) + if (done == HeaderWriteResult.Done) { frame.HeadersFlags = Http2HeadersFrameFlags.END_HEADERS; } @@ -49,14 +50,14 @@ public static void WriteStartStream(this PipeWriter writer, int streamId, Dynami Http2FrameWriter.WriteHeader(frame, writer); writer.Write(buffer.Slice(0, length)); - while (done != HPackHeaderWriter.HeaderWriteResult.Done) + while (done != HeaderWriteResult.Done) { frame.PrepareContinuation(Http2ContinuationFrameFlags.NONE, streamId); done = HPackHeaderWriter.ContinueEncodeHeaders(hpackEncoder, headers, buffer, out length); frame.PayloadLength = length; - if (done == HPackHeaderWriter.HeaderWriteResult.Done) + if (done == HeaderWriteResult.Done) { frame.ContinuationFlags = Http2ContinuationFrameFlags.END_HEADERS; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 1a5b95034e90..c334ee588dab 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -849,7 +849,7 @@ internal async Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HPackHeaderWriter.HeaderWriteResult.Done; + return done == HeaderWriteResult.Done; } internal Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags flags, IEnumerable> headers) @@ -919,7 +919,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HPackHeaderWriter.HeaderWriteResult.Done; + return done == HeaderWriteResult.Done; } internal async Task SendContinuationAsync(int streamId, Http2ContinuationFrameFlags flags, byte[] payload) @@ -947,7 +947,7 @@ internal async Task SendContinuationAsync(int streamId, Http2ContinuationF Http2FrameWriter.WriteHeader(frame, outputWriter); await SendAsync(buffer.Span.Slice(0, length)); - return done == HPackHeaderWriter.HeaderWriteResult.Done; + return done == HeaderWriteResult.Done; } internal Http2HeadersEnumerator GetHeadersEnumerator(IEnumerable> headers) From a98500b4d3b26a9888316b7677ee239a93ee7dd7 Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 6 Jun 2024 08:03:35 +0200 Subject: [PATCH 24/27] Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs Co-authored-by: Andrew Casey --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 7b2947e54bff..90f174f8784b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -75,6 +75,8 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private byte[] _headerEncodingBuffer; + // Keep track of the high-water mark of _headerEncodingBuffer's size so we don't have to grow + // through intermediate sizes repeatedly. private int _headersEncodingLargeBufferSize = Http2PeerSettings.MinAllowedMaxFrameSize * HeaderBufferSizeMultiplier; private long _unflushedBytes; From 9014f28dce232a15423f4bf21739ef6b3d106b9e Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 6 Jun 2024 08:26:11 +0200 Subject: [PATCH 25/27] - Moving comments around for _headersEncodingLargeBufferSize - Rename splitting method to SplitHeaderAcrossFrames --- .../Core/src/Internal/Http2/Http2FrameWriter.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 90f174f8784b..b799ef02797c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -75,6 +75,7 @@ internal sealed class Http2FrameWriter private int _maxFrameSize = Http2PeerSettings.MinAllowedMaxFrameSize; private byte[] _headerEncodingBuffer; + // Keep track of the high-water mark of _headerEncodingBuffer's size so we don't have to grow // through intermediate sizes repeatedly. private int _headersEncodingLargeBufferSize = Http2PeerSettings.MinAllowedMaxFrameSize * HeaderBufferSizeMultiplier; @@ -574,7 +575,7 @@ private ValueTask WriteDataAndTrailersAsync(Http2Stream stream, in } } - private void SplitHeaderFramesToOutput(int streamId, ReadOnlySpan dataToFrame, bool endOfHeaders, bool isFramePrepared) + private void SplitHeaderAcrossFrames(int streamId, ReadOnlySpan dataToFrame, bool endOfHeaders, bool isFramePrepared) { var shouldPrepareFrame = !isFramePrepared; while (dataToFrame.Length > 0) @@ -631,12 +632,10 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, writeResult = HPackHeaderWriter.RetryBeginEncodeHeaders(_hpackEncoder, _headersEnumerator, buffer, out payloadLength); if (writeResult != HeaderWriteResult.BufferTooSmall) { - SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: true); + SplitHeaderAcrossFrames(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: true); } else { - // The size is backed in a local field and not reset for further streams on the connections. - // When a large header is repeatedly returned on a connection, the buffer size won't need to be repeatedly increased. _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } ArrayPool.Shared.Return(largeHeaderBuffer); @@ -658,9 +657,6 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, if (largeHeaderBuffer != null) { ArrayPool.Shared.Return(largeHeaderBuffer); - - // The size is backed in a local field and not reset for further streams on the connections. - // When a large header is repeatedly returned on a connection, the buffer size won't need to be repeatedly increased. _headersEncodingLargeBufferSize = checked(_headersEncodingLargeBufferSize * HeaderBufferSizeMultiplier); } largeHeaderBuffer = ArrayPool.Shared.Rent(_headersEncodingLargeBufferSize); @@ -669,7 +665,7 @@ private void FinishWritingHeadersUnsynchronized(int streamId, int payloadLength, else { // In case of Done or MoreHeaders: write to output. - SplitHeaderFramesToOutput(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: false); + SplitHeaderAcrossFrames(streamId, buffer[..payloadLength], endOfHeaders: writeResult == HeaderWriteResult.Done, isFramePrepared: false); } } if (largeHeaderBuffer != null) From 957dfc3894873f7e51f77c8f68af943d9cc89fd4 Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 6 Jun 2024 09:09:54 +0200 Subject: [PATCH 26/27] Merging the implementation of BeginEncodeHeaders and RetryBeginEncodeHeaders --- .../Kestrel/shared/HPackHeaderWriter.cs | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index c13dd110d90b..8a764be817bb 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -60,30 +60,19 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE /// /// Begin encoding headers in the first HEADERS frame. /// - public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) - { - length = 0; - - if (!hpackEncoder.EnsureDynamicTableSizeUpdate(buffer, out var sizeUpdateLength)) - { - throw new HPackEncodingException(SR.net_http_hpack_encode_failure); - } - length += sizeUpdateLength; - - if (!headersEnumerator.MoveNext()) - { - return HeaderWriteResult.Done; - } - - var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); - length += headersLength; - return done; - } + public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) => + BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iteratorBeforeFirstElement: true, out length); /// /// Begin encoding headers in the first HEADERS frame without stepping the iterator. /// - public static HeaderWriteResult RetryBeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) + public static HeaderWriteResult RetryBeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) => + BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iteratorBeforeFirstElement: false, out length); + + /// + /// Begin encoding headers in the first HEADERS frame. + /// + private static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool iteratorBeforeFirstElement, out int length) { length = 0; @@ -93,6 +82,11 @@ public static HeaderWriteResult RetryBeginEncodeHeaders(DynamicHPackEncoder hpac } length += sizeUpdateLength; + if (iteratorBeforeFirstElement && !headersEnumerator.MoveNext()) + { + return HeaderWriteResult.Done; + } + var done = EncodeHeadersCore(hpackEncoder, headersEnumerator, buffer.Slice(length), canRequestLargerBuffer: true, out var headersLength); length += headersLength; return done; From 57cc2648de9e42f5dc5fbd8e46963859b3333d4a Mon Sep 17 00:00:00 2001 From: ladeak Date: Thu, 6 Jun 2024 11:46:31 +0200 Subject: [PATCH 27/27] Apply suggestions from code review Co-authored-by: James Newton-King --- src/Servers/Kestrel/shared/HPackHeaderWriter.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs index 8a764be817bb..454346e86c70 100644 --- a/src/Servers/Kestrel/shared/HPackHeaderWriter.cs +++ b/src/Servers/Kestrel/shared/HPackHeaderWriter.cs @@ -61,18 +61,18 @@ public static HeaderWriteResult BeginEncodeHeaders(int statusCode, DynamicHPackE /// Begin encoding headers in the first HEADERS frame. /// public static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) => - BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iteratorBeforeFirstElement: true, out length); + BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iterateBeforeFirstElement: true, out length); /// /// Begin encoding headers in the first HEADERS frame without stepping the iterator. /// public static HeaderWriteResult RetryBeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, out int length) => - BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iteratorBeforeFirstElement: false, out length); + BeginEncodeHeaders(hpackEncoder, headersEnumerator, buffer, iterateBeforeFirstElement: false, out length); /// /// Begin encoding headers in the first HEADERS frame. /// - private static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool iteratorBeforeFirstElement, out int length) + private static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEncoder, Http2HeadersEnumerator headersEnumerator, Span buffer, bool iterateBeforeFirstElement, out int length) { length = 0; @@ -82,7 +82,7 @@ private static HeaderWriteResult BeginEncodeHeaders(DynamicHPackEncoder hpackEnc } length += sizeUpdateLength; - if (iteratorBeforeFirstElement && !headersEnumerator.MoveNext()) + if (iterateBeforeFirstElement && !headersEnumerator.MoveNext()) { return HeaderWriteResult.Done; }