Repository: netty/netty
Issue: HpackEncoder limits header list size to 8K by default
Labels: defect

### Expected behavior
Sending a header list of arbitrary size would succeed, except when the receiver prohibits header list larger than a certain size.

RFC7540 defines SETTINGS_MAX_HEADER_LIST_SIZE's initial value to be unlimited.

### Actual behavior
If the receiver does not specify a maximum header list size in SETTINGS, Netty disallows header list sizes greater than 8K.

The problem seems to be [in HpackEncoder](https://github.com/netty/netty/blob/netty-4.1.22.Final/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java#L89). While defaulting to 8K makes sense for HpackDecoder in order for it to protect itself against the remote, there's no need for such behavior in HpackEncoder. In fact, since SETTINGS_MAX_HEADER_LIST_SIZE is advisory and the content is locally generated, it could ignore the setting completely and only risk wasting bandwidth.

This was reported against gRPC at grpc/grpc-java#4284. CC @JLofgren

### Netty version
4.1.17.Final

I'd be happy to send a PR changing the initialization to use `MAX_HEADER_LIST_SIZE`. @Scottmitch, @normanmaurer, concerns?

Issue comments:
Looks like I can call `defaultHttp2HeadersEncoder.maxHeaderListSize(MAX_HEADER_LIST_SIZE)` to workaround this. If done immediately on creation, it looks like it won't collide with any received SETTINGS requesting a lower value.
I've created a demo of this bug here: https://github.com/JLofgren/demo-grpc-java-bug-4284

Locally I have built netty initializing the max header size in HpackEncoder to MAX_HEADER_LIST_SIZE and shown that this solves the issue. I am working on a PR, but am having some issues dotting all the i's of your PR process.


Pull request review comments:
if you look at the comments for `DEFAULT_HEADER_LIST_SIZE` it describes that although the value is arbitrary it is not un-intentional, and technically not in violation of the spec. Having unlimited header size may leave the server exposed to memory exhaustion before it has the chance to communicate a lower limit. I think I would prefer to have the default value be configurable rather than just making it infinite. What about extracting the value from `AbstractHttp2ConnectionHandlerBuilder#initialSettings`?
@Scottmitch Are you saying there is a possible scenario in the HTTP/2 protocol in which the receiver may be reading headers from the sender *before* the receiver has sent a settings frame to the sender?
I guess I should say: ... *before* the receiver's settings frame has been processed by the sender?
@Scottmitch ping
doh ... I thought this was in the decoder. I took a look at the decoder and looks like we are already using my suggestion of inferring the value from the initial settings. Sorry for the noise  ... lgtm.

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java
index b9a7703042f..7719072a499 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java
@@ -41,7 +41,6 @@
 import java.util.Map;
 
 import static io.netty.handler.codec.http2.HpackUtil.equalsConstantTime;
-import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_LIST_SIZE;
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_TABLE_SIZE;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_LIST_SIZE;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
@@ -86,7 +85,7 @@ public HpackEncoder(boolean ignoreMaxHeaderListSize) {
     public HpackEncoder(boolean ignoreMaxHeaderListSize, int arraySizeHint) {
         this.ignoreMaxHeaderListSize = ignoreMaxHeaderListSize;
         maxHeaderTableSize = DEFAULT_HEADER_TABLE_SIZE;
-        maxHeaderListSize = DEFAULT_HEADER_LIST_SIZE;
+        maxHeaderListSize = MAX_HEADER_LIST_SIZE;
         // Enforce a bound of [2, 128] because hashMask is a byte. The max possible value of hashMask is one less
         // than the length of this array, and we want the mask to be > 0.
         headerFields = new HeaderEntry[findNextPositivePowerOfTwo(max(2, min(arraySizeHint, 128)))];


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java
index 2996b5db6b3..b1dce05c38b 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HpackEncoderTest.java
@@ -56,4 +56,46 @@ public void testSetMaxHeaderTableSizeOverflow() throws Http2Exception {
             buf.release();
         }
     }
+
+    /**
+     * The encoder should not impose an arbitrary limit on the header size if
+     * the server has not specified any limit.
+     * @throws Http2Exception
+     */
+    @Test
+    public void testWillEncode16MBHeaderByDefault() throws Http2Exception {
+        ByteBuf buf = Unpooled.buffer();
+        String bigHeaderName = "x-big-header";
+        int bigHeaderSize = 1024 * 1024 * 16;
+        String bigHeaderVal = new String(new char[bigHeaderSize]).replace('\0', 'X');
+        Http2Headers headersIn = new DefaultHttp2Headers().add(
+                "x-big-header", bigHeaderVal);
+        Http2Headers headersOut = new DefaultHttp2Headers();
+
+        try {
+            hpackEncoder.encodeHeaders(0, buf, headersIn, Http2HeadersEncoder.NEVER_SENSITIVE);
+            hpackDecoder.setMaxHeaderListSize(bigHeaderSize + 1024, bigHeaderSize + 1024);
+            hpackDecoder.decode(0, buf, headersOut, false);
+        } finally {
+            buf.release();
+        }
+        assertEquals(headersOut.get(bigHeaderName).toString(), bigHeaderVal);
+    }
+
+    @Test(expected = Http2Exception.class)
+    public void testSetMaxHeaderListSizeEnforcedAfterSet() throws Http2Exception {
+        ByteBuf buf = Unpooled.buffer();
+        Http2Headers headers = new DefaultHttp2Headers().add(
+                "x-big-header",
+                new String(new char[1024 * 16]).replace('\0', 'X')
+        );
+
+        hpackEncoder.setMaxHeaderListSize(1000);
+
+        try {
+            hpackEncoder.encodeHeaders(0, buf, headers, Http2HeadersEncoder.NEVER_SENSITIVE);
+        } finally {
+            buf.release();
+        }
+    }
 }