Skip to content

Commit

Permalink
Fix possible concurrent issue with SSL & default connector
Browse files Browse the repository at this point in the history
Signed-off-by: jansupol <[email protected]>
  • Loading branch information
jansupol committed Nov 8, 2024
1 parent ff5f5a8 commit bdb97fc
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
import java.net.Proxy;
import java.net.URL;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Logger;

import javax.ws.rs.client.Client;
Expand Down Expand Up @@ -290,16 +287,12 @@ public interface ConnectionFactory {
* @throws java.io.IOException in case the connection cannot be provided.
*/
default HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
synchronized (this){
return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy);
}
return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy);
}
}

private static class DefaultConnectionFactory implements ConnectionFactory {

private final ConcurrentHashMap<URL, Lock> locks = new ConcurrentHashMap<>();

@Override
public HttpURLConnection getConnection(final URL url) throws IOException {
return connect(url, null);
Expand All @@ -311,13 +304,7 @@ public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException
}

private HttpURLConnection connect(URL url, Proxy proxy) throws IOException {
Lock lock = locks.computeIfAbsent(url, u -> new ReentrantLock());
lock.lock();
try {
return (proxy == null) ? (HttpURLConnection) url.openConnection() : (HttpURLConnection) url.openConnection(proxy);
} finally {
lock.unlock();
}
return (proxy == null) ? (HttpURLConnection) url.openConnection() : (HttpURLConnection) url.openConnection(proxy);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,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 Value<SSLSocketFactory> DEFAULT_SSL_SOCKET_FACTORY =
private static final LazyValue<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 @@ -387,6 +387,10 @@ private ClientResponse _apply(final ClientRequest request) throws IOException {
sniUri = request.getUri();
}

if (!DEFAULT_SSL_SOCKET_FACTORY.isInitialized() && "HTTPS".equalsIgnoreCase(sniUri.getScheme())) {
DEFAULT_SSL_SOCKET_FACTORY.get();
}

proxy.ifPresent(clientProxy -> ClientProxy.setBasicAuthorizationHeader(request.getHeaders(), proxy.get()));
uc = this.connectionFactory.getConnection(sniUri.toURL(), proxy.isPresent() ? proxy.get().proxy() : null);
uc.setDoInput(true);
Expand Down
18 changes: 18 additions & 0 deletions tests/e2e-tls/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,24 @@
</http.patch.addopens>
</properties>
</profile>
<profile>
<id>JDK_17-</id>
<activation>
<jdk>[1.8,17)</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<!-- The certificate is not working with JDK 11 -->
<excludes><exclude>**/ConcurrentHttpsUrlConnectionTest*</exclude></excludes>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* 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.tests.e2e.tls.connector;

import org.junit.jupiter.api.Test;

import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.Client;
import javax.ws.rs.core.GenericType;
import javax.ws.rs.core.MediaType;

import java.io.InputStream;
import java.net.URL;
import java.security.KeyStore;
import java.util.Random;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSession;
import javax.net.ssl.TrustManagerFactory;

/**
* Jersey client seems to be not thread-safe:
* When the first GET request is in progress,
* all parallel requests from other Jersey client instances fail
* with SSLHandshakeException: PKIX path building failed.
* <p>
* Once the first GET request is completed,
* all subsequent requests work without error.
* <p>
* BUG 5749
*/
public class ConcurrentHttpsUrlConnectionTest {
private static int THREAD_NUMBER = 2;

private static volatile int responseCounter = 0;

private static SSLContext createContext() throws Exception {
URL url = ConcurrentHttpsUrlConnectionTest.class.getResource("keystore.jks");
KeyStore keyStore = KeyStore.getInstance("JKS");
try (InputStream is = url.openStream()) {
keyStore.load(is, "password".toCharArray());
}
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
kmf.init(keyStore, "password".toCharArray());
TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
tmf.init(keyStore);
SSLContext context = SSLContext.getInstance("TLS");
context.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
return context;
}

@Test
public void testSSLConnections() throws Exception {
if (THREAD_NUMBER == 1) {
System.out.println("\nThis is the working case (THREAD_NUMBER==1). Set THREAD_NUMBER > 1 to reproduce the error! \n");
}

final HttpsServer server = new HttpsServer(createContext());
Executors.newFixedThreadPool(1).submit(server);

// set THREAD_NUMBER > 1 to reproduce an issue
ExecutorService executorService2clients = Executors.newFixedThreadPool(THREAD_NUMBER);

final ClientBuilder builder = ClientBuilder.newBuilder().sslContext(createContext())
.hostnameVerifier(new HostnameVerifier() {
public boolean verify(String arg0, SSLSession arg1) {
return true;
}
});

AtomicInteger counter = new AtomicInteger(0);

for (int i = 0; i < THREAD_NUMBER; i++) {
executorService2clients.submit(new Runnable() {
@Override
public void run() {
try {
Client client = builder.build();
String ret = client.target("https://127.0.0.1:" + server.getPort() + "/" + new Random().nextInt())
.request(MediaType.TEXT_HTML)
.get(new GenericType<String>() {
});
System.out.print(++responseCounter + ". Server returned: " + ret);
} catch (Exception e) {
//get an exception here, if jersey lib is buggy and THREAD_NUMBER > 1:
//jakarta.ws.rs.ProcessingException: javax.net.ssl.SSLHandshakeException: PKIX path building failed:
e.printStackTrace();
} finally {
System.out.println(counter.incrementAndGet());
}
}
});
}

while (counter.get() != THREAD_NUMBER) {
Thread.sleep(100L);
}
server.stop();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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.tests.e2e.tls.connector;

import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintWriter;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLSocket;

class HttpsServer implements Runnable {
private final SSLServerSocket sslServerSocket;
private boolean closed = false;

public HttpsServer(SSLContext context) throws Exception {
sslServerSocket = (SSLServerSocket) context.getServerSocketFactory().createServerSocket(0);
}

public int getPort() {
return sslServerSocket.getLocalPort();
}

@Override
public void run() {
System.out.printf("Server started on port %d%n", getPort());
while (!closed) {
SSLSocket s;
try {
s = (SSLSocket) sslServerSocket.accept();
} catch (IOException e2) {
s = null;
}
final SSLSocket socket = s;
new Thread(new Runnable() {
public void run() {
try {
if (socket != null) {
InputStream is = new BufferedInputStream(socket.getInputStream());
byte[] data = new byte[2048];
int len = is.read(data);
if (len <= 0) {
throw new IOException("no data received");
}
//System.out.printf("Server received: %s\n", new String(data, 0, len));
PrintWriter writer = new PrintWriter(socket.getOutputStream());
writer.println("HTTP/1.1 200 OK");
writer.println("Content-Type: text/html");
writer.println();
writer.println("Hello from server!");
writer.flush();
writer.close();
socket.close();
}
} catch (Exception e1) {
e1.printStackTrace();
}
}
}).start();
}
}

void stop() {
try {
closed = true;
sslServerSocket.close();
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}
Binary file not shown.

0 comments on commit bdb97fc

Please sign in to comment.