nacx commented on a change in pull request #64: JCLOUDS-1520 - Reuse
SSLSocketFactory in JavaUrlHttpCommandExecutorSe…
URL: https://github.com/apache/jclouds/pull/64#discussion_r390260781
##########
File path:
core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java
##########
@@ -202,19 +206,38 @@ protected HttpURLConnection initConnection(HttpRequest
request) throws IOExcepti
HttpURLConnection connection = (HttpURLConnection)
url.openConnection(proxyForURI.apply(request.getEndpoint()));
if (connection instanceof HttpsURLConnection) {
HttpsURLConnection sslCon = (HttpsURLConnection) connection;
- if (utils.relaxHostname())
- sslCon.setHostnameVerifier(verifier);
- if (sslContextSupplier != null) {
- // used for providers which e.g. use certs for authentication
(like FGCP)
- // Provider provides SSLContext impl (which inits context with
key manager)
-
sslCon.setSSLSocketFactory(sslContextSupplier.get().getSocketFactory());
- } else if (utils.trustAllCerts()) {
-
sslCon.setSSLSocketFactory(untrustedSSLContextProvider.get().getSocketFactory());
- }
+ configureSslConnection(sslCon);
}
return connection;
}
+ private void configureSslConnection(HttpsURLConnection sslCon) {
+ if (utils.relaxHostname())
+ sslCon.setHostnameVerifier(verifier);
+
+ SSLSocketFactory sslSocketFactory = null;
+ if (sslContextSupplier != null) {
+ // used for providers which e.g. use certs for authentication (like
FGCP)
+ // Provider provides SSLContext impl (which inits context with key
manager)
+ setSslSocketFactoryReferenceIfAbsent(sslSocketFactoryReference,
sslContextSupplier);
+ sslSocketFactory = sslSocketFactoryReference.get();
+ } else if (utils.trustAllCerts()) {
+ setSslSocketFactoryReferenceIfAbsent(sslSocketFactoryReference,
untrustedSSLContextProvider);
+ sslSocketFactory = sslSocketFactoryReference.get();
+ }
+
+ if (sslSocketFactory != null) {
+ sslCon.setSSLSocketFactory(sslSocketFactory);
+ }
+ }
+
+ private void
setSslSocketFactoryReferenceIfAbsent(AtomicReference<SSLSocketFactory>
sslSocketFactoryReference,
+ Supplier<SSLContext>
sslContextSupplier) {
+ if (sslSocketFactoryReference.get() == null) {
+
sslSocketFactoryReference.set(sslContextSupplier.get().getSocketFactory());
+ }
Review comment:
Apart from the comment above, this is racy.
```suggestion
sslSocketFactoryReference.compareAndSet(null,
sslContextSupplier.get().getSocketFactory());
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
