Skip to content

Commit d9ca50d

Browse files
minwooxminwoox
authored
Prevent sharing the index of the continuation frame header ByteBuf. (#13786)
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.
1 parent 0e7c27c commit d9ca50d

2 files changed

Lines changed: 39 additions & 12 deletions

File tree

‎codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ private ChannelFuture writeContinuationFrames(ChannelHandlerContext ctx, int str
567567
ByteBuf fragment = headerBlock.readRetainedSlice(fragmentReadableBytes);
568568

569569
if (headerBlock.isReadable()) {
570-
ctx.write(buf.retain(), promiseAggregator.newPromise());
570+
ctx.write(buf.retainedSlice(), promiseAggregator.newPromise());
571571
} else {
572572
// The frame header is different for the last frame, so re-allocate and release the old buffer
573573
flags = flags.endOfHeaders(true);

‎codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriterTest.java‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public void writeLargeHeaders() throws Exception {
189189
int streamId = 1;
190190
Http2Headers headers = new DefaultHttp2Headers()
191191
.method("GET").path("/").authority("foo.com").scheme("https");
192-
headers = dummyHeaders(headers, 20);
192+
headers = dummyHeaders(headers, 60);
193193

194194
http2HeadersEncoder.configuration().maxHeaderListSize(Integer.MAX_VALUE);
195195
frameWriter.headersConfiguration().maxHeaderListSize(Integer.MAX_VALUE);
@@ -198,7 +198,7 @@ public void writeLargeHeaders() throws Exception {
198198

199199
byte[] expectedPayload = headerPayload(streamId, headers);
200200

201-
// First frame: HEADER(length=0x4000, flags=0x01)
201+
// First frame: HEADER(length=0x4000, type=0x01, flags=0x01)
202202
assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
203203
outbound.readUnsignedMedium());
204204
assertEquals(0x01, outbound.readByte());
@@ -207,22 +207,49 @@ public void writeLargeHeaders() throws Exception {
207207

208208
byte[] firstPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
209209
outbound.readBytes(firstPayload);
210+
int index = 0;
211+
assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + firstPayload.length),
212+
firstPayload);
213+
index += firstPayload.length;
210214

211-
int remainPayloadLength = expectedPayload.length - Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND;
212-
// Second frame: CONTINUATION(length=remainPayloadLength, flags=0x04)
215+
// Second frame: HEADER(length=0x4000, type=0x09, flags=0x00)
216+
assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
217+
outbound.readUnsignedMedium());
218+
assertEquals(0x09, outbound.readByte());
219+
assertEquals(0x00, outbound.readByte());
220+
assertEquals(streamId, outbound.readInt());
221+
222+
byte[] secondPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
223+
outbound.readBytes(secondPayload);
224+
assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + secondPayload.length),
225+
secondPayload);
226+
index += secondPayload.length;
227+
228+
// third frame: HEADER(length=0x4000, type=0x09, flags=0x00)
229+
assertEquals(Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND,
230+
outbound.readUnsignedMedium());
231+
assertEquals(0x09, outbound.readByte());
232+
assertEquals(0x00, outbound.readByte());
233+
assertEquals(streamId, outbound.readInt());
234+
235+
byte[] thirdPayload = new byte[Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND];
236+
outbound.readBytes(thirdPayload);
237+
assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + thirdPayload.length),
238+
thirdPayload);
239+
index += thirdPayload.length;
240+
241+
int remainPayloadLength = expectedPayload.length - index;
242+
// Second frame: CONTINUATION(length=remainPayloadLength, type=0x09, flags=0x04)
213243
assertEquals(remainPayloadLength, outbound.readUnsignedMedium());
214244
assertEquals(0x09, outbound.readByte());
215245
assertEquals(0x04, outbound.readByte());
216246
assertEquals(streamId, outbound.readInt());
217247

218-
byte[] secondPayload = new byte[remainPayloadLength];
219-
outbound.readBytes(secondPayload);
248+
byte[] fourthPayload = new byte[remainPayloadLength];
249+
outbound.readBytes(fourthPayload);
220250

221-
assertArrayEquals(Arrays.copyOfRange(expectedPayload, 0, firstPayload.length),
222-
firstPayload);
223-
assertArrayEquals(Arrays.copyOfRange(expectedPayload, firstPayload.length,
224-
expectedPayload.length),
225-
secondPayload);
251+
assertArrayEquals(Arrays.copyOfRange(expectedPayload, index, index + fourthPayload.length),
252+
fourthPayload);
226253
}
227254

228255
@Test

0 commit comments

Comments
 (0)