From a4146b706c0a0577f9ce65d711ad64a6847e3e24 Mon Sep 17 00:00:00 2001 From: JLofgren Date: Mon, 16 Apr 2018 16:27:36 -0500 Subject: [PATCH] Do not enforce arbitrary max header list size in HpackEncoder (#7853) Motivation: When connecting to an HTTP/2 server that did not set any value for the SETTINGS_MAX_HEADER_LIST_SIZE in the settings frame, the netty client was imposing an arbitrary maximum header list size of 8kB. There should be no need for the client to enforce such a limit if the server has not specified any limit. This caused an issue for a grpc-java client that needed to send a large header to a server via an Envoy proxy server. The error condition is demonstrated here: https://github.com/JLofgren/demo-grpc-java-bug-4284 Fixes grpc-java issue #4284 - https://github.com/grpc/grpc-java/issues/4284 and netty issue #7825 - https://github.com/netty/netty/issues/7825 Modifications: In HpackEncoder use MAX_HEADER_LIST_SIZE as default maxHeader list size. Result: HpackEncoder will only enforce a max header list size if the server has specified a limit in its settings frame. --- .../handler/codec/http2/HpackEncoder.java | 3 +- .../handler/codec/http2/HpackEncoderTest.java | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) 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 b9a7703042..7719072a49 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.Arrays; 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 @@ final class HpackEncoder { 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)))]; 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 2996b5db6b..b1dce05c38 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 class HpackEncoderTest { 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(); + } + } }