Repository: netty/netty (Java)

Issue:
HPACK dynamic table size update must happen at the beginning of the header block
### Expected behavior

HPACK decoder accepts dynamic table size update at the beginning of the block.

Cf RFC 7531, section 4.2

### Actual behavior

HPACK decoder tolerates dynamic table size update anywhere in the block

`io.netty.handler.codec.http2.HpackDecoder` can be modified to only allow setDynamicTableSize to be called only if that is the first of the block processing and fail otherwise with a connection error like it does already.

### Steps to reproduce

Send an HTTP/2 header frame with a dynamic table size update anywhere, e.g at the end of the stream

### Minimal yet complete reproducer code (or URL to code)

N/A

### Netty version

4.1.84.Final

### JVM version (e.g. `java -version`)

N/A

### OS version (e.g. `uname -a`)

N/A


Hints:
I can contribute a fix for this.

Patch:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDecoder.java
@@ -79,15 +79,14 @@ final class HpackDecoder {
             Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - max dynamic table size change required",
                     Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "decode(..)");
     private static final byte READ_HEADER_REPRESENTATION = 0;
-    private static final byte READ_MAX_DYNAMIC_TABLE_SIZE = 1;
-    private static final byte READ_INDEXED_HEADER = 2;
-    private static final byte READ_INDEXED_HEADER_NAME = 3;
-    private static final byte READ_LITERAL_HEADER_NAME_LENGTH_PREFIX = 4;
-    private static final byte READ_LITERAL_HEADER_NAME_LENGTH = 5;
-    private static final byte READ_LITERAL_HEADER_NAME = 6;
-    private static final byte READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX = 7;
-    private static final byte READ_LITERAL_HEADER_VALUE_LENGTH = 8;
-    private static final byte READ_LITERAL_HEADER_VALUE = 9;
+    private static final byte READ_INDEXED_HEADER = 1;
+    private static final byte READ_INDEXED_HEADER_NAME = 2;
+    private static final byte READ_LITERAL_HEADER_NAME_LENGTH_PREFIX = 3;
+    private static final byte READ_LITERAL_HEADER_NAME_LENGTH = 4;
+    private static final byte READ_LITERAL_HEADER_NAME = 5;
+    private static final byte READ_LITERAL_HEADER_VALUE_LENGTH_PREFIX = 6;
+    private static final byte READ_LITERAL_HEADER_VALUE_LENGTH = 7;
+    private static final byte READ_LITERAL_HEADER_VALUE = 8;
 
     private final HpackHuffmanDecoder huffmanDecoder = new HpackHuffmanDecoder();
     private final HpackDynamicTable hpackDynamicTable;
@@ -127,6 +126,9 @@ final class HpackDecoder {
     void decode(int streamId, ByteBuf in, Http2Headers headers, boolean validateHeaders) throws Http2Exception {
         Http2HeadersSink sink = new Http2HeadersSink(
                 streamId, headers, maxHeaderListSize, validateHeaders);
+        // Check for dynamic table size updates, which must occur at the beginning:
+        // https://www.rfc-editor.org/rfc/rfc7541.html#section-4.2
+        decodeDynamicTableSizeUpdates(in);
         decode(in, sink);
 
         // Now that we've read all of our headers we can perform the validation steps. We must
@@ -134,6 +136,19 @@ void decode(int streamId, ByteBuf in, Http2Headers headers, boolean validateHead
         sink.finish();
     }
 
+    private void decodeDynamicTableSizeUpdates(ByteBuf in) throws Http2Exception {
+        byte b;
+        while (in.isReadable() && ((b = in.getByte(in.readerIndex())) & 0x20) == 0x20 && ((b & 0xC0) == 0x00)) {
+            in.readByte();
+            int index = b & 0x1F;
+            if (index == 0x1F) {
+                setDynamicTableSize(decodeULE128(in, (long) index));
+            } else {
+                setDynamicTableSize(index);
+            }
+        }
+    }
+
     private void decode(ByteBuf in, Http2HeadersSink sink) throws Http2Exception {
         int index = 0;
         int nameLength = 0;
@@ -184,13 +199,9 @@ private void decode(ByteBuf in, Http2HeadersSink sink) throws Http2Exception {
                         }
                     } else if ((b & 0x20) == 0x20) {
                         // Dynamic Table Size Update
-                        index = b & 0x1F;
-                        if (index == 0x1F) {
-                            state = READ_MAX_DYNAMIC_TABLE_SIZE;
-                        } else {
-                            setDynamicTableSize(index);
-                            state = READ_HEADER_REPRESENTATION;
-                        }
+                        // See https://www.rfc-editor.org/rfc/rfc7541.html#section-4.2
+                        throw connectionError(COMPRESSION_ERROR, "Dynamic table size update must happen " +
+                            "at the beginning of the header block");
                     } else {
                         // Literal Header Field without Indexing / never Indexed
                         indexType = (b & 0x10) == 0x10 ? IndexType.NEVER : IndexType.NONE;
@@ -211,11 +222,6 @@ private void decode(ByteBuf in, Http2HeadersSink sink) throws Http2Exception {
                     }
                     break;
 
-                case READ_MAX_DYNAMIC_TABLE_SIZE:
-                    setDynamicTableSize(decodeULE128(in, (long) index));
-                    state = READ_HEADER_REPRESENTATION;
-                    break;
-
                 case READ_INDEXED_HEADER:
                     HpackHeaderField indexedHeader = getIndexedHeader(decodeULE128(in, index));
                     sink.appendToHeaderList(


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackDecoderTest.java
@@ -403,6 +403,22 @@ public void execute() throws Throwable {
         });
     }
 
+    @Test
+    public void testDynamicTableSizeUpdateAfterTheBeginingOfTheBlock() throws Http2Exception {
+        assertThrows(Http2Exception.class, new Executable() {
+            @Override
+            public void execute() throws Throwable {
+                decode("8120");
+            }
+        });
+        assertThrows(Http2Exception.class, new Executable() {
+            @Override
+            public void execute() throws Throwable {
+                decode("813FE11F");
+            }
+        });
+    }
+
     @Test
     public void testLiteralWithIncrementalIndexingWithEmptyName() throws Http2Exception {
         decode("400005" + hex("value"));