Two new thoughts on this discussion: 1) I don't think we should deprecate methods that we've never released. IMO, deprecation is for backward compatibility for perhaps one release cycle to allow developers who've used a method to transition to the newer API. I'd say let's remove the methods instead of deprecating them. If we need them later, they're in SVN and can be easily restored.
2) I think MD5 should go in core/util -- there's nothing specifically "web" about it. Thoughts? -T On Fri, Oct 23, 2009 at 5:48 AM, Sandro Martini <[email protected]>wrote: > Hi Chris, > > first the simple answer, for Todd: > > I was just looking at that class... I usually wrap the MessageDigest > > stuff in a utility class myself, mainly because it's a few lines of > > code that would otherwise get repeated quite a lot. > right. > > > > I see that our MD5 class also lets you convert to bytes. It would be > > easy enough to extract that from the above snippet - but does anyone > > use the byte[] version of an MD5 hash? > In this class, portions of code are taken from Apache Tomcat > (org.apache.catalina.util.MD5Encoder), and this is only an "utility" > class, to solve some things in the more complex DigestAuthentication, > so when I wrote the code, this class was as closer as the original > one, but your suggestions are good. > We could add your method and deprecate others (they are working, and > in some cases they could be useful ...). > But in any case I'd keep the MD5 class, could be useful to reuse also > from other things. > > > I would also question why our MD5 class is synchronising on itself - > > as far as I was aware using MessageDigest was a thread-safe operation > > - then again, I don't actually know and the API docs don't seem to > > mention this one way or the other. > In the original source (take a look here > > http://www.docjar.com/html/api/org/apache/catalina/authenticator/DigestAuthenticator.java.html > ), they do the same, so to avoid problems I do it ... but probably you > are right, it's unnecessary, I know little this (many years of Web > Sites only) and I forgot this part. > > > Finally - why bother with the static decode method? Hashes are > > intended to be one way, right? Even if not, this method seems totally > redundant. > Yes, this was an idea to have it as a marker method for other classes > that needs to do something like MD5 (that could have the decode > feature), but we can drop it. > > > Ah, a little note: > some of my things to verify were taken from features of the Digest as > seen in the RFC, like using SHA instead of MD5, but I haven't found > anyone that uses it, and the same for other things. > Other things to verify were taken from the Tomcat implementation of > this code but probably here are unnecessary because they were > Tomcat-specific. > > Important tests still to do are to make it working also with Tomcat, > and also try to call the same URL more times (the RFC says there is a > counter in the answer that have to be read and do a +1 in the next > request) but I wasn't able to make this test ... I hope to find time > for this stuff next week. > > > Just some thoughts. > No problem, comments/suggestions are always welcome :-) . > > If you want, be free to make changes to the code, but only remember to > run the Unit Test to ensure that all works after changes ... only tell > me, to avoid doing in two the same changes :-) . > > Byeee >
