diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java index bff86df733..15568344ac 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java @@ -14,20 +14,21 @@ */ package io.netty.handler.codec.http2; -import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_FRAME_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.FRAME_HEADER_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.INT_FIELD_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_ENTRY_LENGTH; +import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_MAX_FRAME_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTING_ENTRY_LENGTH; +import static io.netty.handler.codec.http2.Http2CodecUtil.isMaxFrameSizeValid; +import static io.netty.handler.codec.http2.Http2CodecUtil.readUnsignedInt; +import static io.netty.handler.codec.http2.Http2Error.ENHANCE_YOUR_CALM; import static io.netty.handler.codec.http2.Http2Error.FLOW_CONTROL_ERROR; import static io.netty.handler.codec.http2.Http2Error.FRAME_SIZE_ERROR; import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; -import static io.netty.handler.codec.http2.Http2CodecUtil.isMaxFrameSizeValid; -import static io.netty.handler.codec.http2.Http2CodecUtil.readUnsignedInt; -import static io.netty.handler.codec.http2.Http2Exception.streamError; import static io.netty.handler.codec.http2.Http2Exception.connectionError; +import static io.netty.handler.codec.http2.Http2Exception.streamError; import static io.netty.handler.codec.http2.Http2FrameTypes.CONTINUATION; import static io.netty.handler.codec.http2.Http2FrameTypes.DATA; import static io.netty.handler.codec.http2.Http2FrameTypes.GO_AWAY; @@ -421,9 +422,8 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize final HeadersBlockBuilder hdrBlockBuilder = headersBlockBuilder(); hdrBlockBuilder.addFragment(fragment, ctx.alloc(), endOfHeaders); if (endOfHeaders) { - listener.onHeadersRead(ctx, headersStreamId, hdrBlockBuilder.headers(), - streamDependency, weight, exclusive, padding, headersFlags.endOfStream()); - close(); + listener.onHeadersRead(ctx, headersStreamId, hdrBlockBuilder.headers(), streamDependency, + weight, exclusive, padding, headersFlags.endOfStream()); } } }; @@ -449,7 +449,6 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize if (endOfHeaders) { listener.onHeadersRead(ctx, headersStreamId, hdrBlockBuilder.headers(), padding, headersFlags.endOfStream()); - close(); } } }; @@ -519,10 +518,8 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize Http2FrameListener listener) throws Http2Exception { headersBlockBuilder().addFragment(fragment, ctx.alloc(), endOfHeaders); if (endOfHeaders) { - Http2Headers headers = headersBlockBuilder().headers(); - listener.onPushPromiseRead(ctx, pushPromiseStreamId, promisedStreamId, headers, - padding); - close(); + listener.onPushPromiseRead(ctx, pushPromiseStreamId, promisedStreamId, + headersBlockBuilder().headers(), padding); } } }; @@ -626,6 +623,16 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize protected class HeadersBlockBuilder { private ByteBuf headerBlock; + /** + * The local header size maximum has been exceeded while accumulating bytes. + * @throws Http2Exception A connection error indicating too much data has been received. + */ + private void headerSizeExceeded() throws Http2Exception { + close(); + throw connectionError(ENHANCE_YOUR_CALM, "Header size exceeded max allowed size (%d)", + headersDecoder.configuration().maxHeaderSize()); + } + /** * Adds a fragment to the block. * @@ -635,8 +642,11 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize * This is used for an optimization for when the first fragment is the full * block. In that case, the buffer is used directly without copying. */ - final void addFragment(ByteBuf fragment, ByteBufAllocator alloc, boolean endOfHeaders) { + final void addFragment(ByteBuf fragment, ByteBufAllocator alloc, boolean endOfHeaders) throws Http2Exception { if (headerBlock == null) { + if (fragment.readableBytes() > headersDecoder.configuration().maxHeaderSize()) { + headerSizeExceeded(); + } if (endOfHeaders) { // Optimization - don't bother copying, just use the buffer as-is. Need // to retain since we release when the header block is built. @@ -647,6 +657,10 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize } return; } + if (headersDecoder.configuration().maxHeaderSize() - fragment.readableBytes() < + headerBlock.readableBytes()) { + headerSizeExceeded(); + } if (headerBlock.isWritable(fragment.readableBytes())) { // The buffer can hold the requested bytes, just write it directly. headerBlock.writeBytes(fragment); 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 index be207263c2..2b35e61602 100644 --- 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 @@ -18,6 +18,7 @@ package io.netty.handler.codec.http2; 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.Http2Error.COMPRESSION_ERROR; +import static io.netty.handler.codec.http2.Http2Error.ENHANCE_YOUR_CALM; import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; import static io.netty.handler.codec.http2.Http2Exception.connectionError; @@ -32,6 +33,7 @@ import com.twitter.hpack.Decoder; import com.twitter.hpack.HeaderListener; public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2HeadersDecoder.Configuration { + private final int maxHeaderSize; private final Decoder decoder; private final Http2HeaderTable headerTable; @@ -40,8 +42,12 @@ public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2Hea } public DefaultHttp2HeadersDecoder(int maxHeaderSize, int maxHeaderTableSize) { + if (maxHeaderSize <= 0) { + throw new IllegalArgumentException("maxHeaderSize must be positive: " + maxHeaderSize); + } decoder = new Decoder(maxHeaderSize, maxHeaderTableSize); headerTable = new Http2HeaderTableDecoder(); + this.maxHeaderSize = maxHeaderSize; } @Override @@ -49,11 +55,24 @@ public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2Hea return headerTable; } + @Override + public int maxHeaderSize() { + return maxHeaderSize; + } + @Override public Configuration configuration() { return this; } + /** + * Respond to headers block resulting in the maximum header size being exceeded. + * @throws Http2Exception If we can not recover from the truncation. + */ + protected void maxHeaderSizeExceeded() throws Http2Exception { + throw connectionError(ENHANCE_YOUR_CALM, "Header size exceeded max allowed bytes (%d)", maxHeaderSize); + } + @Override public Http2Headers decodeHeaders(ByteBuf headerBlock) throws Http2Exception { InputStream in = new ByteBufInputStream(headerBlock); @@ -67,9 +86,8 @@ public class DefaultHttp2HeadersDecoder implements Http2HeadersDecoder, Http2Hea }; decoder.decode(in, listener); - boolean truncated = decoder.endHeaderBlock(); - if (truncated) { - // TODO: what's the right thing to do here? + if (decoder.endHeaderBlock()) { + maxHeaderSizeExceeded(); } if (headers.size() > headerTable.maxHeaderListSize()) { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeadersDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeadersDecoder.java index e43bafed3a..098e46a1d6 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeadersDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2HeadersDecoder.java @@ -29,6 +29,11 @@ public interface Http2HeadersDecoder { * Access the Http2HeaderTable for this {@link Http2HeadersDecoder} */ Http2HeaderTable headerTable(); + + /** + * Get the maximum number of bytes that is allowed before truncation occurs. + */ + int maxHeaderSize(); } /** diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java index 306ab30b07..d0c464d3c5 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameIOTest.java @@ -15,15 +15,20 @@ package io.netty.handler.codec.http2; +import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_HEADER_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_UNSIGNED_INT; import static io.netty.handler.codec.http2.Http2TestUtil.as; import static io.netty.handler.codec.http2.Http2TestUtil.randomString; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyShort; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; - import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; import io.netty.buffer.Unpooled; @@ -33,8 +38,10 @@ import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPromise; +import io.netty.util.ByteString; import io.netty.util.CharsetUtil; import io.netty.util.concurrent.EventExecutor; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -291,6 +298,20 @@ public class DefaultHttp2FrameIOTest { eq(true)); } + @Test + public void headersThatAreTooBigShouldFail() throws Exception { + Http2Headers headers = headersOfSize(DEFAULT_MAX_HEADER_SIZE + 1); + writer.writeHeaders(ctx, 1, headers, 2, (short) 3, true, 0xFF, true, promise); + try { + reader.readFrame(ctx, buffer, listener); + fail(); + } catch (Http2Exception e) { + verify(listener, never()).onHeadersRead(any(ChannelHandlerContext.class), anyInt(), + any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), + anyBoolean()); + } + } + @Test public void emptypushPromiseShouldRoundtrip() throws Exception { Http2Headers headers = EmptyHttp2Headers.INSTANCE; @@ -357,4 +378,13 @@ public class DefaultHttp2FrameIOTest { } return headers; } + + private Http2Headers headersOfSize(final int minSize) { + final ByteString singleByte = new ByteString(new byte[]{0}); + DefaultHttp2Headers headers = new DefaultHttp2Headers(); + for (int size = 0; size < minSize; size += 2) { + headers.add(singleByte, singleByte); + } + return headers; + } } 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 abd4b35b72..2067d9fd11 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 @@ -15,11 +15,13 @@ package io.netty.handler.codec.http2; +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.Http2TestUtil.as; import static io.netty.handler.codec.http2.Http2TestUtil.randomBytes; import static io.netty.util.CharsetUtil.UTF_8; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -55,6 +57,17 @@ public class DefaultHttp2HeadersDecoderTest { } } + @Test(expected = Http2Exception.class) + public void testExceedHeaderSize() throws Exception { + ByteBuf buf = encode(randomBytes(DEFAULT_MAX_HEADER_SIZE), randomBytes(1)); + try { + decoder.decodeHeaders(buf); + fail(); + } finally { + buf.release(); + } + } + private static byte[] b(String string) { return string.getBytes(UTF_8); } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java index 641b7179da..68900e5956 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java @@ -1189,6 +1189,7 @@ public class DefaultHttp2RemoteFlowControllerTest { final Http2RemoteFlowController.FlowControlled flowControlled = mockedFlowControlledThatThrowsOnWrite(); final Http2Stream stream = stream(STREAM_A); doAnswer(new Answer() { + @Override public Void answer(InvocationOnMock invocationOnMock) { stream.closeLocalSide(); return null; @@ -1212,6 +1213,7 @@ public class DefaultHttp2RemoteFlowControllerTest { final Http2RemoteFlowController.FlowControlled flowControlled = mockedFlowControlledThatThrowsOnWrite(); final Http2Stream stream = stream(STREAM_A); doAnswer(new Answer() { + @Override public Void answer(InvocationOnMock invocationOnMock) { throw new RuntimeException("error failed"); } @@ -1257,6 +1259,7 @@ public class DefaultHttp2RemoteFlowControllerTest { final Http2Stream stream = stream(STREAM_A); doAnswer(new Answer() { + @Override public Void answer(InvocationOnMock invocationOnMock) { throw new RuntimeException("writeComplete failed"); } @@ -1286,6 +1289,7 @@ public class DefaultHttp2RemoteFlowControllerTest { when(flowControlled.size()).thenReturn(100); doThrow(new RuntimeException("write failed")).when(flowControlled).write(anyInt()); doAnswer(new Answer() { + @Override public Void answer(InvocationOnMock invocationOnMock) { stream.close(); return null; diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java index 7925267b85..8c3060465c 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java @@ -70,7 +70,14 @@ final class Http2TestUtil { * Returns a byte array filled with random data. */ public static byte[] randomBytes() { - byte[] data = new byte[100]; + return randomBytes(100); + } + + /** + * Returns a byte array filled with random data. + */ + public static byte[] randomBytes(int size) { + byte[] data = new byte[size]; new Random().nextBytes(data); return data; }