Skip to content

Commit

Permalink
Fix custom SSLSocketFactory not being set because of an unsafe lazy-i…
Browse files Browse the repository at this point in the history
…nitialization in JDK (#4566)

* Avoid unsafe lazy-initialization for SSL sockets

This prevents calling HttpsURLConnection.getDefaultSSLSocketFactory() in
an unsafe manner due to the poorly implemented lazy-initialization on
the JDK.

When multiple threads call that method concurrently (calling
secureConnection()) the SSLSocketFactory is instantiated two times,
making one thread fail the check and overriding the custom socket
factory with the default one.

Also-by: Kevin Conaway <[email protected]>
Signed-off-by: Adrian Haasler García <[email protected]>
  • Loading branch information
ahaasler committed Dec 17, 2020
1 parent d264efa commit 3996617
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ public class HttpUrlConnector implements Connector {

private static final Logger LOGGER = Logger.getLogger(HttpUrlConnector.class.getName());
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 SSLSocketFactory DEFAULT_SSL_SOCKET_FACTORY = HttpsURLConnection.getDefaultSSLSocketFactory();
// The list of restricted headers is extracted from sun.net.www.protocol.http.HttpURLConnection
private static final String[] restrictedHeaders = {
"Access-Control-Request-Headers",
Expand Down Expand Up @@ -302,7 +305,7 @@ protected void secureConnection(final JerseyClient client, final HttpURLConnecti
suc.setHostnameVerifier(verifier);
}

if (HttpsURLConnection.getDefaultSSLSocketFactory() == suc.getSSLSocketFactory()) {
if (DEFAULT_SSL_SOCKET_FACTORY == suc.getSSLSocketFactory()) {
// indicates that the custom socket factory was not set
suc.setSSLSocketFactory(sslSocketFactory.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,18 @@
package org.glassfish.jersey.tests.e2e.client.connector.ssl;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.HttpURLConnection;
import java.net.InetAddress;
import java.net.Socket;
import java.net.URL;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
Expand Down Expand Up @@ -79,6 +87,61 @@ public HttpURLConnection getConnection(final URL url) throws IOException {
assertTrue(socketFactory.isVisited());
}

/**
* Test for https://github.com/jersey/jersey/issues/3293
*
* @author Kevin Conaway
*/
@Test
public void testConcurrentRequestsWithCustomSSLContext() throws Exception {
final SSLContext sslContext = getSslContext();

final Client client = ClientBuilder.newBuilder()
.sslContext(sslContext)
.register(HttpAuthenticationFeature.basic("user", "password"))
.register(LoggingFeature.class)
.build();

int numThreads = 5;
CyclicBarrier barrier = new CyclicBarrier(numThreads);
ExecutorService service = Executors.newFixedThreadPool(numThreads);
List<Exception> exceptions = new CopyOnWriteArrayList<>();

for (int i = 0; i < numThreads; i++) {
service.submit(() -> {
try {
barrier.await(1, TimeUnit.MINUTES);
for (int call = 0; call < 10; call++) {
final Response response = client.target(Server.BASE_URI).path("/").request().get();
assertEquals(200, response.getStatus());
}
} catch (Exception ex) {
exceptions.add(ex);
}
});
}

service.shutdown();

assertTrue(
service.awaitTermination(1, TimeUnit.MINUTES)
);

assertTrue(
toString(exceptions),
exceptions.isEmpty()
);
}

private String toString(List<Exception> exceptions) {
StringWriter writer = new StringWriter();
PrintWriter printWriter = new PrintWriter(writer);

exceptions.forEach(e -> e.printStackTrace(printWriter));

return writer.toString();
}

public static class CustomSSLSocketFactory extends SSLSocketFactory {

private boolean visited = false;
Expand Down

0 comments on commit 3996617

Please sign in to comment.