From 7a7160f176597e97f451748a05c54d6b1518524c Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Fri, 29 Jan 2016 16:07:03 -0800 Subject: [PATCH] HTTP/2 Buffer Leak if UTF8 Conversion Fails 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. --- .../io/netty/handler/codec/http2/Http2CodecUtil.java | 10 +++++++++- .../handler/codec/http2/Http2ConnectionHandler.java | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java index 90051fc1b7..fd58f2cb1c 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java @@ -152,7 +152,15 @@ public final class Http2CodecUtil { // 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; } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java index 0dfd1ffd28..8e98c3cfcd 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java @@ -16,6 +16,7 @@ package io.netty.handler.codec.http2; 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; @@ -705,7 +706,13 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http */ 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()); }