arankin-irl commented on a change in pull request #728: ZOOKEEPER-3160: Custom
User SSLContext
URL: https://github.com/apache/zookeeper/pull/728#discussion_r248803778
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
##########
@@ -220,6 +224,24 @@ public int getSslHandshakeTimeoutMillis() {
}
public SSLContext createSSLContext(ZKConfig config) throws
SSLContextException {
+ if (config.getProperty(sslClientContextProperty) != null) {
+ LOG.debug("Loading SSLContext from property '" +
sslClientContextProperty + "'");
+ String sslClientContextClass =
config.getProperty(sslClientContextProperty);
+ try {
+ Class<?> sslContextClass =
Class.forName(sslClientContextClass);
+ ZKClientSSLContext sslContext = (ZKClientSSLContext)
sslContextClass.getConstructor().newInstance();
Review comment:
I can use the Supplier interface - I just wanted to double check before
updating the PR. Haven't really seen it implemented, but that's something
that's a bit unique to this use-case. It is just an interface at the end of the
day.
Renaming the property is grand as well - the name made sense back before
ClientX509Util for my specific use-case, but given the changes, renaming it
makes more sense.
Just checking - we get an unchecked cast warning when instantiating the
supplier and casting it to Supplier<SSLContext> - should I leave that warning,
or suppress that warning?
*Edit:* Looks like SuppressWarnings is used elsewhere, so I'll go that route.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
