From 4506bbd27bf17aeb112478f483b0cc046b11d703 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Tue, 5 Apr 2016 10:47:57 -0700
Subject: [PATCH] fcbeebf6dff759a1e0cfe2e20fe5103e4c40075c unit test bug

Motivation:
fcbeebf6dff759a1e0cfe2e20fe5103e4c40075c introduced a unit test to verify ApplicationProtocolNegotiationHandler is compatible with SniHandler. However only the server attempts ALPN and verifies that it completes and the client doesn't verify the handshake is completed. This can lead to the client side SSL engine to prematurely close and throw an exception.

Modifications:
- The client should wait for the SSL handshake and ALPN to complete before the test exits.

Result:
SniHandlerTest.testSniWithApnHandler is more reliable.
---
 .../io/netty/handler/ssl/SniHandlerTest.java  | 22 +++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
index 0496e21e88..fd5a2a6c36 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
@@ -22,6 +22,7 @@ import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelInitializer;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.EventLoopGroup;
@@ -40,6 +41,7 @@ import java.net.InetSocketAddress;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
+import javax.net.ssl.HandshakeCompletedEvent;
 import javax.xml.bind.DatatypeConverter;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -165,7 +167,8 @@ public class SniHandlerTest {
         SslContext leanContext = makeSslContext();
         final SslContext clientContext = makeSslClientContext();
         SslContext wrappedLeanContext = null;
-        final CountDownLatch apnDoneLatch = new CountDownLatch(1);
+        final CountDownLatch serverApnDoneLatch = new CountDownLatch(1);
+        final CountDownLatch clientApnDoneLatch = new CountDownLatch(1);
 
         final DomainNameMapping<SslContext> mapping = new DomainMappingBuilder(nettyContext)
                                                          .add("*.netty.io", nettyContext)
@@ -191,11 +194,13 @@ public class SniHandlerTest {
                 @Override
                 protected void initChannel(Channel ch) throws Exception {
                     ChannelPipeline p = ch.pipeline();
+                    // Server side SNI.
                     p.addLast(new SniHandler(mapping));
+                    // Catch the notification event that APN has completed successfully.
                     p.addLast(new ApplicationProtocolNegotiationHandler("foo") {
                         @Override
                         protected void configurePipeline(ChannelHandlerContext ctx, String protocol) throws Exception {
-                            apnDoneLatch.countDown();
+                            serverApnDoneLatch.countDown();
                         }
                     });
                 }
@@ -207,10 +212,18 @@ public class SniHandlerTest {
             cb.handler(new ChannelInitializer<Channel>() {
                 @Override
                 protected void initChannel(Channel ch) throws Exception {
-                    ChannelPipeline p = ch.pipeline();
+                    // Simulate client side SNI (which is currently not supported).
                     ch.write(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex1)));
                     ch.writeAndFlush(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex)));
+                    // Add a SslHandler so we can do the client side of the handshake.
                     ch.pipeline().addLast(clientContext.newHandler(ch.alloc()));
+                    // Catch the notification event that APN has completed successfully.
+                    ch.pipeline().addLast(new ApplicationProtocolNegotiationHandler("foo") {
+                        @Override
+                        protected void configurePipeline(ChannelHandlerContext ctx, String protocol) throws Exception {
+                            clientApnDoneLatch.countDown();
+                        }
+                    });
                 }
             });
 
@@ -220,7 +233,8 @@ public class SniHandlerTest {
             assertTrue(ccf.awaitUninterruptibly().isSuccess());
             clientChannel = ccf.channel();
 
-            assertTrue(apnDoneLatch.await(5, TimeUnit.SECONDS));
+            assertTrue(serverApnDoneLatch.await(5, TimeUnit.SECONDS));
+            assertTrue(clientApnDoneLatch.await(5, TimeUnit.SECONDS));
         } finally {
             if (serverChannel != null) {
                 serverChannel.close().sync();