eclipse-ee4j / eclipse-ee4j/jersey

Connection is not stable (wrong detection if SSL context is configured)

Open
#5,637 5 comments 1 reaction 0 assignees View on GitHub
Dominant language
Java
Stars
730
Forks
382
PR merge metrics
No merged PRs in 30d

Description

The [class `org.glassfish.jersey.client.internal.HttpUrlConnector`](https://github.com/eclipse-ee4j/jersey/blame/4.0/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java`) contains this code.

```java
private static final LazyValue DEFAULT_SSL_SOCKET_FACTORY =
Values.lazy((Value) () -> HttpsURLConnection.getDefaultSSLSocketFactory());
```
https://github.com/eclipse-ee4j/jersey/blame/d377a30a033cb7468d66e9901ee832bea6cbd8db/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java#L86-L87

```java
if (DEFAULT_SSL_SOCKET_FACTORY.get() == suc.getSSLSocketFactory()) {
// indicates that the custom socket factory was not set
suc.setSSLSocketFactory(sslSocketFactory.get());
}
```
https://github.com/eclipse-ee4j/jersey/blame/d377a30a033cb7468d66e9901ee832bea6cbd8db/core-client/src/main/java/org/glassfish/jersey/client/internal/HttpUrlConnector.java#L316-L319

It has a relationship with the default constructor of class `javax.net.ssl.HttpsURLConnection`
```java
protected HttpsURLConnection(URL url) {
super(url);
this.hostnameVerifier = defaultHostnameVerifier;
this.sslSocketFactory = getDefaultSSLSocketFactory();
}
```

The idea of this code is to detect if the connection has set an extra SSL Context or if it was not defined. There is one potential issue, once some other code changes the default configuration (`HttpsURLConnection.getDefaultSSLSocketFactory()`) the condition stops working. The SSL context is never set and it uses the default one. It means once the user sets a context and then changes the default context for any reason the connection starts using the default one. Keep in mind, that it could also leverage the connection (ie. adding a client certificate to a connection could change behavior, trust store is changed, etc.).

The proper solution should be just to set the SSL context provided by a client (`HttpsURLConnection.getDefaultSSLSocketFactory()`). The current behavior does not make any sense. If the user configures an SSLContext, it should be used, otherwise, the default one is expected (it means without the settings).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.