[GitHub] [accumulo] EdColeman commented on a change in pull request #1803: Fix #1303 Create unique dirs for sorted WALs

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

EdColeman commented on a change in pull request #1803:
URL: https://github.com/apache/accumulo/pull/1803#discussion_r568086238



##########
File path: server/base/src/main/java/org/apache/accumulo/server/master/recovery/RecoveryPath.java
##########
@@ -50,7 +47,8 @@ public static Path getRecoveryPath(Path walPath, String portHost) {
 
       // drop wal
       walPath = walPath.getParent();
-      walPath = new Path(walPath, FileType.RECOVERY.getDirectory() + "-" + convertToSha1(portHost));
+      walPath =
+          new Path(walPath, FileType.RECOVERY.getDirectory() + "-" + DigestUtils.sha1Hex(portHost));

Review comment:
       Recently it was noted that using sha1 can be flagged by some code security scan utilities - would using sha256 or even sha512 eliminate those utilities from flagging "unsecure usages"?  This is not a security related hash so it does not matter, but those utilities don't take that into account.
   
   The length of the string could also be shortened using something like:
   
   ```String s = new BigInteger(DigestUtils.sha256(portHost).toString(MAX_RADIX).substring(0,40)```
   
   This would have the same number of characters as the sha1 digest, but more "bits" using base36 vs base16 - or the string length could be shortened so that it was equivalent to the 40 digit sha1 without increasing the (small) risk of a collision. This also would allow using sha512 without a ridiculous string length.




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