[#5874] [#5971] Ensure SniHandlerTest.testServerNameParsing not fails with SslProvider.JDK

Motivation:

The SniHandlerTest.testServerNameParsing did fail when SslProvider.JDK was used as it the JDK SSLEngineImpl does not send an alert.

Modifications:

Ensure tests pass with JDK and OPENSSL ssl implementations.

Result:

SniHandlerTest will run with all SslProvider and not fail when SslProvider.JDK is used.

handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java

@@ -26,6 +26,8 @@ import static org.junit.Assume.assumeTrue;
 
 import java.io.File;
 import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -62,7 +64,10 @@ import io.netty.util.ReferenceCountUtil;
 import io.netty.util.ReferenceCounted;
 import io.netty.util.concurrent.Promise;
 import io.netty.util.internal.ObjectUtil;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
+@RunWith(Parameterized.class)
 public class SniHandlerTest {
 
     private static ApplicationProtocolConfig newApnConfig() {
@@ -75,37 +80,80 @@ public class SniHandlerTest {
                 "myprotocol");
     }
 
-    private static SslContext makeSslContext() throws Exception {
+    private static void assumeApnSupported(SslProvider provider) {
-        return makeSslContext(null);
+        switch (provider) {
+            case OPENSSL:
+            case OPENSSL_REFCNT:
+                assumeTrue(OpenSsl.isAlpnSupported());
+                break;
+            case JDK:
+                assumeTrue(JdkAlpnSslEngine.isAvailable());
+                break;
+            default:
+                throw new Error();
+        }
+    }
+
+    private static SslContext makeSslContext(SslProvider provider, boolean apn) throws Exception {
+        if (apn) {
+            assumeApnSupported(provider);
         }
 
-    private static SslContext makeSslContext(SslProvider provider) throws Exception {
         File keyFile = new File(SniHandlerTest.class.getResource("test_encrypted.pem").getFile());
         File crtFile = new File(SniHandlerTest.class.getResource("test.crt").getFile());
 
-        return SslContextBuilder.forServer(crtFile, keyFile, "12345")
+        SslContextBuilder sslCtxBuilder = SslContextBuilder.forServer(crtFile, keyFile, "12345")
-                .sslProvider(provider)
+                .sslProvider(provider);
-                .applicationProtocolConfig(newApnConfig()).build();
+        if (apn) {
+            sslCtxBuilder.applicationProtocolConfig(newApnConfig());
+        }
+        return sslCtxBuilder.build();
+    }
+
+    private static SslContext makeSslClientContext(SslProvider provider, boolean apn) throws Exception {
+        if (apn) {
+            assumeApnSupported(provider);
         }
 
-    private static SslContext makeSslClientContext() throws Exception {
         File crtFile = new File(SniHandlerTest.class.getResource("test.crt").getFile());
 
-        return SslContextBuilder.forClient().trustManager(crtFile).applicationProtocolConfig(newApnConfig()).build();
+        SslContextBuilder sslCtxBuilder = SslContextBuilder.forClient().trustManager(crtFile).sslProvider(provider);
+        if (apn) {
+            sslCtxBuilder.applicationProtocolConfig(newApnConfig());
+        }
+        return sslCtxBuilder.build();
+    }
+
+    @Parameterized.Parameters(name = "{index}: sslProvider={0}")
+    public static Iterable<? extends Object> data() {
+        List<SslProvider> params = new ArrayList<SslProvider>(3);
+        if (OpenSsl.isAvailable()) {
+            params.add(SslProvider.OPENSSL);
+            params.add(SslProvider.OPENSSL_REFCNT);
+        }
+        params.add(SslProvider.JDK);
+        return params;
+    }
+
+    private final SslProvider provider;
+
+    public SniHandlerTest(SslProvider provider) {
+        this.provider = provider;
     }
 
     @Test
     public void testServerNameParsing() throws Exception {
-        SslContext nettyContext = makeSslContext();
+        SslContext nettyContext = makeSslContext(provider, false);
-        SslContext leanContext = makeSslContext();
+        SslContext leanContext = makeSslContext(provider, false);
-        SslContext leanContext2 = makeSslContext();
+        SslContext leanContext2 = makeSslContext(provider, false);
 
+        try {
             DomainNameMapping<SslContext> mapping = new DomainNameMappingBuilder<SslContext>(nettyContext)
                     .add("*.netty.io", nettyContext)
                     // input with custom cases
                     .add("*.LEANCLOUD.CN", leanContext)
-                // a hostname conflict with previous one, since we are using order-sensitive config, the engine won't
+                    // a hostname conflict with previous one, since we are using order-sensitive config,
-                // be used with the handler.
+                    // the engine won't be used with the handler.
                     .add("chat4.leancloud.cn", leanContext2)
                     .build();
 
@@ -132,7 +180,11 @@ public class SniHandlerTest {
                 // expected
             }
 
-        assertThat(ch.finish(), is(true));
+            // Just call finish and not assert the return value. This is because OpenSSL correct produces an alert
+            // while the JDK SSLEngineImpl does not atm.
+            // See https://github.com/netty/netty/issues/5874
+            ch.finish();
+
             assertThat(handler.hostname(), is("chat4.leancloud.cn"));
             assertThat(handler.sslContext(), is(leanContext));
 
@@ -143,20 +195,24 @@ public class SniHandlerTest {
                 }
                 ReferenceCountUtil.release(msg);
             }
+        } finally {
+            releaseAll(leanContext, leanContext2, nettyContext);
+        }
     }
 
     @Test
     public void testFallbackToDefaultContext() throws Exception {
-        SslContext nettyContext = makeSslContext();
+        SslContext nettyContext = makeSslContext(provider, false);
-        SslContext leanContext = makeSslContext();
+        SslContext leanContext = makeSslContext(provider, false);
-        SslContext leanContext2 = makeSslContext();
+        SslContext leanContext2 = makeSslContext(provider, false);
 
+        try {
             DomainNameMapping<SslContext> mapping = new DomainNameMappingBuilder<SslContext>(nettyContext)
                     .add("*.netty.io", nettyContext)
                     // input with custom cases
                     .add("*.LEANCLOUD.CN", leanContext)
-                // a hostname conflict with previous one, since we are using order-sensitive config, the engine won't
+                    // a hostname conflict with previous one, since we are using order-sensitive config,
-                // be used with the handler.
+                    // the engine won't be used with the handler.
                     .add("chat4.leancloud.cn", leanContext2)
                     .build();
 
@@ -164,7 +220,7 @@ public class SniHandlerTest {
             EmbeddedChannel ch = new EmbeddedChannel(handler);
 
             // invalid
-        byte[] message = { 22, 3, 1, 0, 0 };
+            byte[] message = {22, 3, 1, 0, 0};
 
             try {
                 // Push the handshake message.
@@ -176,13 +232,17 @@ public class SniHandlerTest {
             assertThat(ch.finish(), is(false));
             assertThat(handler.hostname(), nullValue());
             assertThat(handler.sslContext(), is(nettyContext));
+        } finally {
+            releaseAll(leanContext, leanContext2, nettyContext);
+        }
     }
 
     @Test
     public void testSniWithApnHandler() throws Exception {
-        SslContext nettyContext = makeSslContext();
+        SslContext nettyContext = makeSslContext(provider, true);
-        SslContext sniContext = makeSslContext();
+        SslContext sniContext = makeSslContext(provider, true);
-        final SslContext clientContext = makeSslClientContext();
+        final SslContext clientContext = makeSslClientContext(provider, true);
+        try {
             final CountDownLatch serverApnDoneLatch = new CountDownLatch(1);
             final CountDownLatch clientApnDoneLatch = new CountDownLatch(1);
 
@@ -206,7 +266,7 @@ public class SniHandlerTest {
                         // Catch the notification event that APN has completed successfully.
                         p.addLast(new ApplicationProtocolNegotiationHandler("foo") {
                             @Override
-                        protected void configurePipeline(ChannelHandlerContext ctx, String protocol) throws Exception {
+                            protected void configurePipeline(ChannelHandlerContext ctx, String protocol) {
                                 serverApnDoneLatch.countDown();
                             }
                         });
@@ -224,7 +284,7 @@ public class SniHandlerTest {
                         // 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 {
+                            protected void configurePipeline(ChannelHandlerContext ctx, String protocol) {
                                 clientApnDoneLatch.countDown();
                             }
                         });
@@ -250,25 +310,29 @@ public class SniHandlerTest {
                 }
                 group.shutdownGracefully(0, 0, TimeUnit.MICROSECONDS);
             }
+        } finally {
+            releaseAll(clientContext, nettyContext, sniContext);
+        }
     }
 
     @Test(timeout = 10L * 1000L)
     public void testReplaceHandler() throws Exception {
-
+        switch (provider) {
-        assumeTrue(OpenSsl.isAvailable());
+            case OPENSSL:
-
+            case OPENSSL_REFCNT:
                 final String sniHost = "sni.netty.io";
                 LocalAddress address = new LocalAddress("testReplaceHandler-" + Math.random());
                 EventLoopGroup group = new DefaultEventLoopGroup(1);
                 Channel sc = null;
                 Channel cc = null;
+                SslContext sslContext = null;
 
                 SelfSignedCertificate cert = new SelfSignedCertificate();
 
                 try {
                     final SslContext sslServerContext = SslContextBuilder
                             .forServer(cert.key(), cert.cert())
-                    .sslProvider(SslProvider.OPENSSL)
+                            .sslProvider(provider)
                             .build();
 
                     final Mapping<String, SslContext> mapping = new Mapping<String, SslContext>() {
@@ -322,15 +386,16 @@ public class SniHandlerTest {
                     };
 
                     ServerBootstrap sb = new ServerBootstrap();
-            sc = sb.group(group).channel(LocalServerChannel.class).childHandler(new ChannelInitializer<Channel>() {
+                    sc = sb.group(group).channel(LocalServerChannel.class)
+                            .childHandler(new ChannelInitializer<Channel>() {
                         @Override
                         protected void initChannel(Channel ch) throws Exception {
                             ch.pipeline().addFirst(handler);
                         }
                     }).bind(address).syncUninterruptibly().channel();
 
-            SslContext sslContext = SslContextBuilder.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE)
+                    sslContext = SslContextBuilder.forClient().sslProvider(provider)
-                    .build();
+                            .trustManager(InsecureTrustManagerFactory.INSTANCE).build();
 
                     Bootstrap cb = new Bootstrap();
                     cc = cb.group(group).channel(LocalChannel.class).handler(new SslHandler(
@@ -358,10 +423,18 @@ public class SniHandlerTest {
                     if (sc != null) {
                         sc.close().syncUninterruptibly();
                     }
+                    if (sslContext != null) {
+                        ReferenceCountUtil.release(sslContext);
+                    }
                     group.shutdownGracefully();
 
                     cert.delete();
                 }
+            case JDK:
+                return;
+            default:
+                throw new Error();
+        }
     }
 
     /**
@@ -384,4 +457,10 @@ public class SniHandlerTest {
             ReferenceCountUtil.release(sslContext);
         }
     }
+
+    private static void releaseAll(SslContext... contexts) {
+        for (SslContext ctx: contexts) {
+            ReferenceCountUtil.release(ctx);
+        }
+    }
 }