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

more ldap-c-api problems



And just when you thought you were about to release a new version of
the draft...


Bug in 12.2 or Appendix A:

  [ldap_<first/next>_attribute()]
  return a pointer to an allocated buffer containing the
  current attribute name. This should be freed when no longer in use by
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  calling ldap_memfree().
  ^^^^^^^^^^^^^^^^^^^^^^

The sample code in Appendix A does not free the attribute.
UMICH LDAP blows up if ldap_memfree(a) is inserted in that code; its
ldap_<first/next>_attribute() functions apparently return a pointer to
a buffer owned by the LDAP structure.

BTW, if we *not* free the attribute, the returned attribute name can't
be used after the next ldap_next_attribute(), and it can probably only
be used while the entry that attribute came from, is "live".


The lifetimes of other values need to be clarified too.
E.g.:

The entry from ldap_<first/next>_entry() is dead after the next
ldap_next_entry(), and probably after ldap_msgfree(<search result>)
and ldap_unbind.

Some ldap_get_option()s return strings, e.g. LDAP_OPT_HOST_NAME.
Are these strings allocated and should be ldap_memfree()'d, or do they
live in the LDAP structure and are undefined after ldap_unbind()?


Another matter:

Which values returned from the LDAP library may we overwrite?
Say at least something like "only those explicitly allowed" or
"everything except opaque structs and what is explicitly forbidden".
Examples: May we overwrite strings returned by ldap_get_values with
their equivalents in the local charset (provided these are not longer,
of course)?  May we lowercase the attribute names returned from
ldap_<first/next>_attribute?  The hostname from LDAP_OPT_HOST_NAME?



Bug in 12.4:

  ldap_get_dn() (...) returns a pointer to malloc'ed space
                                           ^^^^^^
  that the caller should free by calling ldap_memfree()
  (...)
  ldap_dn2ufn() converts the DN into the user friendly format described in
  [5]. The UFN returned is malloc'ed space
                           ^^^^^^
Oops -- say "newly allocated".  Otherwise people will believe the
system malloc is used, so they can also free the pointers with free().


Inconsistency:

12.2 talks about "attribute names", the rest of the draft says
"attribute types".


Suggestion:

I've been looking at the #ifdef mess in code which tries to support
several LDAP implementations and tries to figure out which
implementation it has encountered...
It would help to add something like `#define LDAP_C_API_VERSION 1'
or `#define LDAP_RFCnnn 1' (<nnn> is the number this RFC will have)
to the draft.

-- 
Hallvard