Prevent blowing connections number for reoccurring SSLContentFatory i…

…nstances

Signed-off-by: jansupol <[email protected]>

core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java

@@ -66,6 +66,7 @@
 import org.glassfish.jersey.client.spi.AsyncConnectorCallback;
 import org.glassfish.jersey.client.spi.Connector;
 import org.glassfish.jersey.internal.util.PropertiesHelper;
+import org.glassfish.jersey.internal.util.collection.LRU;
 import org.glassfish.jersey.internal.util.collection.LazyValue;
 import org.glassfish.jersey.internal.util.collection.UnsafeValue;
 import org.glassfish.jersey.internal.util.collection.Value;
@@ -83,7 +84,7 @@ public class HttpUrlConnector implements Connector {
     private static final String ALLOW_RESTRICTED_HEADERS_SYSTEM_PROPERTY = "sun.net.http.allowRestrictedHeaders";
     // Avoid multi-thread uses of HttpsURLConnection.getDefaultSSLSocketFactory() because it does not implement a
     // proper lazy-initialization. See https://github.com/jersey/jersey/issues/3293
-    private static final LazyValue<SSLSocketFactory> DEFAULT_SSL_SOCKET_FACTORY =
+    private static final Value<SSLSocketFactory> DEFAULT_SSL_SOCKET_FACTORY =
             Values.lazy((Value<SSLSocketFactory>) () -> HttpsURLConnection.getDefaultSSLSocketFactory());
     // The list of restricted headers is extracted from sun.net.www.protocol.http.HttpURLConnection
     private static final String[] restrictedHeaders = {
@@ -114,7 +115,12 @@ public class HttpUrlConnector implements Connector {
     private final boolean fixLengthStreaming;
     private final boolean setMethodWorkaround;
     private final boolean isRestrictedHeaderPropertySet;
-    private LazyValue<SSLSocketFactory> sslSocketFactory;
+    private Value<SSLSocketFactory> sslSocketFactory;
+
+    // SSLContext#getSocketFactory not idempotent
+    // JDK KeepAliveCache keeps connections per Factory
+    // SSLContext set per request blows that -> keep factory in LRU
+    private final LRU<SSLContext, SSLSocketFactory> sslSocketFactoryCache = LRU.create();
 
     private final ConnectorExtension<HttpURLConnection, IOException> connectorExtension
             = new HttpUrlExpect100ContinueConnectorExtension();
@@ -143,6 +149,13 @@ public HttpUrlConnector(
         this.fixLengthStreaming = fixLengthStreaming;
         this.setMethodWorkaround = setMethodWorkaround;
 
+        this.sslSocketFactory = Values.lazy(new Value<SSLSocketFactory>() {
+            @Override
+            public SSLSocketFactory get() {
+                return client.getSslContext().getSocketFactory();
+            }
+        });
+
         // check if sun.net.http.allowRestrictedHeaders system property has been set and log the result
         // the property is being cached in the HttpURLConnection, so this is only informative - there might
         // already be some connection(s), that existed before the property was set/changed.
@@ -342,16 +355,23 @@ private void secureConnection(
         }
     }
 
-    private void setSslContextFactory(Client client, ClientRequest request) {
+    protected void setSslContextFactory(Client client, ClientRequest request) {
         final Supplier<SSLContext> supplier = request.resolveProperty(ClientProperties.SSL_CONTEXT_SUPPLIER, Supplier.class);
 
-        sslSocketFactory = Values.lazy(new Value<SSLSocketFactory>() {
-            @Override
-            public SSLSocketFactory get() {
-                final SSLContext ctx = supplier == null ? client.getSslContext() : supplier.get();
-                return ctx.getSocketFactory();
-            }
-        });
+        if (supplier != null) {
+            sslSocketFactory = Values.lazy(new Value<SSLSocketFactory>() { // lazy for double-check locking if multiple requests
+                @Override
+                public SSLSocketFactory get() {
+                    SSLContext sslContext = supplier.get();
+                    SSLSocketFactory factory = sslSocketFactoryCache.getIfPresent(sslContext);
+                    if (factory == null) {
+                        factory = sslContext.getSocketFactory();
+                        sslSocketFactoryCache.put(sslContext, factory);
+                    }
+                    return factory;
+                }
+            });
+        }
     }
 
     private ClientResponse _apply(final ClientRequest request) throws IOException {

core-client/src/test/java/org/glassfish/jersey/client/SSLSocketFactoryTest.java

@@ -0,0 +1,157 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
+package org.glassfish.jersey.client;
+
+import org.glassfish.jersey.client.internal.HttpUrlConnector;
+import org.glassfish.jersey.client.spi.Connector;
+import org.glassfish.jersey.internal.MapPropertiesDelegate;
+import org.glassfish.jersey.internal.PropertiesDelegate;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Test;
+
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocketFactory;
+import javax.ws.rs.client.Client;
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.core.Response;
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.URLConnection;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+
+public class SSLSocketFactoryTest {
+    static final AtomicReference<SSLSocketFactory> factoryHolder = new AtomicReference<>();
+    static SSLSocketFactory defaultSocketFactory = HttpsURLConnection.getDefaultSSLSocketFactory();
+
+    // @Test
+    // Alternative test
+    // Check KeepAliveCache#get(URL url, Object obj)
+    public void testSingleConnection() throws InterruptedException, IOException {
+        Client client = ClientBuilder.newClient();
+
+        for (int i = 0; i < 3; i++) {
+            try (Response response = client.target("https://www.spiegel.de")
+                    .request()
+                    .get()) {
+
+                response.readEntity(String.class);
+                System.out.println(String.format("response = %s", response));
+                Thread.sleep(1000);
+            }
+        }
+
+        System.in.read();
+    }
+
+    @Test
+    public void testSslContextFactoryOnClientIsSameForConsecutiveRequests() throws IOException, URISyntaxException {
+        int firstRequestFactory, secondRequestFactory = 0;
+        Client client = ClientBuilder.newClient();
+        HttpUrlConnectorProvider.ConnectionFactory connectionFactory = (url) -> (HttpURLConnection) url.openConnection();
+        SSLSocketFactoryConnector connector = (SSLSocketFactoryConnector) new SSlSocketFactoryUrlConnectorProvider()
+                .createHttpUrlConnector(client, connectionFactory, 4096, true, false);
+        URL url = new URL("https://somewhere.whereever:8080");
+        URLConnection urlConnection = url.openConnection();
+
+        // First Request
+        connector.setSslContextFactory(client, new ClientRequest(url.toURI(),
+                (ClientConfig) client.getConfiguration(), new MapPropertiesDelegate()));
+        connector.secureConnection((JerseyClient) client, (HttpURLConnection) urlConnection);
+        firstRequestFactory = factoryHolder.get().hashCode();
+
+        // reset to the default socketFactory
+        ((HttpsURLConnection) urlConnection).setSSLSocketFactory(defaultSocketFactory);
+
+        // Second Request
+        connector.setSslContextFactory(client, new ClientRequest(url.toURI(),
+                (ClientConfig) client.getConfiguration(), new MapPropertiesDelegate()));
+        connector.secureConnection((JerseyClient) client, (HttpURLConnection) urlConnection);
+        secondRequestFactory = factoryHolder.get().hashCode();
+
+        MatcherAssert.assertThat(firstRequestFactory, Matchers.equalTo(secondRequestFactory));
+    }
+
+    @Test
+    public void testSslContextFactoryOnRequestIsSameForConsecutiveRequests() throws IOException, URISyntaxException {
+        int firstRequestFactory, secondRequestFactory = 0;
+        Client client = ClientBuilder.newClient();
+        SSLContext sslContext = new SslContextClientBuilder().build();
+        HttpUrlConnectorProvider.ConnectionFactory connectionFactory = (url) -> (HttpURLConnection) url.openConnection();
+        SSLSocketFactoryConnector connector = (SSLSocketFactoryConnector) new SSlSocketFactoryUrlConnectorProvider()
+                .createHttpUrlConnector(client, connectionFactory, 4096, true, false);
+        URL url = new URL("https://somewhere.whereever:8080");
+        URLConnection urlConnection = url.openConnection();
+        PropertiesDelegate propertiesDelegate = new MapPropertiesDelegate();
+        propertiesDelegate.setProperty(ClientProperties.SSL_CONTEXT_SUPPLIER, (Supplier<SSLContext>) () -> sslContext);
+
+        // First Request
+        connector.setSslContextFactory(client, new ClientRequest(url.toURI(),
+                (ClientConfig) client.getConfiguration(), propertiesDelegate));
+        connector.secureConnection((JerseyClient) client, (HttpURLConnection) urlConnection);
+        firstRequestFactory = factoryHolder.get().hashCode();
+
+        // reset to the default socketFactory
+        ((HttpsURLConnection) urlConnection).setSSLSocketFactory(defaultSocketFactory);
+
+        // Second Request
+        connector.setSslContextFactory(client, new ClientRequest(url.toURI(),
+                (ClientConfig) client.getConfiguration(), propertiesDelegate));
+        connector.secureConnection((JerseyClient) client, (HttpURLConnection) urlConnection);
+        secondRequestFactory = factoryHolder.get().hashCode();
+
+        MatcherAssert.assertThat(firstRequestFactory, Matchers.equalTo(secondRequestFactory));
+    }
+
+    private static class SSLSocketFactoryConnector extends HttpUrlConnector {
+        public SSLSocketFactoryConnector(Client client, HttpUrlConnectorProvider.ConnectionFactory connectionFactory,
+                                         int chunkSize, boolean fixLengthStreaming, boolean setMethodWorkaround) {
+            super(client, connectionFactory, chunkSize, fixLengthStreaming, setMethodWorkaround);
+        }
+
+        @Override
+        protected void secureConnection(JerseyClient client, HttpURLConnection uc) {
+            super.secureConnection(client, uc);
+            if (HttpsURLConnection.class.isInstance(uc)) {
+                SSLSocketFactory factory = ((HttpsURLConnection) uc).getSSLSocketFactory();
+                factoryHolder.set(factory);
+            }
+        }
+
+        @Override
+        protected void setSslContextFactory(Client client, ClientRequest request) {
+            super.setSslContextFactory(client, request);
+        }
+    }
+
+    private static class SSlSocketFactoryUrlConnectorProvider extends HttpUrlConnectorProvider {
+        @Override
+        protected Connector createHttpUrlConnector(Client client, ConnectionFactory connectionFactory, int chunkSize,
+                                                   boolean fixLengthStreaming, boolean setMethodWorkaround) {
+            return new SSLSocketFactoryConnector(
+                    client,
+                    connectionFactory,
+                    chunkSize,
+                    fixLengthStreaming,
+                    setMethodWorkaround);
+        }
+    }
+}