From c9924cf797e4260c05ae1db7b36f11bc5abc5ce6 Mon Sep 17 00:00:00 2001 From: jansupol <15908245+jansupol@users.noreply.github.com> Date: Tue, 18 Jun 2024 12:43:24 +0200 Subject: [PATCH] Prevent blowing connections number for reoccurring SSLContextFatories (#5677) Signed-off-by: jansupol --- .../client/internal/HttpUrlConnector.java | 40 +++-- .../jersey/client/SSLSocketFactoryTest.java | 157 ++++++++++++++++++ 2 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 core-client/src/test/java/org/glassfish/jersey/client/SSLSocketFactoryTest.java diff --git a/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java b/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java index 443349631d..350ad2b2a7 100644 --- a/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java +++ b/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 DEFAULT_SSL_SOCKET_FACTORY = + private static final Value DEFAULT_SSL_SOCKET_FACTORY = Values.lazy((Value) () -> 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; + private Value sslSocketFactory; + + // SSLContext#getSocketFactory not idempotent + // JDK KeepAliveCache keeps connections per Factory + // SSLContext set per request blows that -> keep factory in LRU + private final LRU sslSocketFactoryCache = LRU.create(); private final ConnectorExtension connectorExtension = new HttpUrlExpect100ContinueConnectorExtension(); @@ -143,6 +149,13 @@ public HttpUrlConnector( this.fixLengthStreaming = fixLengthStreaming; this.setMethodWorkaround = setMethodWorkaround; + this.sslSocketFactory = Values.lazy(new Value() { + @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 supplier = request.resolveProperty(ClientProperties.SSL_CONTEXT_SUPPLIER, Supplier.class); - sslSocketFactory = Values.lazy(new Value() { - @Override - public SSLSocketFactory get() { - final SSLContext ctx = supplier == null ? client.getSslContext() : supplier.get(); - return ctx.getSocketFactory(); - } - }); + if (supplier != null) { + sslSocketFactory = Values.lazy(new Value() { // 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 { diff --git a/core-client/src/test/java/org/glassfish/jersey/client/SSLSocketFactoryTest.java b/core-client/src/test/java/org/glassfish/jersey/client/SSLSocketFactoryTest.java new file mode 100644 index 0000000000..49110fd8af --- /dev/null +++ b/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 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 { + SSLSocketFactory firstRequestFactory, secondRequestFactory = null; + 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); + + // First Request + connector.setSslContextFactory(client, new ClientRequest(url.toURI(), + (ClientConfig) client.getConfiguration(), propertiesDelegate)); + connector.secureConnection((JerseyClient) client, (HttpURLConnection) urlConnection); + firstRequestFactory = factoryHolder.get(); + + // 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(); + + 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); + } + } +}