SslHandler avoid calling wrap/unwrap when unnecessary

Motivation:
The SSLEngine wrap and unwrap methods can be called in a way that has no side effects, but this could involve costly validation and allocation. The SslHandler should avoid calling into these methods if possible.

Modifications:
- wrapNonAppData should provide additional status which can be used by wrap to breakout early if possible

Result:
SslHandler invokes the SSLEngine less.
This commit is contained in:
Scott Mitchell 2017-02-03 16:19:27 -08:00
parent 72916b9960
commit 6353c229fd

View File

@ -725,8 +725,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
} }
} }
// This method will not call setHandshakeFailure(...) ! /**
private void wrapNonAppData(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException { * This method will not call
* {@link #setHandshakeFailure(io.netty.channel.ChannelHandlerContext, Throwable, boolean)} or
* {@link #setHandshakeFailure(io.netty.channel.ChannelHandlerContext, Throwable)}.
* @return {@code true} if this method ends on {@link SSLEngineResult.HandshakeStatus#NOT_HANDSHAKING}.
*/
private boolean wrapNonAppData(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException {
ByteBuf out = null; ByteBuf out = null;
ByteBufAllocator alloc = ctx.alloc(); ByteBufAllocator alloc = ctx.alloc();
try { try {
@ -752,7 +757,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
switch (result.getHandshakeStatus()) { switch (result.getHandshakeStatus()) {
case FINISHED: case FINISHED:
setHandshakeSuccess(); setHandshakeSuccess();
break; return false;
case NEED_TASK: case NEED_TASK:
runDelegatedTasks(); runDelegatedTasks();
break; break;
@ -770,7 +775,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
if (!inUnwrap) { if (!inUnwrap) {
unwrapNonAppData(ctx); unwrapNonAppData(ctx);
} }
break; return true;
default: default:
throw new IllegalStateException("Unknown handshake status: " + result.getHandshakeStatus()); throw new IllegalStateException("Unknown handshake status: " + result.getHandshakeStatus());
} }
@ -790,6 +795,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
out.release(); out.release();
} }
} }
return false;
} }
private SSLEngineResult wrap(ByteBufAllocator alloc, SSLEngine engine, ByteBuf in, ByteBuf out) private SSLEngineResult wrap(ByteBufAllocator alloc, SSLEngine engine, ByteBuf in, ByteBuf out)
@ -1113,7 +1119,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
try { try {
// 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()) { unwrapLoop: while (!ctx.isRemoved()) {
final SSLEngineResult result = engineType.unwrap(this, packet, offset, length, decodeOut); final SSLEngineResult result = engineType.unwrap(this, packet, offset, length, decodeOut);
final Status status = result.getStatus(); final Status status = result.getStatus();
final HandshakeStatus handshakeStatus = result.getHandshakeStatus(); final HandshakeStatus handshakeStatus = result.getHandshakeStatus();
@ -1163,7 +1169,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
case NEED_UNWRAP: case NEED_UNWRAP:
break; break;
case NEED_WRAP: case NEED_WRAP:
wrapNonAppData(ctx, true); // If the wrap operation transitions the status to NOT_HANDSHAKING and there is no more data to
// unwrap then the next call to unwrap will not produce any data. We can avoid the potentially
// costly unwrap operation and break out of the loop.
if (wrapNonAppData(ctx, true) && length == 0) {
break unwrapLoop;
}
break; break;
case NEED_TASK: case NEED_TASK:
runDelegatedTasks(); runDelegatedTasks();
@ -1196,7 +1207,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
flushedBeforeHandshake = false; flushedBeforeHandshake = false;
wrapLater = true; wrapLater = true;
} }
// If we are not handshaking and there is no more data to unwrap then the next call to unwrap
// will not produce any data. We can avoid the potentially costly unwrap operation and break
// out of the loop.
if (length == 0) {
break unwrapLoop;
}
break; break;
default: default:
throw new IllegalStateException("unknown handshake status: " + handshakeStatus); throw new IllegalStateException("unknown handshake status: " + handshakeStatus);