Skip to content

Commit

Permalink
Fix possible buffer leak when stream can't be mapped (#14746)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
normanmaurer authored Jan 31, 2025
1 parent 8f9eadb commit 1cfd3a6
Showing 1 changed file with 24 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -604,8 +604,8 @@ public void onUnknownFrame(
// Ignore unknown frames on connection stream, for example: HTTP/2 GREASE testing
return;
}
onHttp2Frame(ctx, newHttp2UnknownFrame(frameType, streamId, flags, payload.retain())
.stream(requireStream(streamId)));
Http2FrameStream stream = requireStream(streamId);
onHttp2Frame(ctx, newHttp2UnknownFrame(frameType, streamId, flags, payload.retain()).stream(stream));
}

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

@Override
public void onRstStreamRead(ChannelHandlerContext ctx, int streamId, long errorCode) {
onHttp2Frame(ctx, new DefaultHttp2ResetFrame(errorCode).stream(requireStream(streamId)));
Http2FrameStream stream = requireStream(streamId);
onHttp2Frame(ctx, new DefaultHttp2ResetFrame(errorCode).stream(stream));
}

@Override
Expand All @@ -634,7 +635,8 @@ public void onWindowUpdateRead(ChannelHandlerContext ctx, int streamId, int wind
// Ignore connection window updates.
return;
}
onHttp2Frame(ctx, new DefaultHttp2WindowUpdateFrame(windowSizeIncrement).stream(requireStream(streamId)));
Http2FrameStream stream = requireStream(streamId);
onHttp2Frame(ctx, new DefaultHttp2WindowUpdateFrame(windowSizeIncrement).stream(stream));
}

@Override
Expand All @@ -647,22 +649,31 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId,
@Override
public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers,
int padding, boolean endOfStream) {
onHttp2Frame(ctx, new DefaultHttp2HeadersFrame(headers, endOfStream, padding)
.stream(requireStream(streamId)));
Http2FrameStream stream = requireStream(streamId);
onHttp2Frame(ctx, new DefaultHttp2HeadersFrame(headers, endOfStream, padding).stream(stream));
}

@Override
public int onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
boolean endOfStream) {
onHttp2Frame(ctx, new DefaultHttp2DataFrame(data, endOfStream, padding)
.stream(requireStream(streamId)).retain());
Http2FrameStream stream = requireStream(streamId);
final Http2DataFrame dataframe;
try {
dataframe = new DefaultHttp2DataFrame(data.retain(), endOfStream, padding);
} catch (IllegalArgumentException e) {
// Might be thrown in case of invalid padding / length.
data.release();
throw e;
}
dataframe.stream(stream);
onHttp2Frame(ctx, dataframe);
// We return the bytes in consumeBytes() once the stream channel consumed the bytes.
return 0;
}

@Override
public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) {
onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData).retain());
onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData.retain()));
}

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

@Override
Expand All @@ -686,10 +698,11 @@ public void onSettingsAckRead(ChannelHandlerContext ctx) {
@Override
public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promisedStreamId,
Http2Headers headers, int padding) {
Http2FrameStream stream = requireStream(streamId);
onHttp2Frame(ctx, new DefaultHttp2PushPromiseFrame(headers, padding, promisedStreamId)
.pushStream(new DefaultHttp2FrameStream()
.setStreamAndProperty(streamKey, connection().stream(promisedStreamId)))
.stream(requireStream(streamId)));
.stream(stream));
}

private Http2FrameStream requireStream(int streamId) {
Expand Down

0 comments on commit 1cfd3a6

Please # to comment.