commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christian Hammers ...@lathspell.de>
Subject Re: [codec] Vacuous bit mask operation on integer value in UnixCrypt
Date Thu, 30 Aug 2012 00:00:40 GMT
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 <garydgregory@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.neidhart@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 <ch@lathspell.de>:
> > >
> > >> Hello
> > >>
> > >> Am Mon, 27 Aug 2012 16:39:23 -0400
> > >> schrieb Gary Gregory <garydgregory@gmail.com>:
> > >>
> > >>> On Mon, Aug 27, 2012 at 2:53 PM, Thomas Neidhart
> > >>> <thomas.neidhart@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-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message