113 lines
6.3 KiB
Diff
113 lines
6.3 KiB
Diff
From 284ad8e5ce3ec87964dea6ec8a611dfb4df62a29 Mon Sep 17 00:00:00 2001
|
|
From: songmw725 <songmw725@gmail.com>
|
|
Date: Thu, 18 Jan 2024 00:11:34 +0900
|
|
Subject: [PATCH] Prevent sharing the index of the continuation frame header
|
|
ByteBuf. Motivation: The current implementation uses the `byteBuf` for a
|
|
continuation frame header multiple times if the header length exceeds `3 *
|
|
maxFrameLength`. However, it fails to slice the `byteBuf` during usage.
|
|
[Reference](https://github.com/netty/netty/blob/d027ba7320d430743992d613e52596b0182ca854/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java#L570)
|
|
|
|
Modification:
|
|
- Introduce `ByteBuf.retainedSlice()` for a continuation frame header when it's used to prevent sharing the index.
|
|
|
|
Result:
|
|
- Correctly send continuation frame headers to the remote peer, addressing the issue of reusing the index of the ByteBuf.
|
|
---
|
|
.../codec/http2/DefaultHttp2FrameWriter.java | 2 +-
|
|
.../http2/DefaultHttp2FrameWriterTest.java | 49 ++++++++++++++-----
|
|
2 files changed, 39 insertions(+), 12 deletions(-)
|
|
|
|
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 9b608921c28b..8540e0c2ed6e 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
|
|
@@ -567,7 +567,7 @@ private ChannelFuture writeContinuationFrames(ChannelHandlerContext ctx, int str
|
|
ByteBuf fragment = headerBlock.readRetainedSlice(fragmentReadableBytes);
|
|
|
|
if (headerBlock.isReadable()) {
|
|
- ctx.write(buf.retain(), promiseAggregator.newPromise());
|
|
+ ctx.write(buf.retainedSlice(), promiseAggregator.newPromise());
|
|
} else {
|
|
// The frame header is different for the last frame, so re-allocate and release the old buffer
|
|
flags = flags.endOfHeaders(true);
|
|
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 e134fb8194df..8311a20823ef 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
|
|
@@ -189,7 +189,7 @@ public void writeLargeHeaders() throws Exception {
|
|
int streamId = 1;
|
|
Http2Headers headers = new DefaultHttp2Headers()
|
|
.method("GET").path("/").authority("foo.com").scheme("https");
|
|
- headers = dummyHeaders(headers, 20);
|
|
+ headers = dummyHeaders(headers, 60);
|
|
|
|
http2HeadersEncoder.configuration().maxHeaderListSize(Integer.MAX_VALUE);
|
|
frameWriter.headersConfiguration().maxHeaderListSize(Integer.MAX_VALUE);
|
|
@@ -198,7 +198,7 @@ public void writeLargeHeaders() throws Exception {
|
|
|
|
byte[] expectedPayload = headerPayload(streamId, headers);
|
|
|
|
- // First frame: HEADER(length=0x4000, flags=0x01)
|
|
+ // First frame: HEADER(length=0x4000, type=0x01, flags=0x01)
|
|
assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
|
outbound.readUnsignedMedium());
|
|
assertEquals(0x01, outbound.readByte());
|
|
@@ -207,22 +207,49 @@ public void writeLargeHeaders() throws Exception {
|
|
|
|
byte[] firstPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
|
outbound.readBytes(firstPayload);
|
|
+ int index = 0;
|
|
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + firstPayload.length),
|
|
+ firstPayload);
|
|
+ index += firstPayload.length;
|
|
|
|
- int remainPayloadLength = expectedPayload.length - Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND;
|
|
- // Second frame: CONTINUATION(length=remainPayloadLength, flags=0x04)
|
|
+ // Second frame: HEADER(length=0x4000, type=0x09, flags=0x00)
|
|
+ assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
|
+ outbound.readUnsignedMedium());
|
|
+ assertEquals(0x09, outbound.readByte());
|
|
+ assertEquals(0x00, outbound.readByte());
|
|
+ assertEquals(streamId, outbound.readInt());
|
|
+
|
|
+ byte[] secondPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
|
+ outbound.readBytes(secondPayload);
|
|
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + secondPayload.length),
|
|
+ secondPayload);
|
|
+ index += secondPayload.length;
|
|
+
|
|
+ // third frame: HEADER(length=0x4000, type=0x09, flags=0x00)
|
|
+ assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
|
|
+ outbound.readUnsignedMedium());
|
|
+ assertEquals(0x09, outbound.readByte());
|
|
+ assertEquals(0x00, outbound.readByte());
|
|
+ assertEquals(streamId, outbound.readInt());
|
|
+
|
|
+ byte[] thirdPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
|
|
+ outbound.readBytes(thirdPayload);
|
|
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + thirdPayload.length),
|
|
+ thirdPayload);
|
|
+ index += thirdPayload.length;
|
|
+
|
|
+ int remainPayloadLength = expectedPayload.length - index;
|
|
+ // Second frame: CONTINUATION(length=remainPayloadLength, type=0x09, 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);
|
|
+ byte[] fourthPayload = new byte[remainPayloadLength];
|
|
+ outbound.readBytes(fourthPayload);
|
|
|
|
- assertArrayEquals(Arrays.copyOfRange(expectedPayload, 0, firstPayload.length),
|
|
- firstPayload);
|
|
- assertArrayEquals(Arrays.copyOfRange(expectedPayload, firstPayload.length,
|
|
- expectedPayload.length),
|
|
- secondPayload);
|
|
+ assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + fourthPayload.length),
|
|
+ fourthPayload);
|
|
}
|
|
|
|
@Test
|