MilovdZee commented on a change in pull request #336:
URL: https://github.com/apache/tomcat/pull/336#discussion_r467140671
##########
File path: java/org/apache/catalina/realm/MessageDigestCredentialHandler.java
##########
@@ -32,16 +32,13 @@
/**
* This credential handler supports the following forms of stored passwords:
* <ul>
- * <li><b>encodedCredential</b> - a hex encoded digest of the password digested
- * using the configured digest</li>
- * <li><b>{MD5}encodedCredential</b> - a Base64 encoded MD5 digest of the
- * password</li>
- * <li><b>{SHA}encodedCredential</b> - a Base64 encoded SHA1 digest of the
- * password</li>
- * <li><b>{SSHA}encodedCredential</b> - 20 character salt followed by the
salted
- * SHA1 digest Base64 encoded</li>
- * <li><b>salt$iterationCount$encodedCredential</b> - a hex encoded salt,
- * iteration code and a hex encoded credential, each separated by $</li>
+ * <li><b>encodedCredential</b> - a hex encoded digest of the password
digested using the configured digest</li>
+ * <li><b>{MD5}encodedCredential</b> - a Base64 encoded MD5 digest of the
password</li>
+ * <li><b>{SHA}encodedCredential</b> - a Base64 encoded SHA1 digest of the
password</li>
+ * <li><b>{SSHA}encodedCredential</b> - 20 character SHA1 digest Base64
encoded followed by salt</li>
+ * <li><b>{SSHA2}encodedCredential</b> - 20 character salt followed by the
salted digest Base64 encoded</li>
Review comment:
> What does `{SSHAv2}` add that the existing `salt$iteration$hash`
doesn't already provide?
I would have preferred to do the SSHA implementation differently but that
would break backward compatibility. So I choose SSHAv2...
I agree that I'm now using the `salt$iteration$hash` way and indeed SSHAv2
does not add functionality. Adding SSHAv2 introduces yet another option and
needs support...
Maybe better to refactor the existing code to be more readable?
But why allow the algorithm to be set when the SSHA only works when it is
set to SHA-1? I did not know that SSHA was something of a standard and I now
see that the implementation is indeed correct. Only the javadoc is incorrect.
I'll just do a refactor of the implementation and fix the javadoc then. And
I won't add the SSHAv2 option.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
