Fix tests after Apache HttpClient5 / HttpCore5 update since those use deprecated APIs

reta · reta · commit 6d16c0a3b9c2 · 2025-01-16T16:54:18.000-05:00
Signed-off-by: Andriy Redko &lt;andriy.redko@aiven.io&gt;
diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java b/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java
@@ -28,6 +28,7 @@
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
+import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -223,7 +224,7 @@ private CloseableHttpClient createHttpClient(HttpCacheStorage httpCacheStorage)
 
         if (sslConfig != null) {
             final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
-                .setSSLSocketFactory(sslConfig.toSSLConnectionSocketFactory())
+                .setTlsSocketStrategy(new DefaultClientTlsStrategy(sslConfig.getSslContext()))
                 .build();
 
             builder.setConnectionManager(cm);
diff --git a/src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java b/src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java
@@ -32,12 +32,13 @@
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
+import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
 import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
 import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
-import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
 import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.io.entity.StringEntity;
+import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
 import org.apache.hc.core5.ssl.SSLContextBuilder;
 import org.apache.hc.core5.ssl.TrustStrategy;
 import org.apache.http.HttpStatus;
@@ -380,15 +381,16 @@ public boolean isTrusted(X509Certificate[] chain, String authType) {
             HttpClientBuilder hcb = HttpClients.custom().setDefaultRequestConfig(config);
             if (!verifySSL) {
                 SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(trustAllStrategy).build();
-                final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(
+                final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
                     sslContext,
                     null,
                     null,
+                    SSLBufferMode.STATIC,
                     NoopHostnameVerifier.INSTANCE
                 );
 
                 final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
-                    .setSSLSocketFactory(sslsf)
+                    .setTlsSocketStrategy(sslsf)
                     .setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
                     .build();
                 hcb.setConnectionManager(cm);
@@ -399,10 +401,16 @@ public boolean isTrusted(X509Certificate[] chain, String authType) {
                 return HttpClients.custom().setDefaultRequestConfig(config).build();
             }
             SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(effectiveTruststore, null).build();
-            final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(sslContext, null, null, new DefaultHostnameVerifier());
+            final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
+                sslContext,
+                null,
+                null,
+                SSLBufferMode.STATIC,
+                new DefaultHostnameVerifier()
+            );
 
             final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
-                .setSSLSocketFactory(sslsf)
+                .setTlsSocketStrategy(sslsf)
                 .setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
                 .build();
             hcb.setConnectionManager(cm);
diff --git a/src/main/java/org/opensearch/security/httpclient/HttpClient.java b/src/main/java/org/opensearch/security/httpclient/HttpClient.java
@@ -30,23 +30,21 @@
 import java.util.stream.Collectors;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
-import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
 
 import com.google.common.collect.Lists;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
 import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
-import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
+import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
 import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
 import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
-import org.apache.hc.core5.function.Factory;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
-import org.apache.hc.core5.reactor.ssl.TlsDetails;
+import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
 import org.apache.hc.core5.ssl.PrivateKeyDetails;
 import org.apache.hc.core5.ssl.PrivateKeyStrategy;
 import org.apache.hc.core5.ssl.SSLContextBuilder;
@@ -280,19 +278,13 @@ public String chooseAlias(Map<String, PrivateKeyDetails> aliases, SSLParameters
             final HostnameVerifier hnv = verifyHostnames ? new DefaultHostnameVerifier() : NoopHostnameVerifier.INSTANCE;
 
             final SSLContext sslContext = sslContextBuilder.build();
-            TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
-                .setSslContext(sslContext)
-                .setTlsVersions(supportedProtocols)
-                .setCiphers(supportedCipherSuites)
-                .setHostnameVerifier(hnv)
-                // See please https://issues.apache.org/jira/browse/HTTPCLIENT-2219
-                .setTlsDetailsFactory(new Factory<SSLEngine, TlsDetails>() {
-                    @Override
-                    public TlsDetails create(final SSLEngine sslEngine) {
-                        return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol());
-                    }
-                })
-                .build();
+            final TlsStrategy tlsStrategy = new DefaultClientTlsStrategy(
+                sslContext,
+                supportedProtocols,
+                supportedCipherSuites,
+                SSLBufferMode.STATIC,
+                hnv
+            );
 
             final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create().setTlsStrategy(tlsStrategy).build();
             httpClientBuilder.setConnectionManager(cm);
diff --git a/src/test/java/org/opensearch/security/InitializationIntegrationTests.java b/src/test/java/org/opensearch/security/InitializationIntegrationTests.java
@@ -178,7 +178,7 @@ public void testWhoAmIForceHttp1() throws Exception {
             Response whoAmIRes = restHighLevelClient.getLowLevelClient().performRequest(new Request("GET", "/_plugins/_security/whoami"));
             assertThat(200, is(whoAmIRes.getStatusLine().getStatusCode()));
             // The HTTP/1.1 is forced and should be used instead
-            assertThat(HttpVersion.HTTP_1_1, is(whoAmIRes.getStatusLine().getProtocolVersion()));
+            assertThat(whoAmIRes.getStatusLine().getProtocolVersion(), is(HttpVersion.HTTP_1_1));
             JsonNode whoAmIResNode = DefaultObjectMapper.objectMapper.readTree(whoAmIRes.getEntity().getContent());
             String whoAmIResponsePayload = whoAmIResNode.toPrettyString();
             assertThat(whoAmIResponsePayload, whoAmIResNode.get("dn").asText(), is("CN=spock,OU=client,O=client,L=Test,C=DE"));
diff --git a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java
@@ -44,8 +44,8 @@
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
-import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
 import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
 import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
 import org.apache.hc.core5.function.Factory;
@@ -197,14 +197,13 @@ public TlsDetails create(final SSLEngine sslEngine) {
                     })
                     .build();
 
-                final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create()
-                    .setTlsStrategy(tlsStrategy)
-                    .build();
-                builder.setConnectionManager(cm);
+                final PoolingAsyncClientConnectionManagerBuilder cm = PoolingAsyncClientConnectionManagerBuilder.create()
+                    .setTlsStrategy(tlsStrategy);
+
                 if (httpVersionPolicy != null) {
-                    builder.setVersionPolicy(httpVersionPolicy);
+                    cm.setDefaultTlsConfig(TlsConfig.custom().setVersionPolicy(httpVersionPolicy).build());
                 }
-                return builder;
+                return builder.setConnectionManager(cm.build());
             });
             return new RestHighLevelClient(restClientBuilder);
         } catch (Exception e) {
diff --git a/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java b/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java
@@ -381,7 +381,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
             hcb.setConnectionManager(cm);
         }
 
-        final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom().setResponseTimeout(Timeout.ofSeconds(60));
+        final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()
+            .setResponseTimeout(Timeout.ofSeconds(60))
+            .setProtocolUpgradeEnabled(false);
 
         return hcb.setDefaultRequestConfig(requestConfigBuilder.build()).disableAutomaticRetries().build();
     }

Original file line number	Diff line number	Diff line change
`@@ -381,7 +381,9 @@ public TlsDetails create(final SSLEngine sslEngine) {`
`381`	`381`	`hcb.setConnectionManager(cm);`
`382`	`382`	`}`
`383`	`383`
`384`		`- final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom().setResponseTimeout(Timeout.ofSeconds(60));`
	`384`	`+ final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()`
	`385`	`+ .setResponseTimeout(Timeout.ofSeconds(60))`
	`386`	`+ .setProtocolUpgradeEnabled(false);`
`385`	`387`
`386`	`388`	`return hcb.setDefaultRequestConfig(requestConfigBuilder.build()).disableAutomaticRetries().build();`
`387`	`389`	`}`