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
