Skip to content

Commit

Permalink
HTTP/2 Buffer Leak if UTF8 Conversion Fails
Browse files Browse the repository at this point in the history
Motivation:
Http2CodecUtil uses ByteBufUtil.writeUtf8 but does not account for it
throwing an exception. If an exception is thrown because the format is
not valid UTF16 encoded UTF8 then the buffer will leak.

Modifications:
- Make sure the buffer is released if an exception is thrown
- Ensure call sites of the Http2CodecUtil.toByteBuf can tolerate and
  exception being thrown

Result:
No leak if exception data can not be converted to UTF8.
  • Loading branch information
Scottmitch committed Feb 2, 2016
1 parent a75dcb2 commit 7a7160f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,15 @@ public static ByteBuf toByteBuf(ChannelHandlerContext ctx, Throwable cause) {

// Create the debug message. `* 3` because UTF-8 max character consumes 3 bytes.
ByteBuf debugData = ctx.alloc().buffer(cause.getMessage().length() * 3);
ByteBufUtil.writeUtf8(debugData, cause.getMessage());
boolean shouldRelease = true;
try {
ByteBufUtil.writeUtf8(debugData, cause.getMessage());
shouldRelease = false;
} finally {
if (shouldRelease) {
debugData.release();
}
}
return debugData;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
Expand Down Expand Up @@ -705,7 +706,13 @@ private void checkCloseConnection(ChannelFuture future) {
*/
private ChannelFuture goAway(ChannelHandlerContext ctx, Http2Exception cause) {
long errorCode = cause != null ? cause.error().code() : NO_ERROR.code();
ByteBuf debugData = Http2CodecUtil.toByteBuf(ctx, cause);
ByteBuf debugData = Unpooled.EMPTY_BUFFER;
try {
debugData = Http2CodecUtil.toByteBuf(ctx, cause);
} catch (Throwable t) {
// We must continue on to prevent our internal state from becoming corrupted but we log this exception.
logger.warn("caught exception while translating " + cause + " to ByteBuf", t);
}
int lastKnownStream = connection().remote().lastStreamCreated();
return goAway(ctx, lastKnownStream, errorCode, debugData, ctx.newPromise());
}
Expand Down

0 comments on commit 7a7160f

Please sign in to comment.