jersey
jersey copied to clipboard
#3293 Pre-load default ssl socket factory in HttpUrlConnector
@mpotociar @petrbouda this PR is a potential fix for #3293
From my understanding of the current code, the logic for using a custom SSL socket factory is:
- If the
HttpsUrlConnectionreturned by theConnectionFactoryalready has a customSSLSocketFactoryset on it, do nothing - Otherwise, set the socket factory on the connection to be the one provided from the
SSLContextin the client config.
Currently, the first check is done via:
if (HttpsURLConnection.getDefaultSSLSocketFactory() == suc.getSSLSocketFactory()) {
// indicates that the custom socket factory was not set
suc.setSSLSocketFactory(sslSocketFactory.get());
}
The issue with this code is that HttpsURLConnection.getDefaultSSLSocketFactory() is not thread safe. The implementation looks like:
public static SSLSocketFactory getDefaultSSLSocketFactory() {
if (defaultSSLSocketFactory == null) {
defaultSSLSocketFactory = (SSLSocketFactory)SSLSocketFactory.getDefault();
}
return defaultSSLSocketFactory;
}
If there are concurrent requests, one of them may end up creating a separate SSLSocketFactory that is not the default. The comparison check above fails and the request ends up using the default SSL context instead of the provided one.
The fix in this PR is to call HttpsURLConnection.getDefaultSSLSocketFactory() in a static initializer block for the class so that the default SSLContext is present when the comparison is made in HttpUrlConnector#secureConnector
I'm open to feedback on alternate approaches to resolving this.
@pavelbucek are you the correct person to review this?