Ticket #406 - Impossible to rename entry (modrdn) with Attribute Uniqueness plugin enabled

https://fedorahosted.org/389/ticket/406
Resolves: Ticket #406
Bug Description: Impossible to rename entry (modrdn) with Attribute Uniqueness plugin enabled
Reviewed by: mreynolds (Thanks!)
Branch: master
Fix Description: Convert attribute uniqueness to use Slapi_DN* instead of
char *.  Also found that op_shared_search was always freeing the passed
in SEARCH_TARGET_SDN, so made that conditional.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no

ldap/servers/plugins/uiduniq/plugin-utils.h

@@ -86,11 +86,11 @@
 #define END } while(0);
 
 int op_error(int internal_error);
-Slapi_PBlock *readPblockAndEntry( const char *baseDN, const char *filter,
+Slapi_PBlock *readPblockAndEntry( Slapi_DN *baseDN, const char *filter,
 								  char *attrs[] );
 int entryHasObjectClass(Slapi_PBlock *pb, Slapi_Entry *e,
 						const char *objectClass);
-Slapi_PBlock *dnHasObjectClass( const char *baseDN, const char *objectClass );
+Slapi_PBlock *dnHasObjectClass( Slapi_DN *baseDN, const char *objectClass );
 Slapi_PBlock *dnHasAttribute( const char *baseDN, const char *attrName );
 
 #endif /* _PLUGIN_UTILS_H_ */

ldap/servers/plugins/uiduniq/uid.c

@@ -69,8 +69,8 @@ int ldap_quote_filter_value(
       int *outLen);
 
 
-static int search_one_berval(const char *baseDN, const char *attrName,
-		const struct berval *value, const char *requiredObjectClass, const char *target);
+static int search_one_berval(Slapi_DN *baseDN, const char *attrName,
+		const struct berval *value, const char *requiredObjectClass, Slapi_DN *target);
 
 /*
  * ISSUES:
@@ -224,16 +224,16 @@ create_filter(const char *attribute, const struct berval *value, const char *req
  *   LDAP_OPERATIONS_ERROR - a server failure.
  */
 static int
-search(const char *baseDN, const char *attrName, Slapi_Attr *attr,
+search(Slapi_DN *baseDN, const char *attrName, Slapi_Attr *attr,
   struct berval **values, const char *requiredObjectClass,
-  const char *target)
+  Slapi_DN *target)
 {
   int result;
 
 #ifdef DEBUG
     slapi_log_error(SLAPI_LOG_PLUGIN, plugin_name,
-      "SEARCH baseDN=%s attr=%s target=%s\n", baseDN, attrName, 
-      target?target:"None");
+                    "SEARCH baseDN=%s attr=%s target=%s\n", slapi_sdn_get_dn(baseDN), attrName, 
+                    target?slapi_sdn_get_dn(target):"None");
 #endif
 
   result = LDAP_SUCCESS;
@@ -282,9 +282,9 @@ search(const char *baseDN, const char *attrName, Slapi_Attr *attr,
 
 
 static int
-search_one_berval(const char *baseDN, const char *attrName,
+search_one_berval(Slapi_DN *baseDN, const char *attrName,
 		const struct berval *value, const char *requiredObjectClass,
-		const char *target)
+		Slapi_DN *target)
 {
 	int result;
     char *filter;
@@ -317,7 +317,7 @@ search_one_berval(const char *baseDN, const char *attrName,
       spb = slapi_pblock_new();
       if (!spb) { result = uid_op_error(2); break; }
 
-      slapi_search_internal_set_pb(spb, baseDN, LDAP_SCOPE_SUBTREE,
+      slapi_search_internal_set_pb_ext(spb, baseDN, LDAP_SCOPE_SUBTREE,
       	filter, attrs, 0 /* attrs only */, NULL, NULL, plugin_identity, 0 /* actions */);
       slapi_search_internal_pb(spb);
 
@@ -340,18 +340,16 @@ search_one_berval(const char *baseDN, const char *attrName,
        */
       for(;*entries;entries++)
       {
-        char *ndn = slapi_entry_get_ndn(*entries); /* get the normalized dn */
-
 #ifdef DEBUG
         slapi_log_error(SLAPI_LOG_PLUGIN, plugin_name,
-          "SEARCH entry dn=%s\n", ndn);
+                        "SEARCH entry dn=%s\n", slapi_entry_get_dn(*entries));
 #endif
 
         /*
          * It is a Constraint Violation if any entry is found, unless
          * the entry is the target entry (if any).
          */
-        if (!target || strcmp(ndn, target) != 0)
+        if (!target || slapi_sdn_compare(slapi_entry_get_sdn(*entries), target) != 0)
         {
           result = LDAP_CONSTRAINT_VIOLATION;
           break;
@@ -394,7 +392,7 @@ search_one_berval(const char *baseDN, const char *attrName,
 static int
 searchAllSubtrees(int argc, char *argv[], const char *attrName,
   Slapi_Attr *attr, struct berval **values, const char *requiredObjectClass,
-  const char *dn)
+  Slapi_DN *dn)
 {
   int result = LDAP_SUCCESS;
 
@@ -404,13 +402,17 @@ searchAllSubtrees(int argc, char *argv[], const char *attrName,
    */
   for(;argc > 0;argc--,argv++)
   {
+    Slapi_DN *sufdn = slapi_sdn_new_dn_byref(*argv);
     /*
      * The DN should already be normalized, so we don't have to
      * worry about that here.
      */
-    if (slapi_dn_issuffix(dn, *argv)) {
-      result = search(*argv, attrName, attr, values, requiredObjectClass, dn);
+    if (slapi_sdn_issuffix(dn, sufdn)) {
+      result = search(sufdn, attrName, attr, values, requiredObjectClass, dn);
+      slapi_sdn_free(&sufdn);
       if (result) break;
+    } else {
+      slapi_sdn_free(&sufdn);
     }
   }
   return result;
@@ -497,27 +499,35 @@ getArguments(Slapi_PBlock *pb, char **attrName, char **markerObjectClass,
  *   LDAP_OPERATIONS_ERROR - a server failure.
  */
 static int
-findSubtreeAndSearch(char *parentDN, const char *attrName, Slapi_Attr *attr,
-  struct berval **values, const char *requiredObjectClass, const char *target,
+findSubtreeAndSearch(Slapi_DN *parentDN, const char *attrName, Slapi_Attr *attr,
+  struct berval **values, const char *requiredObjectClass, Slapi_DN *target,
   const char *markerObjectClass)
 {
   int result = LDAP_SUCCESS;
   Slapi_PBlock *spb = NULL;
+  Slapi_DN *curpar = slapi_sdn_new();
+  Slapi_DN *newpar = NULL;
 
-  while (NULL != (parentDN = slapi_dn_parent(parentDN)))
+  slapi_sdn_get_parent(parentDN, curpar);
+  while ((curpar != NULL) && (slapi_sdn_get_dn(curpar) != NULL))
   {
-        if ((spb = dnHasObjectClass(parentDN, markerObjectClass)))
+        if ((spb = dnHasObjectClass(curpar, markerObjectClass)))
         {
           freePblock(spb);
           /*
            * Do the search.   There is no entry that is allowed
            * to have the attribute already.
            */
-          result = search(parentDN, attrName, attr, values, requiredObjectClass,
+          result = search(curpar, attrName, attr, values, requiredObjectClass,
                           target);
           break;
         }
+        newpar = slapi_sdn_new();
+        slapi_sdn_copy(curpar, newpar);
+        slapi_sdn_get_parent(newpar, curpar);
+        slapi_sdn_free(&newpar);
   }
+  slapi_sdn_free(&curpar);
   return result;
 }
 
@@ -547,7 +557,6 @@ preop_add(Slapi_PBlock *pb)
     int err;
     char *markerObjectClass = NULL;
     char *requiredObjectClass = NULL;
-    const char *dn = NULL;
     Slapi_DN *sdn = NULL;
     int isupdatedn;
     Slapi_Entry *e;
@@ -593,10 +602,8 @@ preop_add(Slapi_PBlock *pb)
     err = slapi_pblock_get(pb, SLAPI_ADD_TARGET_SDN, &sdn);
     if (err) { result = uid_op_error(51); break; }
 
-    dn = slapi_sdn_get_dn(sdn);
-
 #ifdef DEBUG
-    slapi_log_error(SLAPI_LOG_PLUGIN, plugin_name, "ADD target=%s\n", dn);
+    slapi_log_error(SLAPI_LOG_PLUGIN, plugin_name, "ADD target=%s\n", slapi_sdn_get_dn(sdn));
 #endif
 
        /*
@@ -629,14 +636,14 @@ preop_add(Slapi_PBlock *pb)
         if (NULL != markerObjectClass)
         {
           /* Subtree defined by location of marker object class */
-                result = findSubtreeAndSearch((char *)dn, attrName, attr, NULL,
-                                              requiredObjectClass, dn,
+                result = findSubtreeAndSearch(sdn, attrName, attr, NULL,
+                                              requiredObjectClass, sdn,
                                               markerObjectClass);
         } else
         {
           /* Subtrees listed on invocation line */
           result = searchAllSubtrees(argc, argv, attrName, attr, NULL,
-                                     requiredObjectClass, dn);
+                                     requiredObjectClass, sdn);
         }
   END
 
@@ -703,7 +710,6 @@ preop_modify(Slapi_PBlock *pb)
     int modcount = 0;
     int ii;
     LDAPMod *mod;
-    const char *dn = NULL;
     Slapi_DN *sdn = NULL;
     int isupdatedn;
     int argc;
@@ -775,12 +781,11 @@ preop_modify(Slapi_PBlock *pb)
     err = slapi_pblock_get(pb, SLAPI_MODIFY_TARGET_SDN, &sdn);
     if (err) { result = uid_op_error(11); break; }
 
-    dn = slapi_sdn_get_dn(sdn);
     /*
      * Check if it has the required object class
      */
     if (requiredObjectClass &&
-        !(spb = dnHasObjectClass(dn, requiredObjectClass))) {
+        !(spb = dnHasObjectClass(sdn, requiredObjectClass))) {
         break;
     }
 
@@ -798,14 +803,14 @@ preop_modify(Slapi_PBlock *pb)
         if (NULL != markerObjectClass)
         {
             /* Subtree defined by location of marker object class */
-            result = findSubtreeAndSearch((char *)dn, attrName, NULL, 
+            result = findSubtreeAndSearch(sdn, attrName, NULL, 
                                           mod->mod_bvalues, requiredObjectClass,
-                                          dn, markerObjectClass);
+                                          sdn, markerObjectClass);
         } else
         {
             /* Subtrees listed on invocation line */
             result = searchAllSubtrees(argc, argv, attrName, NULL,
-                                       mod->mod_bvalues, requiredObjectClass, dn);
+                                       mod->mod_bvalues, requiredObjectClass, sdn);
         }
     }
   END
@@ -947,7 +952,7 @@ preop_modrdn(Slapi_PBlock *pb)
 
     /* Apply the rename operation to the dummy entry. */
     /* slapi_entry_rename does not expect rdn normalized */
-    err = slapi_entry_rename(e, rdn, deloldrdn, slapi_sdn_get_dn(superior));
+    err = slapi_entry_rename(e, rdn, deloldrdn, superior);
     if (err != LDAP_SUCCESS) { result = uid_op_error(36); break; }
 
         /*
@@ -970,14 +975,14 @@ preop_modrdn(Slapi_PBlock *pb)
         if (NULL != markerObjectClass)
         {
           /* Subtree defined by location of marker object class */
-                result = findSubtreeAndSearch(slapi_entry_get_dn(e), attrName, attr, NULL,
-                                              requiredObjectClass, dn,
+                result = findSubtreeAndSearch(slapi_entry_get_sdn(e), attrName, attr, NULL,
+                                              requiredObjectClass, sdn,
                                               markerObjectClass);
         } else
         {
           /* Subtrees listed on invocation line */
           result = searchAllSubtrees(argc, argv, attrName, attr, NULL,
-                                     requiredObjectClass, dn);
+                                     requiredObjectClass, sdn);
         }
   END
   /* Clean-up */

ldap/servers/plugins/uiduniq/utils.c

@@ -81,7 +81,7 @@ op_error(int internal_error) {
  *   A pblock containing the entry, or NULL
  */
 Slapi_PBlock *
-readPblockAndEntry( const char *baseDN, const char *filter,
+readPblockAndEntry( Slapi_DN *baseDN, const char *filter,
 					char *attrs[] ) {
 	Slapi_PBlock *spb = NULL;
 
@@ -89,7 +89,7 @@ readPblockAndEntry( const char *baseDN, const char *filter,
         int sres;
 
 		/* Perform the search - the new pblock needs to be freed */
-		spb = slapi_search_internal((char *)baseDN, LDAP_SCOPE_BASE,
+		spb = slapi_search_internal(slapi_sdn_get_dn(baseDN), LDAP_SCOPE_BASE,
 									(char *)filter, NULL, attrs, 0);
 		if ( !spb ) {
 			op_error(20);
@@ -150,7 +150,7 @@ entryHasObjectClass(Slapi_PBlock *pb, Slapi_Entry *e,
  *   A pblock containing the entry, or NULL
  */
 Slapi_PBlock *
-dnHasObjectClass( const char *baseDN, const char *objectClass ) {
+dnHasObjectClass( Slapi_DN *baseDN, const char *objectClass ) {
 	char *filter = NULL;
 	Slapi_PBlock *spb = NULL;
 

ldap/servers/slapd/back-ldbm/ldbm_search.c

@@ -1396,10 +1396,6 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension )
     
     inst = (ldbm_instance *) be->be_instance_info;
 
-    if (NULL == basesdn) {
-        basesdn = slapi_sdn_new_normdn_byref(base);
-        slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, basesdn);
-    }
     /* Return to the cache the entry we handed out last time */
     /* If we are using the extension, the front end will tell
      * us when to do this so we don't do it now */

ldap/servers/slapd/entry.c

@@ -3155,16 +3155,16 @@ slapi_entry_has_children(const Slapi_Entry *entry)
  * Renames an entry to simulate a MODRDN operation
  */
 int
-slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const char *newsuperior)
+slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, Slapi_DN *newsuperior)
 {
     int err = LDAP_SUCCESS;
-    char *newdn = NULL;
-    char *olddn = NULL;
-    Slapi_RDN *oldrdn = NULL;
+    Slapi_DN *olddn = NULL;
     Slapi_Mods *smods = NULL;
+    Slapi_DN newsrdn;
 
     LDAPDebug( LDAP_DEBUG_TRACE, "=> slapi_entry_rename\n", 0, 0, 0 );
 
+    slapi_sdn_init(&newsrdn);
     /* Check if entry or newrdn are NULL. */
     if (!e || !newrdn) {
         err = LDAP_PARAM_ERROR;
@@ -3172,7 +3172,7 @@ slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const c
     }
 
     /* Get the old DN. */
-    olddn = slapi_entry_get_dn(e);
+    olddn = slapi_entry_get_sdn(e);
 
     /* If deleteoldrdn, find old RDN values and remove them from the entry. */
     if (deleteoldrdn) {
@@ -3180,8 +3180,7 @@ slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const c
         char * val = NULL;
         int num_rdns = 0;
         int i = 0;
-
-        oldrdn = slapi_rdn_new_dn(olddn);
+        Slapi_RDN *oldrdn = slapi_rdn_new_sdn(olddn);
 
         /* Create mods based on the number of rdn elements. */
         num_rdns = slapi_rdn_get_num_components(oldrdn);
@@ -3194,6 +3193,7 @@ slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const c
                 slapi_mods_add(smods, LDAP_MOD_DELETE, type, strlen(val), val);
             }
         }
+        slapi_rdn_free(&oldrdn);
 
         /* Apply the mods to the entry. */
         if ((err = slapi_entry_apply_mods(e, slapi_mods_get_ldapmods_byref(smods))) != LDAP_SUCCESS) {
@@ -3209,29 +3209,32 @@ slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const c
 
     /* Build new DN.  If newsuperior is set, just use "newrdn,newsuperior".  If
      * newsuperior is not set, need to add newrdn to old superior. */
+    slapi_sdn_init_dn_byref(&newsrdn, newrdn);
     if (newsuperior) {
-        newdn = slapi_create_dn_string("%s,%s", newrdn, newsuperior);
+        slapi_sdn_set_parent(&newsrdn, newsuperior);
     } else {
-        char *oldsuperior = NULL;
-
-        oldsuperior = slapi_dn_parent(olddn);
-        newdn = slapi_create_dn_string("%s,%s", newrdn, oldsuperior);
+        Slapi_DN oldparent;
 
-        slapi_ch_free_string(&oldsuperior);
+        slapi_sdn_init(&oldparent);
+        slapi_sdn_get_parent(olddn, &oldparent);
+        slapi_sdn_set_parent(&newsrdn, &oldparent);
+        slapi_sdn_done(&oldparent);
     }
 
     /* Set the new DN in the entry.  This hands off the memory used by newdn to the entry. */
-    slapi_entry_set_normdn(e, newdn);
+    slapi_entry_set_sdn(e, &newsrdn);
 
     /* Set the RDN in the entry. */
-    slapi_entry_set_rdn(e, newdn);
+    /* note - there isn't a slapi_entry_set_rdn_from_sdn function */
+    slapi_rdn_done(slapi_entry_get_srdn(e));
+    slapi_rdn_init_all_sdn(slapi_entry_get_srdn(e),&newsrdn);
 
     /* Add RDN values to entry. */
     err = slapi_entry_add_rdn_values(e);
 
 done:
-    slapi_rdn_free(&oldrdn);
     slapi_mods_free(&smods);
+    slapi_sdn_done(&newsrdn);
 
     LDAPDebug( LDAP_DEBUG_TRACE, "<= slapi_entry_rename\n", 0, 0, 0 );
     return err;

ldap/servers/slapd/opshared.c

@@ -268,6 +268,8 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
   void *pr_search_result = NULL;
   int pr_reset_processing = 0;
   int pr_idx = -1;
+  Slapi_DN *orig_sdn = NULL;
+  int free_sdn = 0;
 
   be_list[0] = NULL;
   referral_list[0] = NULL;
@@ -279,6 +281,11 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
   if (NULL == sdn) {
     sdn = slapi_sdn_new_dn_byval(base);
     slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, sdn);
+    free_sdn = 1;
+  } else {
+    /* save it so we can restore it later - may have to replace it internally
+       e.g. for onelevel and subtree searches, but need to restore it */
+    orig_sdn = sdn; 
   }
   normbase = slapi_sdn_get_dn(sdn);
 
@@ -725,10 +732,13 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
             int tmp_scope = LDAP_SCOPE_BASE;
             slapi_pblock_set(pb, SLAPI_SEARCH_SCOPE, &tmp_scope);
 
-            slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
-            slapi_sdn_free(&sdn);
+            if (free_sdn) {
+              slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
+              slapi_sdn_free(&sdn);
+            }
             sdn = slapi_sdn_dup(be_suffix);
             slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, (void *)sdn);
+            free_sdn = 1;
           }
           else if (slapi_sdn_issuffix(basesdn, be_suffix))
           {
@@ -749,10 +759,13 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
         {
           if (slapi_sdn_issuffix(be_suffix, basesdn))
           {
-            slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
-            slapi_sdn_free(&sdn);
+            if (free_sdn) {
+              slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
+              slapi_sdn_free(&sdn);
+            }
             sdn = slapi_sdn_dup(be_suffix);
             slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, (void *)sdn);
+            free_sdn = 1;
           }
         }
       }
@@ -968,11 +981,13 @@ free_and_return:
 free_and_return_nolock:
   slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &rc);
   index_subsys_filter_decoders_done(pb);
-  
-  slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
-  slapi_sdn_free(&sdn);
+
+  if (free_sdn) {
+    slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
+    slapi_sdn_free(&sdn);
+  }
   slapi_sdn_free(&basesdn);
-  slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, NULL);
+  slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, orig_sdn);
 
   slapi_ch_free_string(&proxydn);
   slapi_ch_free_string(&proxystr);

ldap/servers/slapd/slapi-plugin.h

@@ -2059,7 +2059,7 @@ int slapi_entry_apply_mod(Slapi_Entry *e, LDAPMod *mod);
  * \return \c LDAP_SUCCESS if the rename was successful, otherwise an LDAP error
  *         is returned.
  */
-int slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, const char *newsuperior);
+int slapi_entry_rename(Slapi_Entry *e, const char *newrdn, int deleteoldrdn, Slapi_DN *newsuperior);
 
 
 /*------------------------