diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java index e2fcc334e0..196cbabb09 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java @@ -386,7 +386,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize } if (!flags.endOfHeaders()) { - writeContinuationFrames(ctx, streamId, headerBlock, padding, promiseAggregator); + writeContinuationFrames(ctx, streamId, headerBlock, promiseAggregator); } } catch (Http2Exception e) { promiseAggregator.setFailure(e); @@ -533,7 +533,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize } if (!flags.endOfHeaders()) { - writeContinuationFrames(ctx, streamId, headerBlock, padding, promiseAggregator); + writeContinuationFrames(ctx, streamId, headerBlock, promiseAggregator); } } catch (Http2Exception e) { promiseAggregator.setFailure(e); @@ -553,28 +553,19 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize * Writes as many continuation frames as needed until {@code padding} and {@code headerBlock} are consumed. */ private ChannelFuture writeContinuationFrames(ChannelHandlerContext ctx, int streamId, - ByteBuf headerBlock, int padding, SimpleChannelPromiseAggregator promiseAggregator) { - Http2Flags flags = new Http2Flags().paddingPresent(padding > 0); - int maxFragmentLength = maxFrameSize - padding; - // TODO: same padding is applied to all frames, is this desired? - if (maxFragmentLength <= 0) { - return promiseAggregator.setFailure(new IllegalArgumentException( - "Padding [" + padding + "] is too large for max frame size [" + maxFrameSize + "]")); - } + ByteBuf headerBlock, SimpleChannelPromiseAggregator promiseAggregator) { + Http2Flags flags = new Http2Flags(); if (headerBlock.isReadable()) { // The frame header (and padding) only changes on the last frame, so allocate it once and re-use - int fragmentReadableBytes = min(headerBlock.readableBytes(), maxFragmentLength); - int payloadLength = fragmentReadableBytes + padding; + int fragmentReadableBytes = min(headerBlock.readableBytes(), maxFrameSize); ByteBuf buf = ctx.alloc().buffer(CONTINUATION_FRAME_HEADER_LENGTH); - writeFrameHeaderInternal(buf, payloadLength, CONTINUATION, flags, streamId); - writePaddingLength(buf, padding); + writeFrameHeaderInternal(buf, fragmentReadableBytes, CONTINUATION, flags, streamId); do { - fragmentReadableBytes = min(headerBlock.readableBytes(), maxFragmentLength); + fragmentReadableBytes = min(headerBlock.readableBytes(), maxFrameSize); ByteBuf fragment = headerBlock.readRetainedSlice(fragmentReadableBytes); - payloadLength = fragmentReadableBytes + padding; if (headerBlock.isReadable()) { ctx.write(buf.retain(), promiseAggregator.newPromise()); } else { @@ -582,17 +573,12 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize flags = flags.endOfHeaders(true); buf.release(); buf = ctx.alloc().buffer(CONTINUATION_FRAME_HEADER_LENGTH); - writeFrameHeaderInternal(buf, payloadLength, CONTINUATION, flags, streamId); - writePaddingLength(buf, padding); + writeFrameHeaderInternal(buf, fragmentReadableBytes, CONTINUATION, flags, streamId); ctx.write(buf, promiseAggregator.newPromise()); } ctx.write(fragment, promiseAggregator.newPromise()); - // Write out the padding, if any. - if (paddingBytes(padding) > 0) { - ctx.write(ZERO_BUFFER.slice(0, paddingBytes(padding)), promiseAggregator.newPromise()); - } } while (headerBlock.isReadable()); } return promiseAggregator; diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java index 81db8fbc1f..af1a9baa8f 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java @@ -202,6 +202,48 @@ public class DefaultHttp2FrameWriterTest { secondPayload); } + @Test + public void writeLargeHeaderWithPadding() throws Exception { + int streamId = 1; + Http2Headers headers = new DefaultHttp2Headers() + .method("GET").path("/").authority("foo.com").scheme("https"); + headers = dummyHeaders(headers, 20); + + http2HeadersEncoder.configuration().maxHeaderListSize(Integer.MAX_VALUE); + frameWriter.headersConfiguration().maxHeaderListSize(Integer.MAX_VALUE); + frameWriter.maxFrameSize(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND); + frameWriter.writeHeaders(ctx, streamId, headers, 5, true, promise); + + byte[] expectedPayload = buildLargeHeaderPayload(streamId, headers, (byte) 4, + Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND); + + // First frame: HEADER(length=0x4000, flags=0x09) + assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND, + outbound.readUnsignedMedium()); + assertEquals(0x01, outbound.readByte()); + assertEquals(0x09, outbound.readByte()); // 0x01 + 0x08 + assertEquals(streamId, outbound.readInt()); + + byte[] firstPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND]; + outbound.readBytes(firstPayload); + + int remainPayloadLength = expectedPayload.length - Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND; + // Second frame: CONTINUATION(length=remainPayloadLength, flags=0x04) + assertEquals(remainPayloadLength, outbound.readUnsignedMedium()); + assertEquals(0x09, outbound.readByte()); + assertEquals(0x04, outbound.readByte()); + assertEquals(streamId, outbound.readInt()); + + byte[] secondPayload = new byte[remainPayloadLength]; + outbound.readBytes(secondPayload); + + assertArrayEquals(Arrays.copyOfRange(expectedPayload, 0, firstPayload.length), + firstPayload); + assertArrayEquals(Arrays.copyOfRange(expectedPayload, firstPayload.length, + expectedPayload.length), + secondPayload); + } + @Test public void writeFrameZeroPayload() throws Exception { frameWriter.writeFrame(ctx, (byte) 0xf, 0, new Http2Flags(), Unpooled.EMPTY_BUFFER, promise); @@ -263,6 +305,22 @@ public class DefaultHttp2FrameWriterTest { } } + private byte[] buildLargeHeaderPayload(int streamId, Http2Headers headers, byte padding, int maxFrameSize) + throws Http2Exception, IOException { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + try { + outputStream.write(padding); + byte[] payload = headerPayload(streamId, headers); + int firstPayloadSize = maxFrameSize - (padding + 1); //1 for padding length + outputStream.write(payload, 0, firstPayloadSize); + outputStream.write(new byte[padding]); + outputStream.write(payload, firstPayloadSize, payload.length - firstPayloadSize); + return outputStream.toByteArray(); + } finally { + outputStream.close(); + } + } + private static Http2Headers dummyHeaders(Http2Headers headers, int times) { final String largeValue = repeat("dummy-value", 100); for (int i = 0; i < times; i++) {