[netty#5762] HTTP/2: SETTINGS_HEADER_TABLE_SIZE should be an unsigned…

… int Motivation: he HTTP/2 spec demands that the max value for SETTINGS_HEADER_TABLE_SIZE should be an unsigned 32-bit integer. Modifications: Change the limit to unsigned 32-bit integer and add tests. Result: Complient to rfc.
Chandler0591 · Sep 9, 2016 · d2389a9 · d2389a9
1 parent 67d3a78
commit d2389a9
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 39 deletions.
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoder.java
@@ -23,6 +23,8 @@
 
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_HEADER_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.Http2Error.COMPRESSION_ERROR;
 import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
 import static io.netty.handler.codec.http2.Http2Exception.connectionError;
@@ -99,9 +101,10 @@ public Http2Headers decodeHeaders(ByteBuf headerBlock) throws Http2Exception {
      */
     private final class Http2HeaderTableDecoder extends DefaultHttp2HeaderTableListSize implements Http2HeaderTable {
         @Override
-        public void maxHeaderTableSize(int max) throws Http2Exception {
-            if (max < 0) {
-                throw connectionError(PROTOCOL_ERROR, "Header Table Size must be non-negative but was %d", max);
+        public void maxHeaderTableSize(long max) throws Http2Exception {
+            if (max < MIN_HEADER_TABLE_SIZE || max > MAX_HEADER_TABLE_SIZE) {
+                throw connectionError(PROTOCOL_ERROR, "Header Table Size must be >= %d and <= %d but was %d",
+                        MIN_HEADER_TABLE_SIZE, MAX_HEADER_TABLE_SIZE, max);
             }
             try {
                 decoder.setMaxHeaderTableSize(max);
@@ -111,7 +114,7 @@ public void maxHeaderTableSize(int max) throws Http2Exception {
         }
 
         @Override
-        public int maxHeaderTableSize() {
+        public long maxHeaderTableSize() {
             return decoder.getMaxHeaderTableSize();
         }
     }

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2HeadersEncoder.java
@@ -23,6 +23,8 @@
 import java.util.Map.Entry;
 
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_TABLE_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.Http2Error.COMPRESSION_ERROR;
 import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
 import static io.netty.handler.codec.http2.Http2Exception.connectionError;
@@ -89,9 +91,10 @@ private void encodeHeader(ByteBuf out, CharSequence key, CharSequence value) {
      */
     private final class Http2HeaderTableEncoder extends DefaultHttp2HeaderTableListSize implements Http2HeaderTable {
         @Override
-        public void maxHeaderTableSize(int max) throws Http2Exception {
-            if (max < 0) {
-                throw connectionError(PROTOCOL_ERROR, "Header Table Size must be non-negative but was %d", max);
+        public void maxHeaderTableSize(long max) throws Http2Exception {
+            if (max < MIN_HEADER_TABLE_SIZE || max > MAX_HEADER_TABLE_SIZE) {
+                throw connectionError(PROTOCOL_ERROR, "Header Table Size must be >= %d and <= %d but was %d",
+                        MIN_HEADER_TABLE_SIZE, MAX_HEADER_TABLE_SIZE, max);
             }
             try {
                 // No headers should be emitted. If they are, we throw.
@@ -102,7 +105,7 @@ public void maxHeaderTableSize(int max) throws Http2Exception {
         }
 
         @Override
-        public int maxHeaderTableSize() {
+        public long maxHeaderTableSize() {
             return encoder.getMaxHeaderTableSize();
         }
     }

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java
@@ -87,7 +87,7 @@ public final class Http2CodecUtil {
     public static final char SETTINGS_MAX_HEADER_LIST_SIZE = 6;
     public static final int NUM_STANDARD_SETTINGS = 6;
 
-    public static final int MAX_HEADER_TABLE_SIZE = Integer.MAX_VALUE; // Size limited by HPACK library
+    public static final long MAX_HEADER_TABLE_SIZE = MAX_UNSIGNED_INT;
     public static final long MAX_CONCURRENT_STREAMS = MAX_UNSIGNED_INT;
     public static final int MAX_INITIAL_WINDOW_SIZE = Integer.MAX_VALUE;
     public static final int MAX_FRAME_SIZE_LOWER_BOUND = 0x4000;

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeaderTable.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeaderTable.java
@@ -24,12 +24,12 @@ public interface Http2HeaderTable {
     /**
      * Sets the maximum size of the HPACK header table used for decoding HTTP/2 headers.
      */
-    void maxHeaderTableSize(int max) throws Http2Exception;
+    void maxHeaderTableSize(long max) throws Http2Exception;
 
     /**
      * Gets the maximum size of the HPACK header table used for decoding HTTP/2 headers.
      */
-    int maxHeaderTableSize();
+    long maxHeaderTableSize();
 
     /**
      * Sets the maximum allowed header elements.

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java
@@ -87,7 +87,7 @@ public Long headerTableSize() {
      *
      * @throws IllegalArgumentException if verification of the setting fails.
      */
-    public Http2Settings headerTableSize(int value) {
+    public Http2Settings headerTableSize(long value) {
         put(SETTINGS_HEADER_TABLE_SIZE, Long.valueOf(value));
         return this;
     }

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/Decoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/Decoder.java
@@ -74,7 +74,7 @@ public final class Decoder {
     private final DynamicTable dynamicTable;
     private final HuffmanDecoder huffmanDecoder;
     private final int maxHeadersLength;
-    private int maxDynamicTableSize;
+    private long maxDynamicTableSize;
     private int encoderMaxDynamicTableSize;
     private boolean maxDynamicTableSizeChangeRequired;
 
@@ -305,7 +305,7 @@ public void decode(ByteBuf in, Http2Headers headers) throws Http2Exception {
      * Set the maximum table size. If this is below the maximum size of the dynamic table used by
      * the encoder, the beginning of the next header block MUST signal this change.
      */
-    public void setMaxHeaderTableSize(int maxHeaderTableSize) {
+    public void setMaxHeaderTableSize(long maxHeaderTableSize) {
         maxDynamicTableSize = maxHeaderTableSize;
         if (maxDynamicTableSize < encoderMaxDynamicTableSize) {
             // decoder requires less space than encoder
@@ -319,7 +319,7 @@ public void setMaxHeaderTableSize(int maxHeaderTableSize) {
      * Return the maximum table size. This is the maximum size allowed by both the encoder and the
      * decoder.
      */
-    public int getMaxHeaderTableSize() {
+    public long getMaxHeaderTableSize() {
         return dynamicTable.capacity();
     }
 
@@ -333,7 +333,7 @@ int length() {
     /**
      * Return the size of the dynamic table. Exposed for testing.
      */
-    int size() {
+    long size() {
         return dynamicTable.size();
     }
 

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/DynamicTable.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/DynamicTable.java
@@ -31,6 +31,8 @@
  */
 package io.netty.handler.codec.http2.internal.hpack;
 
+import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.internal.hpack.HeaderField.HEADER_ENTRY_OVERHEAD;
 
 final class DynamicTable {
@@ -40,7 +42,7 @@ final class DynamicTable {
     int head;
     int tail;
     private int size;
-    private int capacity = -1; // ensure setCapacity creates the array
+    private long capacity = -1; // ensure setCapacity creates the array
 
     /**
      * Creates a new dynamic table with the specified initial capacity.
@@ -65,14 +67,14 @@ public int length() {
     /**
      * Return the current size of the dynamic table. This is the sum of the size of the entries.
      */
-    public int size() {
+    public long size() {
         return size;
     }
 
     /**
      * Return the maximum allowable size of the dynamic table.
      */
-    public int capacity() {
+    public long capacity() {
         return capacity;
     }
 
@@ -149,11 +151,10 @@ public void clear() {
      * Set the maximum size of the dynamic table. Entries are evicted from the dynamic table until
      * the size of the table is less than or equal to the maximum size.
      */
-    public void setCapacity(int capacity) {
-        if (capacity < 0) {
-            throw new IllegalArgumentException("Illegal Capacity: " + capacity);
+    public void setCapacity(long capacity) {
+        if (capacity < MIN_HEADER_TABLE_SIZE || capacity > MAX_HEADER_TABLE_SIZE) {
+            throw new IllegalArgumentException("capacity is invalid: " + capacity);
         }
-
         // initially capacity will be -1 so init won't return here
         if (this.capacity == capacity) {
             return;
@@ -169,7 +170,7 @@ public void setCapacity(int capacity) {
             }
         }
 
-        int maxEntries = capacity / HEADER_ENTRY_OVERHEAD;
+        int maxEntries = (int) (capacity / HEADER_ENTRY_OVERHEAD);
         if (capacity % HEADER_ENTRY_OVERHEAD != 0) {
             maxEntries++;
         }
@@ -192,8 +193,8 @@ public void setCapacity(int capacity) {
             }
         }
 
-        this.tail = 0;
-        this.head = tail + len;
-        this.headerFields = tmp;
+        tail = 0;
+        head = tail + len;
+        headerFields = tmp;
     }
 }
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/Encoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/Encoder.java
@@ -37,6 +37,8 @@
 
 import java.util.Arrays;
 
+import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
+import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.internal.hpack.HpackUtil.IndexType.INCREMENTAL;
 import static io.netty.handler.codec.http2.internal.hpack.HpackUtil.IndexType.NEVER;
 import static io.netty.handler.codec.http2.internal.hpack.HpackUtil.IndexType.NONE;
@@ -52,22 +54,22 @@ public final class Encoder {
             AsciiString.EMPTY_STRING, Integer.MAX_VALUE, null);
     private final HuffmanEncoder huffmanEncoder = new HuffmanEncoder();
     private final byte hashMask;
-    private int size;
-    private int capacity;
+    private long size;
+    private long capacity;
 
     /**
      * Creates a new encoder.
      */
-    public Encoder(int maxHeaderTableSize) {
+    public Encoder(long maxHeaderTableSize) {
         this(maxHeaderTableSize, 16);
     }
 
     /**
      * Creates a new encoder.
      */
-    public Encoder(int maxHeaderTableSize, int arraySizeHint) {
-        if (maxHeaderTableSize < 0) {
-            throw new IllegalArgumentException("Illegal Capacity: " + maxHeaderTableSize);
+    public Encoder(long maxHeaderTableSize, int arraySizeHint) {
+        if (maxHeaderTableSize < MIN_HEADER_TABLE_SIZE || maxHeaderTableSize > MAX_HEADER_TABLE_SIZE) {
+            throw new IllegalArgumentException("maxHeaderTableSize is invalid: " + maxHeaderTableSize);
         }
         capacity = maxHeaderTableSize;
         // Enforce a bound of [2, 128] because hashMask is a byte. The max possible value of hashMask is one less
@@ -133,22 +135,23 @@ public void encodeHeader(ByteBuf out, CharSequence name, CharSequence value, boo
     /**
      * Set the maximum table size.
      */
-    public void setMaxHeaderTableSize(ByteBuf out, int maxHeaderTableSize) {
-        if (maxHeaderTableSize < 0) {
-            throw new IllegalArgumentException("Illegal Capacity: " + maxHeaderTableSize);
+    public void setMaxHeaderTableSize(ByteBuf out, long maxHeaderTableSize) {
+        if (maxHeaderTableSize < MIN_HEADER_TABLE_SIZE || maxHeaderTableSize > MAX_HEADER_TABLE_SIZE) {
+            throw new IllegalArgumentException("maxHeaderTableSize is invalid: " + maxHeaderTableSize);
         }
         if (capacity == maxHeaderTableSize) {
             return;
         }
         capacity = maxHeaderTableSize;
         ensureCapacity(0);
-        encodeInteger(out, 0x20, 5, maxHeaderTableSize);
+        // Casting to integer is safe as we verified the maxHeaderTableSize is a valid unsigned int.
+        encodeInteger(out, 0x20, 5, (int) maxHeaderTableSize);
     }
 
     /**
      * Return the maximum table size.
      */
-    public int getMaxHeaderTableSize() {
+    public long getMaxHeaderTableSize() {
         return capacity;
     }
 
@@ -252,7 +255,7 @@ int length() {
     /**
      * Return the size of the dynamic table. Exposed for testing.
      */
-    int size() {
+    long size() {
         return size;
     }
 

diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java
@@ -20,6 +20,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import io.netty.handler.codec.http.HttpUtil;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -86,4 +87,21 @@ public void boundarySettingsShouldBeSet() {
         settings.maxHeaderListSize((int) settingsValueUpperBound);
         assertEquals(Integer.MAX_VALUE, (long) settings.maxHeaderListSize());
     }
+
+    @Test
+    public void headerTableSizeUnsignedInt() {
+        final long value = 1L << 31;
+        settings.put(Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE, (Long) value);
+        assertEquals(value, (long) settings.get(Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void headerTableSizeBoundCheck() {
+        settings.put(Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE, (Long) Long.MAX_VALUE);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void headerTableSizeBoundCheck2() {
+        settings.put(Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE, Long.valueOf(-1L));
+    }
 }