From 907726988dec6841933afa18d70f07830097b56a Mon Sep 17 00:00:00 2001 From: Christopher Exell Date: Fri, 20 Jan 2017 15:14:42 -0800 Subject: [PATCH] Update hpack Decoder CTOR to allow for overflow in maxHeaderList size, as we do when we apply our ack'ed settings This prevents us from having the first request, that hasn't ack'ed the setting causing a GOAWAY when we'd would be under the maxHeaderListSizeGoAway that would have been set after the settings ack. --- .../http2/DefaultHttp2ConnectionDecoder.java | 3 +-- .../netty/handler/codec/http2/Http2CodecUtil.java | 12 ++++++++++++ .../codec/http2/internal/hpack/Decoder.java | 5 ++++- .../http2/DefaultHttp2HeadersDecoderTest.java | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java index c47b83a9fd..7cd935269b 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java @@ -149,8 +149,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { * @return the threshold in bytes which should trigger a {@code GO_AWAY} if a set of headers exceeds this amount. */ protected long calculateMaxHeaderListSizeGoAway(long maxHeaderListSize) { - // This is equivalent to `maxHeaderListSize * 1.25` but we avoid floating point multiplication. - return maxHeaderListSize + (maxHeaderListSize >>> 2); + return Http2CodecUtil.calculateMaxHeaderListSizeGoAway(maxHeaderListSize); } private int unconsumedBytes(Http2Stream stream) { 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 index 4b2860cd53..0de81ecaed 100644 --- 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 @@ -112,6 +112,18 @@ public final class Http2CodecUtil { public static final long DEFAULT_HEADER_LIST_SIZE = 8192; public static final int DEFAULT_MAX_FRAME_SIZE = MAX_FRAME_SIZE_LOWER_BOUND; + /** + * Calculate the threshold in bytes which should trigger a {@code GO_AWAY} if a set of headers exceeds this amount. + * @param maxHeaderListSize + * SETTINGS_MAX_HEADER_LIST_SIZE for the local + * endpoint. + * @return the threshold in bytes which should trigger a {@code GO_AWAY} if a set of headers exceeds this amount. + */ + public static long calculateMaxHeaderListSizeGoAway(long maxHeaderListSize) { + // This is equivalent to `maxHeaderListSize * 1.25` but we avoid floating point multiplication. + return maxHeaderListSize + (maxHeaderListSize >>> 2); + } + /** * Returns {@code true} if the stream is an outbound stream. * 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 index 045c66c6a1..3e8c8f8050 100644 --- 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 @@ -32,6 +32,7 @@ package io.netty.handler.codec.http2.internal.hpack; import io.netty.buffer.ByteBuf; +import io.netty.handler.codec.http2.Http2CodecUtil; import io.netty.handler.codec.http2.Http2Exception; import io.netty.handler.codec.http2.Http2Headers; import io.netty.handler.codec.http2.internal.hpack.HpackUtil.IndexType; @@ -106,7 +107,9 @@ public final class Decoder { * for testing but violate the RFC if used outside the scope of testing. */ Decoder(long maxHeaderListSize, int initialHuffmanDecodeCapacity, int maxHeaderTableSize) { - this.maxHeaderListSize = maxHeaderListSizeGoAway = checkPositive(maxHeaderListSize, "maxHeaderListSize"); + this.maxHeaderListSize = checkPositive(maxHeaderListSize, "maxHeaderListSize"); + this.maxHeaderListSizeGoAway = Http2CodecUtil.calculateMaxHeaderListSizeGoAway(maxHeaderListSize); + maxDynamicTableSize = encoderMaxDynamicTableSize = maxHeaderTableSize; maxDynamicTableSizeChangeRequired = false; dynamicTable = new DynamicTable(maxHeaderTableSize); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java index b6a6a3dd1b..efa52ab22c 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2HeadersDecoderTest.java @@ -84,6 +84,21 @@ public class DefaultHttp2HeadersDecoderTest { } } + @Test + public void decodeLargerThanHeaderListSizeButLessThanGoAwayWithInitialDecoderSettings() throws Exception { + ByteBuf buf = encode(b(":method"), b("GET"), b("test_header"), + b(String.format("%09000d", 0).replace('0', 'A'))); + final int streamId = 1; + try { + decoder.decodeHeaders(streamId, buf); + fail(); + } catch (Http2Exception.HeaderListSizeException e) { + assertEquals(streamId, e.streamId()); + } finally { + buf.release(); + } + } + @Test public void decodeLargerThanHeaderListSizeGoAway() throws Exception { decoder.maxHeaderListSize(MIN_HEADER_LIST_SIZE, MIN_HEADER_LIST_SIZE);