From 908b68da03eb9b5f20ac5b49ae721f8a8d845f14 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Thu, 20 Nov 2014 16:37:05 -0500 Subject: [PATCH] HTTP/2 Decompress/Compress buffer leak Motivation: The interface for HTTP/2 onDataRead states that buffers will be released by the codec. The decompressor and compressor methods are not releasing buffers created during the decompression/compression process. Modifications: After onDataRead calls the decompressor and compressor classes will release the data buffer. Result: HTTP/2 compressor/decompressors are consistent with onDataRead interface assumptions. --- .../DelegatingDecompressorFrameListener.java | 37 +++++++++++-------- .../codec/http2/DataCompressionHttp2Test.java | 1 - 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java index 8f401e5983..9bc9d88aea 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java @@ -94,23 +94,30 @@ public class DelegatingDecompressorFrameListener extends Http2FrameListenerDecor decompressor.incrementDecompressedByes(compressedBytes); processedBytes = compressedBytes; } else { - decompressor.incrementDecompressedByes(padding); - for (;;) { - ByteBuf nextBuf = nextReadableBuf(channel); - boolean decompressedEndOfStream = nextBuf == null && endOfStream; - if (decompressedEndOfStream && channel.finish()) { - nextBuf = nextReadableBuf(channel); - decompressedEndOfStream = nextBuf == null; - } + try { + decompressor.incrementDecompressedByes(padding); + for (;;) { + ByteBuf nextBuf = nextReadableBuf(channel); + boolean decompressedEndOfStream = nextBuf == null && endOfStream; + if (decompressedEndOfStream && channel.finish()) { + nextBuf = nextReadableBuf(channel); + decompressedEndOfStream = nextBuf == null; + } - decompressor.incrementDecompressedByes(buf.readableBytes()); - processedBytes += listener.onDataRead(ctx, streamId, buf, padding, decompressedEndOfStream); - if (nextBuf == null) { - break; - } + decompressor.incrementDecompressedByes(buf.readableBytes()); + processedBytes += listener.onDataRead(ctx, streamId, buf, padding, decompressedEndOfStream); + if (nextBuf == null) { + break; + } - padding = 0; // Padding is only communicated once on the first iteration - buf = nextBuf; + padding = 0; // Padding is only communicated once on the first iteration + buf.release(); + buf = nextBuf; + } + } finally { + if (buf != null) { + buf.release(); + } } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java index b7f3125101..756c255921 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java @@ -341,7 +341,6 @@ public class DataCompressionHttp2Test { int processedBytes = buf.readableBytes() + padding; buf.readBytes(serverOut, buf.readableBytes()); - buf.release(); return processedBytes; } }).when(serverListener).onDataRead(any(ChannelHandlerContext.class), anyInt(),