[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: JLDAP Bug n fixes (ITS#1512)


I have reviewed your changes for com.novell.ldap.LDAPAttribute.  I
like much of what I see.  You have done some good work.
It has been a long while since we looked at this code, and it
is obvious it needed looking at.

I was thinking of storing all values as byte[] in way similar
to what you have done, hadn't got around to it yet.

I have some questions:

The existing code stores attribute values in an ArrayList.  The
code you supplied puts the values in an Object[] and must
allocate a new Object[] each time an attribute is added
or removed.  This seems much more expensive than simply
adding a value to a list.   We were trying to avoid object
creation a much as possible. What problem were you trying
to solve when you made that change?

If the problem was that null attributes were being incorrectly
counted, I believe there is a better way to solve that problem.
Attribute values passed in as null should not be allowed or
should be ignored.

It certainly doesn't make sense to do something like
  LDAPAttribute("name", new String[]  {"a", "b", null, "c"});
This case should probably throw an exception.

  LDAPAttribute("name", (String)null)
would act the same as LDAPAttribute("name");


>>> <dfoster@equitytg.com> 06-Jan-02 2:07:55 PM >>>
Full_Name: Dane Foster
Version: n/a
OS: WIN2K /Linux
URL: ftp://ftp.openldap.org/incoming/jldapFilez.tar.gz 
Submission from: (NULL) (

- There is a NullPointerException being thrown in the copy constructor
of the
<code>org.ietf.ldap.LDAPAttribute</code> class.

- There is ClassCastException being thrown by the
method of <code>com.novell.ldap.LDAPAttribute</code> if you set the
value of an
attribute using a byte array instead of a String.

- The <code>size</code> method of
incorrectly reports the number of values for an attribute if any of the
are <code>null</code>.  This is open to debate because one may argue
for or
against that <code>null</code> is a valid value.  I've chosen the
latter and
thus consider the behavior a bug.

- I've included a JUnit TestCase class that can reproduce the problem. 
note, the patch that I've made is not simply fixes for the bugs but was
rewrite of a large portion of the
class.  The JUnit testcase was written to ensure that my changes would
introduce new bugs into the JLDAP API.