Drop NettySslPackageAccessor, close AsyncHttpClient#1382

Motivation:

* mess up with provided SslContext
* crash when netty and ahc are in different classloaders

Modifications:

* Drop NettySslPackageAccessor
* Revert to empty conf to use Netty default behavior (only Netty
recommended ciphers are available by default)

Result:

Don’t override provided SslContext behavior.
No more crash when netty and ahc sits in different classloaders

Author: slandelle

client/src/main/java/io/netty/handler/ssl/NettySslPackageAccessor.java

@@ -12,10 +12,6 @@
  */
 package org.asynchttpclient.config;
 
-import io.netty.handler.ssl.NettySslPackageAccessor;
-
-import java.util.Arrays;
-import java.util.Set;
 
 public final class AsyncHttpClientConfigDefaults {
 
@@ -81,9 +77,7 @@ public static String[] defaultEnabledProtocols() {
     }
 
     public static String[] defaultEnabledCipherSuites() {
-        String[] defaultEnabledCipherSuites = AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getStringArray(ASYNC_CLIENT_CONFIG_ROOT + "enabledCipherSuites");
-        Set<String> supportedCipherSuites = NettySslPackageAccessor.jdkSupportedCipherSuites();
-        return Arrays.stream(defaultEnabledCipherSuites).filter(supportedCipherSuites::contains).toArray(String[]::new);
+        return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getStringArray(ASYNC_CLIENT_CONFIG_ROOT + "enabledCipherSuites");
     }
 
     public static boolean defaultUseProxySelector() {

client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java

@@ -94,6 +94,10 @@ public String getString(String key) {
 
         public String[] getStringArray(String key) {
             String s = getString(key);
+            s = s.trim();
+            if (s.isEmpty()) {
+                return null;
+            }
             String[] rawArray = s.split(",");
             String[] array = new String[rawArray.length];
             for (int i = 0; i < rawArray.length; i++)

client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigHelper.java

@@ -13,11 +13,15 @@
  */
 package org.asynchttpclient.netty.ssl;
 
+import static org.asynchttpclient.util.MiscUtils.*;
+
 import io.netty.buffer.ByteBufAllocator;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
 import io.netty.handler.ssl.SslProvider;
 
+import java.util.Arrays;
+
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
 
@@ -28,16 +32,26 @@ public class DefaultSslEngineFactory extends SslEngineFactoryBase {
     private volatile SslContext sslContext;
 
     private SslContext buildSslContext(AsyncHttpClientConfig config) throws SSLException {
-        if (config.getSslContext() != null)
+        if (config.getSslContext() != null) {
             return config.getSslContext();
+        }
 
         SslContextBuilder sslContextBuilder = SslContextBuilder.forClient()//
                 .sslProvider(config.isUseOpenSsl() ? SslProvider.OPENSSL : SslProvider.JDK)//
                 .sessionCacheSize(config.getSslSessionCacheSize())//
                 .sessionTimeout(config.getSslSessionTimeout());
 
-        if (config.isAcceptAnyCertificate())
+        if (isNonEmpty(config.getEnabledProtocols())) {
+            sslContextBuilder.protocols(config.getEnabledProtocols());
+        }
+
+        if (isNonEmpty(config.getEnabledCipherSuites())) {
+            sslContextBuilder.ciphers(Arrays.asList(config.getEnabledCipherSuites()));
+        }
+
+        if (config.isAcceptAnyCertificate()) {
             sslContextBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE);
+        }
 
         return configureSslContextBuilder(sslContextBuilder).build();
     }
@@ -56,8 +70,8 @@ public void init(AsyncHttpClientConfig config) throws SSLException {
     }
 
     /**
-     * The last step of configuring the SslContextBuilder used to create an SslContext when no context is provided in
-     * the {@link AsyncHttpClientConfig}. This defaults to no-op and is intended to be overridden as needed.
+     * The last step of configuring the SslContextBuilder used to create an SslContext when no context is provided in the {@link AsyncHttpClientConfig}. This defaults to no-op and
+     * is intended to be overridden as needed.
      *
      * @param builder builder with normal configuration applied
      * @return builder to be used to build context (can be the same object as the input)

client/src/main/java/org/asynchttpclient/netty/ssl/DefaultSslEngineFactory.java

@@ -13,8 +13,6 @@
  */
 package org.asynchttpclient.netty.ssl;
 
-import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
-
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
 
@@ -28,11 +26,5 @@ protected void configureSslEngine(SSLEngine sslEngine, AsyncHttpClientConfig con
         SSLParameters params = sslEngine.getSSLParameters();
         params.setEndpointIdentificationAlgorithm("HTTPS");
         sslEngine.setSSLParameters(params);
-
-        if (isNonEmpty(config.getEnabledProtocols()))
-            sslEngine.setEnabledProtocols(config.getEnabledProtocols());
-
-        if (isNonEmpty(config.getEnabledCipherSuites()))
-            sslEngine.setEnabledCipherSuites(config.getEnabledCipherSuites());
     }
 }

client/src/main/java/org/asynchttpclient/netty/ssl/SslEngineFactoryBase.java

@@ -12,7 +12,7 @@ org.asynchttpclient.maxRedirects=5
 org.asynchttpclient.compressionEnforced=false
 org.asynchttpclient.userAgent=AHC/2.0
 org.asynchttpclient.enabledProtocols=TLSv1.2, TLSv1.1, TLSv1
-org.asynchttpclient.enabledCipherSuites=TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,TLS_RSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_CBC_SHA
+org.asynchttpclient.enabledCipherSuites=
 org.asynchttpclient.useProxySelector=false
 org.asynchttpclient.useProxyProperties=false
 org.asynchttpclient.validateResponseHeaders=true