[GitHub] ctubbsii commented on a change in pull request #371: ACCUMULO-4792 Improve crypto configuration checks

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

ctubbsii commented on a change in pull request #371: ACCUMULO-4792 Improve crypto configuration checks
URL: https://github.com/apache/accumulo/pull/371#discussion_r165356198
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
 ##########
 @@ -98,13 +110,21 @@ else if (!prop.getType().isValidFormat(value))
       log.warn("Use of {} and {} are deprecated. Consider using {} instead.", INSTANCE_DFS_URI, INSTANCE_DFS_DIR, Property.INSTANCE_VOLUMES);
     }
 
-    if (cipherSuite.equals("NullCipher") && !keyAlgorithm.equals("NullCipher")) {
+    if (cipherSuite.equals(NULL_CIPHER) && !keyAlgorithm.equals(NULL_CIPHER)) {
 
 Review comment:
   Would a single XOR-type check suffice to consolidate this and the following check? (with a reworded message, as appropriate):
   
   ```java
   (cipherSuite.equals(NULL_CIPHER) || keyAlgorithm.equals(NULL_CIPHER)) && !cipherSuite.equals(keyAlgorithm)
   ```

----------------------------------------------------------------
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