Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent blowing connections number for reoccurring SSLContextFatories #5677

Merged
merged 2 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
Copy link
Member

@jbescos jbescos Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we compare factories instead of hashCode?. It is unlikely, but two different instances could have same hashCode.

}

@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);
}
}
}