Implement MaxRequestLineSize for HTTP/2 #2813

aspnet · Sep 6, 2018 · b8e5669 · b8e5669
1 parent 384a518
commit b8e5669
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 4 deletions.
diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs
@@ -953,7 +953,7 @@ public void OnHeader(Span<byte> name, Span<byte> 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
@@ -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
@@ -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
@@ -88,6 +88,8 @@ public long? MaxRequestBufferSize
         /// Defaults to 8,192 bytes (8 KB).
         /// </summary>
         /// <remarks>
+        /// For HTTP/2 this measures the total size of the required pseudo headers
+        /// :method, :scheme, :authority, and :path.
         /// </remarks>
         public int MaxRequestLineSize
         {

diff --git 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<KeyValuePair<string, string>>();
             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
@@ -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<string, string>(HeaderNames.Method, "GET" + new string('a', 1024 * 3)),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/Hello/How/Are/You/" + new string('a', 1024 * 3)),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+                new KeyValuePair<string, string>(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()
         {