nacx commented on a change in pull request #64:
URL: https://github.com/apache/jclouds/pull/64#discussion_r412727800
##########
File path: core/src/main/java/org/jclouds/http/config/SSLModule.java
##########
@@ -43,8 +44,8 @@
@Override
protected void configure() {
bind(HostnameVerifier.class).annotatedWith(Names.named("untrusted")).to(LogToMapHostnameVerifier.class);
- bind(new TypeLiteral<Supplier<SSLContext>>() {
- }).annotatedWith(Names.named("untrusted")).to(new
TypeLiteral<UntrustedSSLContextSupplier>() {
+ bind(new TypeLiteral<Supplier<SSLSocketFactory>>() {
+ }).annotatedWith(Names.named("untrusted")).to(new
TypeLiteral<UntrustedSSLSocketFactorySupplier>() {
Review comment:
I'm a bit concerned about this change. There are still uses of the
SSLContext injection. For example, take a look at the AapcheHC or the OkHttp
drivers. Those would need to be updated too.
I can also imagine that there can also be users that are providing a custom
version of the SSLContext, to customize how it behaves, and I think we should
try not to break them. Is there any way you can introduce your changes by
keeping the old supplier? Would it be possible to have both, the old one and
your SSLSocketFactory new one, and let your supplier use the injected
SSLContext one? This way we keep backward-compatibility and the only thing we
care about is our codebase using the factory supplier instead of directly
getting it from the context.
----------------------------------------------------------------
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:
us...@infra.apache.org
