

(ITS#5408) back-ldif bugs

Full_Name: Hallvard B Furuseth
Version: HEAD, RE23, RE24
OS: Linux
Submission from: (NULL) (
Submitted by: hallvard

of it, but need advice/discussion on functionality changes.

URL for a *draft* back-ldif/ldif.c.

Functionality changes.



    needs another escape character.


  - More characters should likely be hex-escaped:
      latter as directory separator even on Windows.  Others?
    + Control chars.

    I don't use Windows myself though.  Nor Mac...

    characters with OS-specific escaping.

    can't be move between OpenLDAP versions, before and after the


    non-leaf directory.

  - Sorted-value RDNs:


      each filename and '+'.

    + Sorting (in ldif_r_enum_tree()):

      after) would be better.

      sorted values, should they be?


* If slapcat encounters an error in some entry, it outputs a partial
  tree and returns success - no indication that the tree is wrong.

  It looks deliberate: ldif_tool_entry_first() explicitly ignores the
  error code.  Looks to me like it should at least cause be_entry_get()
  to fail.

  Maybe it should also output an empty tree at failure.  (entry_first()
  reads all the entries before it returns anything.)  Or if the intent
  is to output as much as possible in case of failure (at least with
  slapcat -c), it should not give up at the first error it encounters.

* Referral handling is broken:

  - Apparently manageDSAit should only apply to the operation's base
    object: Return a referral if the base is missing and a parent is
    a referral object.  That matches back-bdb, but breaks RFC 3296.

  - ldif_back_referrals() should return non-success for more internal
    errors, like BDB.

  - Search referrals (in ldif_back_referrals()) and continuation
    references (in ldif_back_search() get the wrong scope.
    The former should use op->ors_scope, the latter should use
    the same scope as when it looked up the entry.  (It has already

  - Modify DN should catch newSuperior=<referral object>.

  - The default_referral code can be deleted, see ITS#5339.

* Multiple suffixes break back-ldif.

  Simplest fix is to forbid multiple suffixes: Set be_flag
  SLAP_DBFLAG_ONE_SUFFIX in bi_db_init(), if I understand correctly.

* olcDbDirectory should be 'SINGLE-VALUE'd in the back-ldif schema.

* Internal errors should return LDAP result code 'other', not

  Unless there is some reason it returned those, like back-config
  expects unwillingToPerform?  I didn't see any.

* Check for Abandon.

  At least in Search, which rfc4511 requires to check for Abandon.  As
  far as I can tell we just need to sprinkle be->be_<operation>() with
    "if (op->o_abandon) return SLAPD_ABANDON;"
  and the caller will take care of the rest.

* ITS#5329: ACL deadlock.  Should the ACL check be there?  More checks?


Other bugs for which the fixes should not break anything:

* Modify DN with newSuperior does not create the new parent directory.

* Search should send noSuchObject only when it is the baseobject which
  does not exist.  (ldif_r_enum_tree() checks the scope to see if it
  .is at the baseobject, that doesn't work.)

* ldapadd <suffix entry> creates <database directory>
  if file <database directory>.ldif exists.

* tmpfile-names can in theory match a DN/RDN (and thus a subtree's
  directory name).  A simple fix is to include ",," in tempfile-names.

* slurp_file() buffer overrun if file size > (int)fstat().st_size.
  (Size > INT_MAX, file grew after fstat(), filetype where st_size=0.)

Since I was changing so much anyway, I did some other changes:

* Better result code/error handling, improve/normalize Debug output.
  - Handle errors from most system calls.  modrdn in particular was bad.
  - Drop the one use of op->o_log_prefix in Debug(), it seems out of
  - Return LDAP result codes from more functions.
  - spew_entry():
    + Remove *save_errnop arg, return/print error instead.
    + Wrong save_errno setting after close().
    + Wrong spew_entry() error-return check in ldif_back_modify().
  - move_entry() checked incorrectly for open()/spew_entry() errors.
  - Remove the ENOENT special case in spew_entry().  Should only happen
    if someone is messing with the database contents by hand, or due to
    bugs I've fixed (like in move_entry).

* Move code around, in particular:
  - Move file operations around to help error checking.
  - Do all reads before writes in LDAP update ops, to narrow down locks.
  - Move common add/modify/modrdn code to ldif_prepare_create().
  - Make directories and files in a single function ldif_spew_entry().
  - Reuse computed values more.

* Change some integer types to what they are used for.

* Cleanup:
  - Rename some functions by prepending "ldif_" for better debug output,
    and get_entry/get_entry_for_fd() -> ldif_read_entry/read_entry_file
    so those names differ more from ldif_back_entry_get().
  - Add comments.
  - Reformat somewhat.  Function headers, too long lines, spacing, etc.
  - Remove 'if something != NULL' tests where that's already guaranteed.
  - Reduce switch statements - remove no-op default/break, etc.
  - Rename some variables. dn=>ndn for normalized DN, rs=>rc for result
    code, etc.  Use macros like ors_scope for oq_search.rs_scope.