andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix possible leak in SslHandler if wrap(...) throws.

Browse Source 
Motivation:

We can end up with a buffer leak if SSLEngine.wrap(...) throws.

Modifications:

Correctly release the ByteBuf if SSLEngine.wrap(...) throws.

Result:

Fixes [#7337].
This commit is contained in:
Norman Maurer 2017-10-24 14:19:32 +02:00
parent dcbbae7f90
commit 92b786e2f3
1 changed files with 8 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
handler/src/main/java/io/netty/handler/ssl/SslHandler.java
View File

@ -742,13 +742,14 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
ChannelPromise promise = null; ChannelPromise promise = null;
ByteBufAllocator alloc = ctx.alloc(); ByteBufAllocator alloc = ctx.alloc();
boolean needUnwrap = false; boolean needUnwrap = false;
ByteBuf buf = null;
try { try {
final int wrapDataSize = this.wrapDataSize; final int wrapDataSize = this.wrapDataSize;
// Only continue to loop if the handler was not removed in the meantime. // Only continue to loop if the handler was not removed in the meantime.
// See https://github.com/netty/netty/issues/5860 // See https://github.com/netty/netty/issues/5860
while (!ctx.isRemoved()) { while (!ctx.isRemoved()) {
promise = ctx.newPromise(); promise = ctx.newPromise();
ByteBuf buf = wrapDataSize > 0 ? buf = wrapDataSize > 0 ?
pendingUnencryptedWrites.remove(alloc, wrapDataSize, promise) : pendingUnencryptedWrites.remove(alloc, wrapDataSize, promise) :
pendingUnencryptedWrites.removeFirst(promise); pendingUnencryptedWrites.removeFirst(promise);
if (buf == null) { if (buf == null) {
@ -763,6 +764,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
if (result.getStatus() == Status.CLOSED) { if (result.getStatus() == Status.CLOSED) {
buf.release(); buf.release();
buf = null;
promise.tryFailure(SSLENGINE_CLOSED); promise.tryFailure(SSLENGINE_CLOSED);
promise = null; promise = null;
// SSLEngine has been closed already. // SSLEngine has been closed already.
@ -775,6 +777,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
} else { } else {
buf.release(); buf.release();
} }
buf = null;
switch (result.getHandshakeStatus()) { switch (result.getHandshakeStatus()) {
case NEED_TASK: case NEED_TASK:
@ -801,6 +804,10 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
} }
} }
} finally { } finally {
// Ownership of buffer was not transferred, release it.
if (buf != null) {
buf.release();
}
finishWrap(ctx, out, promise, inUnwrap, needUnwrap); finishWrap(ctx, out, promise, inUnwrap, needUnwrap);
} }
} }