eclipse-ee4j / eclipse-ee4j/jersey
Connection is not stable (wrong detection if SSL context is configured)
- 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
Assessment
This issue has not been assessed yet.