From 991cfa8bd8ed940be32ffeda39f8beeb992c9c02 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Sat, 1 Sep 2018 20:20:52 -0700 Subject: [PATCH] Remove limits on SETTINGS_HEADER_TABLE_SIZE #2874 --- src/Kestrel.Core/Http2Limits.cs | 8 ++++---- .../Internal/Http2/Http2PeerSettings.cs | 6 ------ .../Http2/Http2ConnectionTests.cs | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/Kestrel.Core/Http2Limits.cs b/src/Kestrel.Core/Http2Limits.cs index f4b05c4abb..bf30a0c414 100644 --- a/src/Kestrel.Core/Http2Limits.cs +++ b/src/Kestrel.Core/Http2Limits.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; namespace Microsoft.AspNetCore.Server.Kestrel.Core { @@ -11,11 +12,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core public class Http2Limits { private int _maxStreamsPerConnection = 100; - private int _headerTableSize = MaxAllowedHeaderTableSize; + private int _headerTableSize = (int)Http2PeerSettings.DefaultHeaderTableSize; private int _maxFrameSize = MinAllowedMaxFrameSize; // These are limits defined by the RFC https://tools.ietf.org/html/rfc7540#section-4.2 - public const int MaxAllowedHeaderTableSize = 4096; public const int MinAllowedMaxFrameSize = 16 * 1024; public const int MaxAllowedMaxFrameSize = 16 * 1024 * 1024 - 1; @@ -50,9 +50,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core get => _headerTableSize; set { - if (value <= 0 || value > MaxAllowedHeaderTableSize) + if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.FormatArgumentOutOfRange(0, MaxAllowedHeaderTableSize)); + throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.GreaterThanZeroRequired); } _headerTableSize = value; diff --git a/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs b/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs index e821daac13..f11fed9ca7 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs @@ -38,12 +38,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 switch (setting.Parameter) { case Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE: - if (value > Http2Limits.MaxAllowedHeaderTableSize) - { - throw new Http2SettingsParameterOutOfRangeException(Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE, - lowerBound: 0, - upperBound: Http2Limits.MaxAllowedHeaderTableSize); - } HeaderTableSize = value; break; case Http2SettingsParameter.SETTINGS_ENABLE_PUSH: diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index f876a1f01b..0d599b75ca 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -2100,7 +2100,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, 16 * 1024 - 1, Http2ErrorCode.PROTOCOL_ERROR)] [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, 16 * 1024 * 1024, Http2ErrorCode.PROTOCOL_ERROR)] [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, uint.MaxValue, Http2ErrorCode.PROTOCOL_ERROR)] - [InlineData(Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE, 4097, Http2ErrorCode.PROTOCOL_ERROR)] public async Task SETTINGS_Received_InvalidParameterValue_ConnectionError(Http2SettingsParameter parameter, uint value, Http2ErrorCode expectedErrorCode) { await InitializeConnectionAsync(_noopApplication); @@ -2242,6 +2241,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } + [Fact] + public async Task SETTINGS_Received_ChangesHeaderTableSize() + { + await InitializeConnectionAsync(_noopApplication); + + // Update client settings + _clientSettings.HeaderTableSize = 65536; // Chrome's default, larger than the 4kb spec default + await SendSettingsAsync(); + + // ACK + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 0, + withFlags: (byte)Http2SettingsFrameFlags.ACK, + withStreamId: 0); + + await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task PUSH_PROMISE_Received_ConnectionError() {