From 94a64c09cd44e6f49a7345e9e9985a3f17c2ee2b Mon Sep 17 00:00:00 2001
From: Trustin Lee <t@motd.kr>
Date: Fri, 12 Dec 2014 11:47:52 +0900
Subject: [PATCH] Make sure to notify handshake success even if SSLEngine is
 closed

Related:

e9685ea45aebcb4f9dad0f3a1fc328a06b4932dd

Motivation:

SslHandler.unwrap() does not evaluate the handshake status of
SSLEngine.unwrap() when the status of SSLEngine.unwrap() is CLOSED.

It is not correct because the status does not reflect the state of the
handshake currently in progress, accoding to the API documentation of
SSLEngineResult.Status.

Also, sslCloseFuture can be notified earlier than handshake notification
because we call sslCloseFuture.trySuccess() before evaluating handshake
status.

Modifications:

- Notify sslCloseFuture after the unwrap loop is finished
- Add more assertions to SocketSslEchoTest

Result:

Potentially fix the regression caused by:
- e9685ea45aebcb4f9dad0f3a1fc328a06b4932dd
---
 .../src/main/java/io/netty/handler/ssl/SslHandler.java |  8 ++++++--
 .../testsuite/transport/socket/SocketSslEchoTest.java  | 10 ++++++----
 2 files changed, 12 insertions(+), 6 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 eb618e8714..9baa996989 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java
@@ -887,6 +887,7 @@ public class SslHandler extends ByteToMessageDecoder {
         }
 
         boolean wrapLater = false;
+        boolean notifyClosure = false;
         ByteBuf decodeOut = allocate(ctx, initialOutAppBufCapacity);
         try {
             for (;;) {
@@ -898,8 +899,7 @@ public class SslHandler extends ByteToMessageDecoder {
 
                 if (status == Status.CLOSED) {
                     // notify about the CLOSED state of the SSLEngine. See #137
-                    sslCloseFuture.trySuccess(ctx.channel());
-                    break;
+                    notifyClosure = true;
                 }
 
                 switch (handshakeStatus) {
@@ -941,6 +941,10 @@ public class SslHandler extends ByteToMessageDecoder {
             if (wrapLater) {
                 wrap(ctx, true);
             }
+
+            if (notifyClosure) {
+                sslCloseFuture.trySuccess(ctx.channel());
+            }
         } catch (SSLException e) {
             setHandshakeFailure(e);
             throw e;
diff --git a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java
index 6c944bc47a..b3a1d18eb5 100644
--- a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java
+++ b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java
@@ -290,8 +290,7 @@ public class SocketSslEchoTest extends AbstractSocketTest {
         }
 
         @Override
-        public void channelActive(ChannelHandlerContext ctx)
-                throws Exception {
+        public void channelActive(ChannelHandlerContext ctx) throws Exception {
             channel = ctx.channel();
         }
 
@@ -320,7 +319,10 @@ public class SocketSslEchoTest extends AbstractSocketTest {
                 counter > data.length / 2 && renegoFuture == null) {
 
                 SslHandler sslHandler = ctx.pipeline().get(SslHandler.class);
+
                 Future<Channel> hf = sslHandler.handshakeFuture();
+                assertThat(hf.isDone(), is(true));
+
                 renegoFuture = sslHandler.renegotiate();
                 assertThat(renegoFuture, is(not(sameInstance(hf))));
                 assertThat(renegoFuture, is(sameInstance(sslHandler.handshakeFuture())));
@@ -341,13 +343,13 @@ public class SocketSslEchoTest extends AbstractSocketTest {
         @Override
         public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
             if (evt instanceof SslHandshakeCompletionEvent) {
+                assertSame(SslHandshakeCompletionEvent.SUCCESS, evt);
                 negoCounter ++;
             }
         }
 
         @Override
-        public void exceptionCaught(ChannelHandlerContext ctx,
-                Throwable cause) throws Exception {
+        public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
             if (logger.isWarnEnabled()) {
                 logger.warn(
                         "Unexpected exception from the " +