Hello I did some more unit tests. Could you grant me commit rights (Apache user "chammers") or should I fill your inbox with patches? :-)
BTW, I would have made a git fork but the "$Id$" lines show up in every diff. Why are they not expanded in the git shadow repo? I thought that would happen at the svn commit time? bye, -christian- Am Wed, 29 Aug 2012 09:05:56 -0400 schrieb Gary Gregory <garydgreg...@gmail.com>: > How about this: > > 1. Let's get the digest package to 100% code coverage (or as closed > to it as possible) > 2. Clean up the code (removing the "vacuous" code) > > Also to do is fix the remaining check styles. > > Patches welcome! > > Gary > > On Tue, Aug 28, 2012 at 2:47 AM, Thomas Neidhart > <thomas.neidh...@gmail.com>wrote: > > > On 08/28/2012 03:33 AM, Christian Hammers wrote: > > > Am Tue, 28 Aug 2012 00:09:41 +0200 > > > schrieb Christian Hammers <c...@lathspell.de>: > > > > > >> Hello > > >> > > >> Am Mon, 27 Aug 2012 16:39:23 -0400 > > >> schrieb Gary Gregory <garydgreg...@gmail.com>: > > >> > > >>> On Mon, Aug 27, 2012 at 2:53 PM, Thomas Neidhart > > >>> <thomas.neidh...@gmail.com>wrote: > > >>> > > >>>> On 08/27/2012 03:36 PM, Gary Gregory wrote: > > >>>>> Hi All: > > >>>>> > > >>>>> FindBugs says the following a couple of times for UnixCrypt: > > >>>>> "Vacuous bit mask operation on integer value > > >>>>> (INT_VACUOUS_BIT_OPERATION) > > >>>>> > > >>>>> This is an integer bit operation (and, or, or exclusive or) > > >>>>> that doesn't > > >>>> do > > >>>>> any useful work (e.g., v & 0xffffffff)." > > >>>>> This makes me wonder if the whole class is correct in the > > >>>>> first place and if/how/why these ops got in there. > > >>>>> > > >>>>> Was this translated (incorrectly) from JetSpeed's > > >>>>> implementation? Where > > >>>> the > > >>>>> types (int vs longs?) in JetSpeed different? Where did > > >>>>> JetSpeed get this implementation? > > >>>> > > >>>> I do not know the source of the JetSpeed implementation, but > > >>>> there are several (slightly different) variants of this > > >>>> floating around in other projects: > > >>>> > > >>>> > > >>>> > > http://grepcode.com/file/repository.jboss.org/maven2/postgresql/postgresql/8.4-701.jdbc3/org/postgresql/util/UnixCrypt.java > > >>>> > > >>>> > > >>>> > > http://grepcode.com/file/repo1.maven.org/maven2/org.mortbay.jetty/jetty/6.1.11/org/mortbay/jetty/security/UnixCrypt.jav > > >>>> > > >>>> The jetty version looks the most clean one, and as it is also > > >>>> Apache license, maybe we should take this one and cleanup the > > >>>> code a bit? > > >>>> > > >>> > > >>> The first thing to try would be to drop in the Jetty impl (I > > >>> looked at the one in the 8.1 version) and see our unit tests > > >>> run unchanged (aside from the different APIs). I cannot do this > > >>> today though. Feel try to try ;) > > >>> > > >>> Gary > > >> > > >> The original Jetspeed source also had this "v & 0xffffffff": > > >> > > http://www.javadocexamples.com/java_source/org/apache/jetspeed/services/security/ldap/UnixCrypt.java.html > > >> > > http://archive.apache.org/dist/jakarta/jetspeed/jetspeed-current-src.zip > > >> > > >> But I will have a look now how the Jetty source looks in > > >> comparison... > > > > > > The Aki Yoshida version from Jetty does not pass the UTF-8 unit > > > tests: It gives the same result for ASCII inputs and also for a > > > single 0x80 but two consecutive bytes > 0x80 like 0xC3 0xA4 for > > > "ä" gives a different result than "perl -e 'print crypt("ä", > > > "xx");'. As it looks totally different than all the other > > > UnixCrypt implementations I have no clue how to fix it. > > > > > > Regarding the other ones, most of them have a common ancestors in > > > John Dumas Java port which is the one with the "v & 0xffffffff". > > > http://www.dynamic.net.au/christos/crypt/ gives a nice > > > genealogy overview :) > > > > > > The first one mentioned there, from Davit Scott, also works but is > > > not only significantly slower than all the other ones but also > > > does not have a clear license statement so I doubt that we could > > > use it. > > > > > > Our current version, which is similar to the PostgreSQL JDBC one > > > mentioned above, passes all tests without the "0xffffffff" though. > > > As it "int" is defined as a 4 bytes in the JLS 4.2.1 and not > > > platform dependend like in C, we could just remove it and make > > > FindBugs happy. > > > > > > Apropos, "Math.abs(r.nextInt()) % numSaltChars" could be replaced > > > by "r.nextInt(numSaltChars)" and FindBugs would be completely > > > clean for this file. > > > > Interesting is also that in the postgresql variant, the bitwise and > > operation is using a slightly different argument: > > > > c &= 0x0fffffff; > > > > So they must have made this observation too because the rest of the > > code looks very much the same. > > > > Thomas > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
