diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index c40ba8d917..76953b051c 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -1,17 +1,17 @@  - @@ -578,4 +578,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l A value between {min} and {max} is required. + + Dynamic tables size update did not occur at the beginning of the first header block. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs index ac18c1962d..51d6068ae1 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs @@ -94,6 +94,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack private int _headerValueLength; private bool _index; private bool _huffman; + private bool _headersObserved; public HPackDecoder(int maxDynamicTableSize) : this(maxDynamicTableSize, new DynamicTable(maxDynamicTableSize)) @@ -115,19 +116,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack OnByte(data[i], handler); } - if (endHeaders && _state != State.Ready) + if (endHeaders) { - throw new HPackDecodingException(CoreStrings.HPackErrorIncompleteHeaderBlock); + if (_state != State.Ready) + { + throw new HPackDecodingException(CoreStrings.HPackErrorIncompleteHeaderBlock); + } + + _headersObserved = false; } } - public void OnByte(byte b, IHttpHeadersHandler handler) + private void OnByte(byte b, IHttpHeadersHandler handler) { switch (_state) { case State.Ready: if ((b & IndexedHeaderFieldMask) == IndexedHeaderFieldRepresentation) { + _headersObserved = true; var val = b & ~IndexedHeaderFieldMask; if (_integerDecoder.BeginDecode((byte)val, IndexedHeaderFieldPrefix)) @@ -141,6 +148,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldWithIncrementalIndexingMask) == LiteralHeaderFieldWithIncrementalIndexingRepresentation) { + _headersObserved = true; _index = true; var val = b & ~LiteralHeaderFieldWithIncrementalIndexingMask; @@ -159,6 +167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldWithoutIndexingMask) == LiteralHeaderFieldWithoutIndexingRepresentation) { + _headersObserved = true; _index = false; var val = b & ~LiteralHeaderFieldWithoutIndexingMask; @@ -177,6 +186,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldNeverIndexedMask) == LiteralHeaderFieldNeverIndexedRepresentation) { + _headersObserved = true; _index = false; var val = b & ~LiteralHeaderFieldNeverIndexedMask; @@ -195,10 +205,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & DynamicTableSizeUpdateMask) == DynamicTableSizeUpdateRepresentation) { + // https://tools.ietf.org/html/rfc7541#section-4.2 + // This dynamic table size + // update MUST occur at the beginning of the first header block + // following the change to the dynamic table size. + if (_headersObserved) + { + throw new HPackDecodingException(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock); + } + if (_integerDecoder.BeginDecode((byte)(b & ~DynamicTableSizeUpdateMask), DynamicTableSizeUpdatePrefix)) { - // TODO: validate that it's less than what's defined via SETTINGS - _dynamicTable.Resize(_integerDecoder.Value); + SetDynamicHeaderTableSize(_integerDecoder.Value); } else { @@ -295,13 +313,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack case State.DynamicTableSizeUpdate: if (_integerDecoder.Decode(b)) { - if (_integerDecoder.Value > _maxDynamicTableSize) - { - throw new HPackDecodingException( - CoreStrings.FormatHPackErrorDynamicTableSizeUpdateTooLarge(_integerDecoder.Value, _maxDynamicTableSize)); - } - - _dynamicTable.Resize(_integerDecoder.Value); + SetDynamicHeaderTableSize(_integerDecoder.Value); _state = State.Ready; } @@ -402,5 +414,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack throw new HPackDecodingException(CoreStrings.FormatHPackErrorIndexOutOfRange(index), ex); } } + + private void SetDynamicHeaderTableSize(int size) + { + if (size > _maxDynamicTableSize) + { + throw new HPackDecodingException( + CoreStrings.FormatHPackErrorDynamicTableSizeUpdateTooLarge(size, _maxDynamicTableSize)); + } + + _dynamicTable.Resize(size); + } } } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 06945a5160..c9710cd26e 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2156,6 +2156,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatArgumentOutOfRange(object min, object max) => string.Format(CultureInfo.CurrentCulture, GetString("ArgumentOutOfRange", "min", "max"), min, max); + /// + /// Dynamic tables size update did not occur at the beginning of the first header block. + /// + internal static string HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock + { + get => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock"); + } + + /// + /// Dynamic tables size update did not occur at the beginning of the first header block. + /// + internal static string FormatHPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock() + => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Kestrel.Core.Tests/HPackDecoderTests.cs b/test/Kestrel.Core.Tests/HPackDecoderTests.cs index 28878c6864..70d1f3e085 100644 --- a/test/Kestrel.Core.Tests/HPackDecoderTests.cs +++ b/test/Kestrel.Core.Tests/HPackDecoderTests.cs @@ -375,6 +375,47 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Empty(_decodedHeaders); } + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_Error() + { + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + var exception = Assert.Throws(() => _decoder.Decode(_indexedHeaderStatic.Concat(new byte[] { 0x3e }).ToArray(), endHeaders: true, handler: this)); + Assert.Equal(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock, exception.Message); + } + + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_SubsequentDecodeCall_Error() + { + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + _decoder.Decode(_indexedHeaderStatic, endHeaders: false, handler: this); + Assert.Equal("GET", _decodedHeaders[HeaderNames.Method]); + + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + var exception = Assert.Throws(() => _decoder.Decode(new byte[] { 0x3e }, endHeaders: true, handler: this)); + Assert.Equal(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock, exception.Message); + } + + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_ResetAfterEndHeaders_Succeeds() + { + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + _decoder.Decode(_indexedHeaderStatic, endHeaders: true, handler: this); + Assert.Equal("GET", _decodedHeaders[HeaderNames.Method]); + + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + _decoder.Decode(new byte[] { 0x3e }, endHeaders: true, handler: this); + + Assert.Equal(30, _dynamicTable.MaxSize); + } + [Fact] public void DecodesDynamicTableSizeUpdate_GreaterThanLimit_Error() { diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs index baca985366..138b7fbc48 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 get { var dataset = new TheoryData(); - var toSkip = new[] { "hpack/4.2/1", "http2/5.1/8" }; + var toSkip = new[] { "http2/5.1/8" }; foreach (var testcase in H2SpecCommands.EnumerateTestCases()) {