milleruntime commented on a change in pull request #1834:
URL: https://github.com/apache/accumulo/pull/1834#discussion_r542619852
##########
File path:
server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java
##########
@@ -67,6 +68,27 @@ public static void setUp() throws IOException {
}
}
+ @Test
+ public void testWireVersion() {
+ // sanity check to make sure it's a positive number
+ assertTrue(SystemToken.INTERNAL_WIRE_VERSION > 0);
+ // this is a sanity check that our wire version isn't crazy long, because
+ // it must be less than or equal to 16 chars to be used as the SALT for
SHA-512
+ // when using Crypt.crypt()
+ assertTrue(Integer.toString(SystemToken.INTERNAL_WIRE_VERSION).length() <=
16);
+ }
+
+ @Test
+ public void testCryptDefaults() {
+ // this is a sanity check that the default hash algorithm for
commons-codec's
+ // Crypt.crypt() method is still SHA-512 and the format hasn't changed
+ // if that changes, we need to consider whether the new default is
acceptable, and
+ // whether or not we want to bump the wire version
+ String hash = Crypt.crypt(new byte[0]);
+ assertEquals(3, hash.chars().filter(ch -> ch == '$').count());
+ assertTrue(hash.startsWith(SystemToken.SALT_PREFIX));
+ }
Review comment:
This is a great test. I wish we had more tests like this with
dependencies so the test will simply fail if defaults change on an update.
----------------------------------------------------------------
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:
[email protected]
