Fail the build if we can't load the OpenSSL library (netty#11269)

Motivation: We should better fail the build if we can't load the OpenSSL library to ensure we not introduce a regression at some point related to native library loading Modifications: Remove usages of assumeTrue and let the tests fail if we cant load the native lib Result: Ensure we not regress
shellofyou · May 19, 2021 · 08dbd72 · 08dbd72
1 parent 1a67c1f
commit 08dbd72
Show file tree

Hide file tree

Showing 17 changed files with 55 additions and 42 deletions.
diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml
@@ -25,11 +25,11 @@ services:
 
   build-leak:
     <<: *common
-    command: /bin/bash -cl "./mvnw -Pleak clean install -Dio.netty.testsuite.badHost=netty.io"
+    command: /bin/bash -cl "./mvnw -Pleak clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora"
 
   build:
     <<: *common
-    command: /bin/bash -cl "./mvnw clean install -Dio.netty.testsuite.badHost=netty.io"
+    command: /bin/bash -cl "./mvnw clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora"
 
   deploy:
     <<: *common
@@ -52,7 +52,7 @@ services:
       - ~/.m2:/root/.m2
       - ~/local-staging:/root/local-staging
       - ..:/code
-    command: /bin/bash -cl "cat <(echo -e \"${GPG_PRIVATE_KEY}\") | gpg --batch --import && ./mvnw clean javadoc:jar package gpg:sign org.sonatype.plugins:nexus-staging-maven-plugin:deploy -DnexusUrl=https://oss.sonatype.org -DserverId=sonatype-nexus-staging -DaltStagingDirectory=/root/local-staging -DskipRemoteStaging=true -DskipTests=true -Dgpg.passphrase=${GPG_PASSPHRASE} -Dgpg.keyname=${GPG_KEYNAME}"
+    command: /bin/bash -cl "cat <(echo -e \"${GPG_PRIVATE_KEY}\") | gpg --batch --import && ./mvnw clean javadoc:jar package gpg:sign org.sonatype.plugins:nexus-staging-maven-plugin:deploy -DnexusUrl=https://oss.sonatype.org -DserverId=sonatype-nexus-staging -DaltStagingDirectory=/root/local-staging -DskipRemoteStaging=true -DskipTests=true -Dgpg.passphrase=${GPG_PASSPHRASE} -Dgpg.keyname=${GPG_KEYNAME} -Dtcnative.classifier=linux-x86_64-fedora"
 
   build-boringssl-static:
     <<: *common

diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java
@@ -58,7 +58,7 @@ public ConscryptOpenSslEngineInteropTest(BufferType type, ProtocolCipherCombo co
 
     @BeforeClass
     public static void checkOpenssl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java
@@ -63,7 +63,7 @@ public JdkOpenSslEngineInteroptTest(BufferType type, ProtocolCipherCombo protoco
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java
@@ -17,16 +17,20 @@
 
 import io.netty.internal.tcnative.CertificateVerifier;
 import org.junit.Assert;
-import org.junit.Assume;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.lang.reflect.Field;
 
 public class OpenSslCertificateExceptionTest {
 
+    @BeforeClass
+    public static void ensureOpenSsl() {
+        OpenSsl.ensureAvailability();
+    }
+
     @Test
     public void testValidErrorCode() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
         Field[] fields = CertificateVerifier.class.getFields();
         for (Field field : fields) {
             if (field.isAccessible()) {
@@ -39,13 +43,11 @@ public void testValidErrorCode() throws Exception {
 
     @Test(expected = IllegalArgumentException.class)
     public void testNonValidErrorCode() {
-        Assume.assumeTrue(OpenSsl.isAvailable());
         new OpenSslCertificateException(Integer.MIN_VALUE);
     }
 
     @Test
     public void testCanBeInstancedWhenOpenSslIsNotAvailable() {
-        Assume.assumeFalse(OpenSsl.isAvailable());
         new OpenSslCertificateException(0);
     }
 }
diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java
@@ -21,13 +21,11 @@
 import javax.net.ssl.SSLException;
 import java.io.File;
 
-import static org.junit.Assume.assumeTrue;
-
 public class OpenSslClientContextTest extends SslContextTest  {
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java
@@ -57,7 +57,7 @@ public OpenSslConscryptSslEngineInteropTest(BufferType type, ProtocolCipherCombo
 
     @BeforeClass
     public static void checkOpenssl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
@@ -110,7 +110,7 @@ public OpenSslEngineTest(BufferType type, ProtocolCipherCombo cipherCombo, boole
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+       OpenSsl.ensureAvailability();
     }
 
     @Override
@@ -1321,6 +1321,7 @@ public void testExtractMasterkeyWorksCorrectly() throws Exception {
 
     @Test(expected = SSLException.class)
     public void testNoKeyFound() throws Exception {
+        checkShouldUseKeyManagerFactory();
         clientSslCtx = wrapContext(SslContextBuilder
                 .forClient()
                 .trustManager(InsecureTrustManagerFactory.INSTANCE)

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java
@@ -33,7 +33,7 @@ public class OpenSslKeyMaterialManagerTest {
 
     @Test
     public void testChooseClientAliasReturnsNull() throws SSLException {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
 
         X509ExtendedKeyManager keyManager = new X509ExtendedKeyManager() {
             @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java
@@ -42,7 +42,7 @@ public class OpenSslKeyMaterialProviderTest {
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     protected KeyManagerFactory newKeyManagerFactory() throws Exception {

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java
@@ -30,7 +30,7 @@ public class OpenSslRenegotiateTest extends RenegotiateTest {
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override

diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java
@@ -28,12 +28,11 @@ public class OpenSslServerContextTest extends SslContextTest {
 
     @BeforeClass
     public static void checkOpenSsl() {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
     }
 
     @Override
     protected SslContext newSslContext(File crtFile, File keyFile, String pass) throws SSLException {
-        Assume.assumeTrue(OpenSsl.isAvailable());
         return new OpenSslServerContext(crtFile, keyFile, pass);
     }
 }
diff --git a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java
@@ -45,7 +45,7 @@ public void testPemEncodedOpenSslRef() throws Exception {
     }
 
     private static void testPemEncoded(SslProvider provider) throws Exception {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         assumeFalse(OpenSsl.useKeyManagerFactory());
         PemPrivateKey pemKey;
         PemX509Certificate pemCert;

diff --git a/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java
@@ -48,7 +48,7 @@ protected void cleanupClientSslContext(SslContext ctx) {
 
     @Override
     protected void cleanupClientSslEngine(SSLEngine engine) {
-        ReferenceCountUtil.release(engine);
+        ReferenceCountUtil.release(unwrapEngine(engine));
     }
 
     @Override
@@ -58,7 +58,7 @@ protected void cleanupServerSslContext(SslContext ctx) {
 
     @Override
     protected void cleanupServerSslEngine(SSLEngine engine) {
-        ReferenceCountUtil.release(engine);
+        ReferenceCountUtil.release(unwrapEngine(engine));
     }
 
     @Test(expected = NullPointerException.class)

diff --git a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java
@@ -47,7 +47,7 @@ public void testClientContextFromFileJdk() throws Exception {
 
     @Test
     public void testClientContextFromFileOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testClientContextFromFile(SslProvider.OPENSSL);
     }
 
@@ -58,7 +58,7 @@ public void testClientContextJdk() throws Exception {
 
     @Test
     public void testClientContextOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testClientContext(SslProvider.OPENSSL);
     }
 
@@ -69,7 +69,7 @@ public void testKeyStoreTypeJdk() throws Exception {
 
     @Test
     public void testKeyStoreTypeOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testKeyStoreType(SslProvider.OPENSSL);
     }
 
@@ -80,7 +80,7 @@ public void testServerContextFromFileJdk() throws Exception {
 
     @Test
     public void testServerContextFromFileOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testServerContextFromFile(SslProvider.OPENSSL);
     }
 
@@ -91,7 +91,7 @@ public void testServerContextJdk() throws Exception {
 
     @Test
     public void testServerContextOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testServerContext(SslProvider.OPENSSL);
     }
 
@@ -102,7 +102,7 @@ public void testContextFromManagersJdk() throws Exception {
 
     @Test
     public void testContextFromManagersOpenssl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         Assume.assumeTrue(OpenSsl.useKeyManagerFactory());
         testContextFromManagers(SslProvider.OPENSSL);
     }
@@ -155,13 +155,13 @@ private static void testUnsupportedPrivateKeyFailsFast(boolean server) throws Ex
 
     @Test(expected = IllegalArgumentException.class)
     public void testInvalidCipherJdk() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testInvalidCipher(SslProvider.JDK);
     }
 
     @Test
     public void testInvalidCipherOpenSSL() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         try {
             // This may fail or not depending on the OpenSSL version used
             // See https://github.com/openssl/openssl/issues/7196

diff --git a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java
@@ -127,7 +127,7 @@ public SslErrorTest(SslProvider serverProvider, SslProvider clientProvider,
     public void testCorrectAlert() throws Exception {
         // As this only works correctly at the moment when OpenSslEngine is used on the server-side there is
         // no need to run it if there is no openssl is available at all.
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
 
         SelfSignedCertificate ssc = new SelfSignedCertificate();
 

diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
@@ -367,7 +367,7 @@ public void testIncompleteWriteDoesNotCompletePromisePrematurely() throws NoSuch
 
     @Test
     public void testReleaseSslEngine() throws Exception {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
 
         SelfSignedCertificate cert = new SelfSignedCertificate();
         try {
@@ -1136,7 +1136,7 @@ public void testSessionTicketsWithTLSv13AndNoKey() throws Throwable {
     }
 
     private static void testSessionTickets(SslProvider provider, String protocol, boolean withKey) throws Throwable {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         final SslContext sslClientCtx = SslContextBuilder.forClient()
                 .trustManager(InsecureTrustManagerFactory.INSTANCE)
                 .sslProvider(provider)
@@ -1418,13 +1418,13 @@ public void testHandshakeFailureCipherMissmatchTLSv13Jdk() throws Exception {
 
     @Test
     public void testHandshakeFailureCipherMissmatchTLSv12OpenSsl() throws Exception {
-        Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testHandshakeFailureCipherMissmatch(SslProvider.OPENSSL, false);
     }
 
     @Test
     public void testHandshakeFailureCipherMissmatchTLSv13OpenSsl() throws Exception {
-         Assume.assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         Assume.assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL));
         Assume.assumeFalse("BoringSSL does not support setting ciphers for TLSv1.3 explicit", OpenSsl.isBoringSSL());
         testHandshakeFailureCipherMissmatch(SslProvider.OPENSSL, true);
@@ -1537,7 +1537,7 @@ public void testHandshakeEventsTls12JDK() throws Exception {
 
     @Test
     public void testHandshakeEventsTls12Openssl() throws Exception {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         testHandshakeEvents(SslProvider.OPENSSL, SslUtils.PROTOCOL_TLS_V1_2);
     }
 
@@ -1549,7 +1549,7 @@ public void testHandshakeEventsTls13JDK() throws Exception {
 
     @Test
     public void testHandshakeEventsTls13Openssl() throws Exception {
-        assumeTrue(OpenSsl.isAvailable());
+        OpenSsl.ensureAvailability();
         assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL));
         testHandshakeEvents(SslProvider.OPENSSL, SslUtils.PROTOCOL_TLS_V1_3);
     }

diff --git a/...blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java b/...blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java
@@ -240,14 +240,25 @@ public void testHandshakeWithExecutorTLSv13() throws Exception {
     }
 
     @Test
-    public void testTrustManagerVerify() throws Exception {
-        testTrustManagerVerify("TLSv1.2");
+    public void testTrustManagerVerifyJDK() throws Exception {
+        testTrustManagerVerify(SslProvider.JDK, "TLSv1.2");
     }
 
     @Test
-    public void testTrustManagerVerifyTLSv13() throws Exception {
+    public void testTrustManagerVerifyTLSv13JDK() throws Exception {
         assumeTrue(SslProvider.isTlsv13Supported(SslProvider.JDK));
-        testTrustManagerVerify("TLSv1.3");
+        testTrustManagerVerify(SslProvider.JDK, "TLSv1.3");
+    }
+
+    @Test
+    public void testTrustManagerVerifyOpenSSL() throws Exception {
+        testTrustManagerVerify(SslProvider.OPENSSL, "TLSv1.2");
+    }
+
+    @Test
+    public void testTrustManagerVerifyTLSv13OpenSSL() throws Exception {
+        assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL));
+        testTrustManagerVerify(SslProvider.OPENSSL, "TLSv1.3");
     }
 
     @Test
@@ -378,9 +389,10 @@ protected void run() {
         }
     }
 
-    private static void testTrustManagerVerify(String tlsVersion) throws Exception {
+    private static void testTrustManagerVerify(SslProvider provider, String tlsVersion) throws Exception {
         final SslContext sslClientCtx =
                 SslContextBuilder.forClient()
+                                 .sslProvider(provider)
                                  .protocols(tlsVersion)
                                  .trustManager(ResourcesUtil.getFile(
                                          NettyBlockHoundIntegrationTest.class, "mutual_auth_ca.pem"))
@@ -392,6 +404,7 @@ private static void testTrustManagerVerify(String tlsVersion) throws Exception {
                                             ResourcesUtil.getFile(
                                                     NettyBlockHoundIntegrationTest.class, "localhost_server.key"),
                                             null)
+                                 .sslProvider(provider)
                                  .protocols(tlsVersion)
                                  .build();