Enable hpack/4.2 - Maximum Table Size test

- Ensure dynamic tables size updates occur at the beginning of the header block

src/Kestrel.Core/CoreStrings.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
+  <!--
-    Microsoft ResX Schema 
+    Microsoft ResX Schema
-    
+
     Version 2.0
-    
+
-    The primary goals of this format is to allow a simple XML format 
+    The primary goals of this format is to allow a simple XML format
-    that is mostly human readable. The generation and parsing of the 
+    that is mostly human readable. The generation and parsing of the
-    various data types are done through the TypeConverter classes 
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
+
-    There are any number of "resheader" rows that contain simple 
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
+
-    Each data row contains a name, and value. The row also contains a 
+    Each data row contains a name, and value. The row also contains a
-    type or mimetype. Type corresponds to a .NET class that support 
+    type or mimetype. Type corresponds to a .NET class that support
-    text/value conversion through the TypeConverter architecture. 
+    text/value conversion through the TypeConverter architecture.
-    Classes that don't support this are serialized and stored with the 
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
+
-    The mimetype is used for serialized objects, and tells the 
+    The mimetype is used for serialized objects, and tells the
-    ResXResourceReader how to depersist the object. This is currently not 
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
+
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    Note - application/x-microsoft.net.object.binary.base64 is the format
-    that the ResXResourceWriter will generate, however the reader can 
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -578,4 +578,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
   <data name="ArgumentOutOfRange" xml:space="preserve">
     <value>A value between {min} and {max} is required.</value>
   </data>
+  <data name="HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock" xml:space="preserve">
+    <value>Dynamic tables size update did not occur at the beginning of the first header block.</value>
+  </data>
 </root>

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
+                            SetDynamicHeaderTableSize(_integerDecoder.Value);
-                            _dynamicTable.Resize(_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)
+                        SetDynamicHeaderTableSize(_integerDecoder.Value);
-                        {
-                            throw new HPackDecodingException(
-                                CoreStrings.FormatHPackErrorDynamicTableSizeUpdateTooLarge(_integerDecoder.Value, _maxDynamicTableSize));
-                        }
-
-                        _dynamicTable.Resize(_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);
+        }
     }
 }

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);
 
+        /// <summary>
+        /// Dynamic tables size update did not occur at the beginning of the first header block.
+        /// </summary>
+        internal static string HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock
+        {
+            get => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock");
+        }
+
+        /// <summary>
+        /// Dynamic tables size update did not occur at the beginning of the first header block.
+        /// </summary>
+        internal static string FormatHPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock()
+            => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock");
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

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<HPackDecodingException>(() => _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<HPackDecodingException>(() => _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()
         {

test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs

@@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
             get
             {
                 var dataset = new TheoryData<H2SpecTestCase>();
-                var toSkip = new[] { "hpack/4.2/1", "http2/5.1/8" };
+                var toSkip = new[] { "http2/5.1/8" };
 
                 foreach (var testcase in H2SpecCommands.EnumerateTestCases())
                 {