gemmellr commented on a change in pull request #3851:
URL: https://github.com/apache/activemq-artemis/pull/3851#discussion_r751191216
##########
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java
##########
@@ -188,6 +208,16 @@ public String encode(String secret) throws Exception {
BigInteger n = new BigInteger(encoding);
return n.toString(16);
}
+
+ @Override
+ public boolean verify(char[] inputValue, String storedValue) {
+ try {
+ return Objects.equals(storedValue,
encode(String.valueOf(inputValue)));
Review comment:
The methods here seems somewhat all over the place.
The class implements SensitiveDataCodec<String>, meaning it has to return
String (i.e T) for encode and decode methods, but is only taking Object args
which it then blindly assumes are actually String and casts to that unchecked.
The new compare method also takes Object args and then for some odd reason
requires one of them to be String or char[], but only String in the other. It
converts the first to char[] if it wasnt, so it can call this verify method
which has fixed char[] and String type args. Which then turns the char[]
_[back]_ into a String so it can call encode with a String to get another
String to further compare to the given String.
Why allow different types in the various different methods? Why is one
methods args fixed type and the others floating Object, that are then assumed
or enforced to be a specific types (meaning the broker has to be calling it
with those types), and yet always ending up as String ultimately. Almost
nothing is using the generic T type to pick up the String generic from the
class definition but that appears to be the only type that is used in the
broker so nothing else will actually work.
Actually, why add both new verify and compare methods when they do
essentially the same thing overall? Verify is probably a better name overall
since compare doesnt as much imply the fact one arg has to be manipulated to
equal the other (and the not-present javadoc doesnt say what the method
actually does at all)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
