From 81fb2eede866433be8455c040b0455d4791775ca Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Fri, 2 Jun 2017 09:26:27 -0700
Subject: [PATCH] Revert "Revert "SslHandler avoid calling wrap/unwrap when
 unnecessary""

Motivation:
PR https://github.com/netty/netty/pull/6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization.

Modifications:
- This reverts commit 7f3b75a5091dcd6d882102fdb92daa6931e02c30.

Result:
SslHandler optimization is restored.
---
 .../java/io/netty/handler/ssl/SslHandler.java | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
index e21b2dee15..b03417d15e 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
@@ -837,7 +837,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
      * {@link #setHandshakeFailure(ChannelHandlerContext, Throwable)}.
      * @return {@code true} if this method ends on {@link SSLEngineResult.HandshakeStatus#NOT_HANDSHAKING}.
      */
-    private void wrapNonAppData(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException {
+    private boolean wrapNonAppData(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException {
         ByteBuf out = null;
         ByteBufAllocator alloc = ctx.alloc();
         try {
@@ -863,7 +863,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 switch (result.getHandshakeStatus()) {
                     case FINISHED:
                         setHandshakeSuccess();
-                        break;
+                        return false;
                     case NEED_TASK:
                         runDelegatedTasks();
                         break;
@@ -872,7 +872,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                             // If we asked for a wrap, the engine requested an unwrap, and we are in unwrap there is
                             // no use in trying to call wrap again because we have already attempted (or will after we
                             // return) to feed more data to the engine.
-                            return;
+                            return false;
                         }
 
                         unwrapNonAppData(ctx);
@@ -886,7 +886,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                         if (!inUnwrap) {
                             unwrapNonAppData(ctx);
                         }
-                        break;
+                        return true;
                     default:
                         throw new IllegalStateException("Unknown handshake status: " + result.getHandshakeStatus());
                 }
@@ -906,6 +906,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 out.release();
             }
         }
+        return false;
     }
 
     private SSLEngineResult wrap(ByteBufAllocator alloc, SSLEngine engine, ByteBuf in, ByteBuf out)
@@ -1210,7 +1211,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
         try {
             // Only continue to loop if the handler was not removed in the meantime.
             // 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 Status status = result.getStatus();
                 final HandshakeStatus handshakeStatus = result.getHandshakeStatus();
@@ -1260,7 +1261,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                     case NEED_UNWRAP:
                         break;
                     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;
                     case NEED_TASK:
                         runDelegatedTasks();
@@ -1293,7 +1299,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                             flushedBeforeHandshake = false;
                             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;
                     default:
                         throw new IllegalStateException("unknown handshake status: " + handshakeStatus);