[GitHub] keith-turner commented on a change in pull request #786: fixes #785 avoid using system classloader in client rfile code

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #786: fixes #785 avoid using system classloader in client rfile code
URL: https://github.com/apache/accumulo/pull/786#discussion_r237598920
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/crypto/CryptoServiceFactory.java
 ##########
 @@ -25,11 +26,38 @@
 
   /**
    * Create a new crypto service configured in {@link Property#INSTANCE_CRYPTO_SERVICE}.
+   *
+   * @param useAccumuloClassloader
+   *          Determine if Accumulo class loader is used. When calling from accumulo server code,
+   *          pass true. When calling from Accumulo client code, pass false.
    */
-  public static CryptoService newInstance(AccumuloConfiguration conf) {
-    CryptoService newCryptoService = Property.createInstanceFromPropertyName(conf,
-        Property.INSTANCE_CRYPTO_SERVICE, CryptoService.class, new NoCryptoService());
+  public static CryptoService newInstance(AccumuloConfiguration conf,
+      boolean useAccumuloClassloader) {
+
+    CryptoService newCryptoService;
+
+    if (useAccumuloClassloader) {
+      newCryptoService = Property.createInstanceFromPropertyName(conf,
+          Property.INSTANCE_CRYPTO_SERVICE, CryptoService.class, new NoCryptoService());
+    } else {
+      String clazzName = conf.get(Property.INSTANCE_CRYPTO_SERVICE);
+      if (clazzName == null || clazzName.trim().isEmpty()) {
+        newCryptoService = new NoCryptoService();
+      } else {
+        try {
+          newCryptoService = CryptoServiceFactory.class.getClassLoader().loadClass(clazzName)
+              .asSubclass(CryptoService.class).newInstance();
+        } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
+          throw new RuntimeException(e);
+        }
+      }
+    }
+
     newCryptoService.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX));
     return newCryptoService;
   }
+
+  public static CryptoService newDefaultInstance() {
+    return newInstance(DefaultConfiguration.getInstance(), false);
+  }
 
 Review comment:
   I have not seen a situation in the test where this would be needed.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services