Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/678#discussion_r228737596
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig config)
throws SSLContextException {
}
}
- public static X509KeyManager createKeyManager(String keyStoreLocation,
String keyStorePassword)
+ /**
+ * Creates a key manager by loading the key store from the given file
of the given type, optionally decrypting it
+ * using the given password.
+ * @param keyStoreLocation the location of the key store file.
+ * @param keyStorePassword optional password to decrypt the key store.
If empty, assumes the key store is not
+ * encrypted.
+ * @param keyStoreType must be JKS, PEM, or null. If null, attempts to
autodetect the key store type from the file
+ * extension (.jks / .pem).
+ * @return the key manager.
+ * @throws KeyManagerException if something goes wrong.
+ */
+ public static X509KeyManager createKeyManager(String keyStoreLocation,
String keyStorePassword, StoreFileType keyStoreType)
throws KeyManagerException {
FileInputStream inputStream = null;
+ if (keyStorePassword == null) {
+ keyStorePassword = "";
+ }
try {
char[] keyStorePasswordChars = keyStorePassword.toCharArray();
File keyStoreFile = new File(keyStoreLocation);
- KeyStore ks = KeyStore.getInstance("JKS");
- inputStream = new FileInputStream(keyStoreFile);
- ks.load(inputStream, keyStorePasswordChars);
+ if (keyStoreType == null) {
+ keyStoreType =
detectStoreFileTypeFromFileExtension(keyStoreFile);
+ }
+ KeyStore ks;
+ switch (keyStoreType) {
--- End diff --
Using switch cases to employ different implementations for the same purpose
is usually a code smell to me. What do you think of implementing an abstract
class or an interface to gather all keystore reader implementations and create
child classes for PEM and JKS?
In which case you can,
1. pass only the interface KeystoreLoader here,
2. avoid PemReader being implemented with all static methods,
3. easily mock the impl in unit tests
---
