Skip to content

Commit 1cfd3a6

Browse files
authored
Fix possible buffer leak when stream can't be mapped (#14746)
Motivation: We should guard against the sitation where we can't map the stream. While this should never happen its better to be safe than sorry. Modifications: - Ensure the required Http2FrameStream can be found and only after that try to construct the specific frame. - Release ByteBuffer when DefaultHttp2DataFrame constructor throws Result: Fix possible buffer leaks
1 parent 8f9eadb commit 1cfd3a6

File tree

1 file changed

+24
-11
lines changed

1 file changed

+24
-11
lines changed

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java

+24-11
Original file line numberDiff line numberDiff line change
@@ -604,8 +604,8 @@ public void onUnknownFrame(
604604
// Ignore unknown frames on connection stream, for example: HTTP/2 GREASE testing
605605
return;
606606
}
607-
onHttp2Frame(ctx, newHttp2UnknownFrame(frameType, streamId, flags, payload.retain())
608-
.stream(requireStream(streamId)));
607+
Http2FrameStream stream = requireStream(streamId);
608+
onHttp2Frame(ctx, newHttp2UnknownFrame(frameType, streamId, flags, payload.retain()).stream(stream));
609609
}
610610

611611
@Override
@@ -625,7 +625,8 @@ public void onPingAckRead(ChannelHandlerContext ctx, long data) {
625625

626626
@Override
627627
public void onRstStreamRead(ChannelHandlerContext ctx, int streamId, long errorCode) {
628-
onHttp2Frame(ctx, new DefaultHttp2ResetFrame(errorCode).stream(requireStream(streamId)));
628+
Http2FrameStream stream = requireStream(streamId);
629+
onHttp2Frame(ctx, new DefaultHttp2ResetFrame(errorCode).stream(stream));
629630
}
630631

631632
@Override
@@ -634,7 +635,8 @@ public void onWindowUpdateRead(ChannelHandlerContext ctx, int streamId, int wind
634635
// Ignore connection window updates.
635636
return;
636637
}
637-
onHttp2Frame(ctx, new DefaultHttp2WindowUpdateFrame(windowSizeIncrement).stream(requireStream(streamId)));
638+
Http2FrameStream stream = requireStream(streamId);
639+
onHttp2Frame(ctx, new DefaultHttp2WindowUpdateFrame(windowSizeIncrement).stream(stream));
638640
}
639641

640642
@Override
@@ -647,22 +649,31 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId,
647649
@Override
648650
public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers,
649651
int padding, boolean endOfStream) {
650-
onHttp2Frame(ctx, new DefaultHttp2HeadersFrame(headers, endOfStream, padding)
651-
.stream(requireStream(streamId)));
652+
Http2FrameStream stream = requireStream(streamId);
653+
onHttp2Frame(ctx, new DefaultHttp2HeadersFrame(headers, endOfStream, padding).stream(stream));
652654
}
653655

654656
@Override
655657
public int onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
656658
boolean endOfStream) {
657-
onHttp2Frame(ctx, new DefaultHttp2DataFrame(data, endOfStream, padding)
658-
.stream(requireStream(streamId)).retain());
659+
Http2FrameStream stream = requireStream(streamId);
660+
final Http2DataFrame dataframe;
661+
try {
662+
dataframe = new DefaultHttp2DataFrame(data.retain(), endOfStream, padding);
663+
} catch (IllegalArgumentException e) {
664+
// Might be thrown in case of invalid padding / length.
665+
data.release();
666+
throw e;
667+
}
668+
dataframe.stream(stream);
669+
onHttp2Frame(ctx, dataframe);
659670
// We return the bytes in consumeBytes() once the stream channel consumed the bytes.
660671
return 0;
661672
}
662673

663674
@Override
664675
public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) {
665-
onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData).retain());
676+
onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData.retain()));
666677
}
667678

668679
@Override
@@ -674,8 +685,9 @@ public void onPriorityRead(ChannelHandlerContext ctx, int streamId, int streamDe
674685
// The stream was not opened yet, let's just ignore this for now.
675686
return;
676687
}
688+
Http2FrameStream frameStream = requireStream(streamId);
677689
onHttp2Frame(ctx, new DefaultHttp2PriorityFrame(streamDependency, weight, exclusive)
678-
.stream(requireStream(streamId)));
690+
.stream(frameStream));
679691
}
680692

681693
@Override
@@ -686,10 +698,11 @@ public void onSettingsAckRead(ChannelHandlerContext ctx) {
686698
@Override
687699
public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promisedStreamId,
688700
Http2Headers headers, int padding) {
701+
Http2FrameStream stream = requireStream(streamId);
689702
onHttp2Frame(ctx, new DefaultHttp2PushPromiseFrame(headers, padding, promisedStreamId)
690703
.pushStream(new DefaultHttp2FrameStream()
691704
.setStreamAndProperty(streamKey, connection().stream(promisedStreamId)))
692-
.stream(requireStream(streamId)));
705+
.stream(stream));
693706
}
694707

695708
private Http2FrameStream requireStream(int streamId) {

0 commit comments

Comments
 (0)