From b8e56691cbffa74f07e0db14dc23ce4405e18008 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Fri, 31 Aug 2018 10:36:48 -0700 Subject: [PATCH] Implement MaxRequestLineSize for HTTP/2 #2813 --- .../Internal/Http2/Http2Connection.cs | 2 +- .../Internal/Http2/Http2Frame.Settings.cs | 2 +- .../Internal/Http2/Http2Stream.cs | 8 +++++++ src/Kestrel.Core/KestrelServerLimits.cs | 2 ++ .../Http2/Http2ConnectionTests.cs | 4 ++-- .../Http2/Http2StreamTests.cs | 22 +++++++++++++++++++ 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 3835b0714..5b051ff22 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -953,7 +953,7 @@ public void OnHeader(Span name, Span value) { // https://tools.ietf.org/html/rfc7540#section-6.5.2 // "The value is based on the uncompressed size of header fields, including the length of the name and value in octets plus an overhead of 32 octets for each header field."; - _totalParsedHeaderSize += 32 + name.Length + value.Length; + _totalParsedHeaderSize += HeaderField.RfcOverhead + name.Length + value.Length; if (_totalParsedHeaderSize > _context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize) { throw new Http2ConnectionErrorException(CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http2ErrorCode.PROTOCOL_ERROR); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs index 3039ec1b7..ceddf8089 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 */ public partial class Http2Frame { - private const int SettingSize = 6; // 2 bytes for the id, 4 bytes for the value. + internal const int SettingSize = 6; // 2 bytes for the id, 4 bytes for the value. public Http2SettingsFrameFlags SettingsFlags { diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 82935a343..64e32d560 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -160,6 +160,14 @@ private bool TryValidatePseudoHeaders() return true; } + // Approximate MaxRequestLineSize by totaling the required pseudo header field lengths. + var requestLineLength = _methodText.Length + Scheme.Length + hostText.Length + path.Length; + if (requestLineLength > ServiceContext.ServerOptions.Limits.MaxRequestLineSize) + { + ResetAndAbort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestLineTooLong), Http2ErrorCode.PROTOCOL_ERROR); + return false; + } + var queryIndex = path.IndexOf('?'); QueryString = queryIndex == -1 ? string.Empty : path.Substring(queryIndex); diff --git a/src/Kestrel.Core/KestrelServerLimits.cs b/src/Kestrel.Core/KestrelServerLimits.cs index e7ddc479d..10db0fe66 100644 --- a/src/Kestrel.Core/KestrelServerLimits.cs +++ b/src/Kestrel.Core/KestrelServerLimits.cs @@ -88,6 +88,8 @@ public long? MaxRequestBufferSize /// Defaults to 8,192 bytes (8 KB). /// /// + /// For HTTP/2 this measures the total size of the required pseudo headers + /// :method, :scheme, :authority, and :path. /// public int MaxRequestLineSize { diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 9f8ebd58a..390085f9f 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -1590,7 +1590,7 @@ public Task HEADERS_Received_HeaderBlockOverLimit_ConnectionError() [Fact] public Task HEADERS_Received_TooManyHeaders_ConnectionError() { - // > MaxRequestHeaderCount (100 + // > MaxRequestHeaderCount (100) var headers = new List>(); headers.AddRange(new [] { @@ -2064,7 +2064,7 @@ public async Task SETTINGS_KestrelDefaults_Sent() await SendSettingsAsync(); var frame = await ExpectAsync(Http2FrameType.SETTINGS, - withLength: 6 * 2, + withLength: Http2Frame.SettingSize * 2, withFlags: 0, withStreamId: 0); diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 5085856eb..7df7750f8 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -569,6 +569,28 @@ await WaitForStreamErrorAsync(expectedStreamId: 1, Http2ErrorCode.PROTOCOL_ERROR await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } + [Fact] + public async Task HEADERS_Received_MaxRequestLineSize_Reset() + { + // Default 8kb limit + // This test has to work around the HPack parser limit for incoming field sizes over 4kb. That's going to be a problem for people with long urls. + // https://github.com/aspnet/KestrelHttpServer/issues/2872 + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET" + new string('a', 1024 * 3)), + new KeyValuePair(HeaderNames.Path, "/Hello/How/Are/You/" + new string('a', 1024 * 3)), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.Authority, "localhost" + new string('a', 1024 * 3) + ":80"), + }; + await InitializeConnectionAsync(_noopApplication); + + await StartStreamAsync(1, headers, endStream: true); + + await WaitForStreamErrorAsync(expectedStreamId: 1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.BadRequest_RequestLineTooLong); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task ContentLength_Received_SingleDataFrame_Verified() {