ldap/servers/plugins/replication/urp.c | 18 +++-----
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 35 +++++++++++-----
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2
ldap/servers/slapd/back-ldbm/ldbm_search.c | 2
ldap/servers/slapd/dn.c | 59 ++++++++++++++++++++++++---
ldap/servers/slapd/rdn.c | 24 +++-------
ldap/servers/slapd/slapi-plugin.h | 18 ++++++++
7 files changed, 116 insertions(+), 42 deletions(-)
New commits:
commit c2c64017e0e25a4376139debe29b4f587964710f
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Thu Jun 6 16:52:14 2013 -0700
Ticket #47367 - (phase 1) ldapdelete returns non-leaf entry error while trying to
remove a leaf entry
Bug description: Replication conflict confuses the numsubordinate
count, which leaves an entry that cannot be deleted even its
subordinate entries are all removed.
Fix description:
[urp.c] get_dn_plus_uniqueid: a logic to create a conflict DN
had a bug. It used to call slapi_sdn_get_rdn to get the rdn.
The function slapi_sdn_get_rdn blindly returned the "dn" field
without checking whether the field is NULL or not. Instead,
this patch changes the interface of the helper function get_
dn_plus_uniqueid and use the original Slapi_DN with slapi_
sdn_get_dn, then generates the conflict DN "nsuniqueid=...+
<RDN>,<PARENT>".
[ldbm_delete.c] There is a case a parent of a delete-candidate
entry runs into a conflict and multiple parent entries exist.
Once it occurs, a parent entry found by the parent dn string
may not be the entry which manages the numsubordinate count
the delete-candidate entry belonging to. It confuses the
numsubordinate counts and leaves an entry which cannot be
deleted due to the numsubordinate count mismatch. This patch
retrieves parent entry by parent id if it is available.
[ldbm_entryrdn.c] When traversing the DIT, a special treatment
is needed for a tombstone entry. I.e, 2 RDNs (nsuniqueid=...,
<RDN>) is treated as one RDN. It should decrement the index
(rdnidx) one more to point to the right position of the RDN
array in Slapi_RDN.
[ldbm_search.c] When checking the scope of an entry in ldbm_
back_next_search_entry_ext, a tombstone entry was not properly
examined. This patch introduces a new slapi api slapi_sdn_
scope_test_ext.
[dn.c] In slapi_sdn_get_rdn, use slapi_sdn_get_dn to get the
dn value of Slapi_DN. It was one cause of the problem in
get_dn_plus_uniqueid (urp.c).
This patch adds slapi_sdn_scope_test_ext, which takes flags
to indicates the first argument dn is a tombstone sdn.
Also, this patch replaces "malloc + strcpy + strcat" with
slapi_ch_smprintf to improve the readability of the code.
[rdn.c] This patch replaces "malloc + strcpy + strcat" with
slapi_create_dn_string to normalize the newly added rdn and
improve the readability of the code.
Reviewed by Rich (Thank you!!)
https://fedorahosted.org/389/ticket/47367
diff --git a/ldap/servers/plugins/replication/urp.c
b/ldap/servers/plugins/replication/urp.c
index 1d8799a..e236541 100644
--- a/ldap/servers/plugins/replication/urp.c
+++ b/ldap/servers/plugins/replication/urp.c
@@ -57,7 +57,7 @@ static int urp_annotate_dn (char *sessionid, Slapi_Entry *entry, CSN
*opcsn, con
static int urp_naming_conflict_removal (Slapi_PBlock *pb, char *sessionid, CSN *opcsn,
const char *optype);
static int mod_namingconflict_attr (const char *uniqueid, const Slapi_DN *entrysdn, const
Slapi_DN *conflictsdn, CSN *opcsn);
static int del_replconflict_attr (Slapi_Entry *entry, CSN *opcsn, int opflags);
-static char *get_dn_plus_uniqueid(char *sessionid,const char *olddn,const char
*uniqueid);
+static char *get_dn_plus_uniqueid(char *sessionid,const Slapi_DN *oldsdn,const char
*uniqueid);
static char *get_rdn_plus_uniqueid(char *sessionid,const char *olddn,const char
*uniqueid);
static int is_suffix_entry (Slapi_PBlock *pb, Slapi_Entry *entry, Slapi_DN **parenddn);
@@ -180,7 +180,7 @@ urp_add_operation( Slapi_PBlock *pb )
if (r<0)
{
/* Entry to be added is a loser */
- char *newdn= get_dn_plus_uniqueid (sessionid, basedn, adduniqueid);
+ char *newdn = get_dn_plus_uniqueid (sessionid, (const Slapi_DN *)addentry,
adduniqueid);
if(newdn==NULL)
{
op_result= LDAP_OPERATIONS_ERROR;
@@ -1222,16 +1222,15 @@ bailout:
/* The returned value is either null or
"uniqueid=<uniqueid>+<basedn>" */
static char *
-get_dn_plus_uniqueid(char *sessionid, const char *olddn, const char *uniqueid)
+get_dn_plus_uniqueid(char *sessionid, const Slapi_DN *oldsdn, const char *uniqueid)
{
- Slapi_DN *sdn= slapi_sdn_new_dn_byval(olddn);
Slapi_RDN *rdn= slapi_rdn_new();
char *newdn;
PR_ASSERT(uniqueid!=NULL);
/* Check if the RDN already contains the Unique ID */
- slapi_sdn_get_rdn(sdn,rdn);
+ slapi_rdn_set_dn(rdn, slapi_sdn_get_dn(oldsdn));
if(slapi_rdn_contains(rdn,SLAPI_ATTR_UNIQUEID,uniqueid,strlen(uniqueid)))
{
/* The Unique ID is already in the RDN.
@@ -1241,16 +1240,15 @@ get_dn_plus_uniqueid(char *sessionid, const char *olddn, const
char *uniqueid)
* require admin intercession
*/
slapi_log_error(SLAPI_LOG_FATAL, sessionid,
- "Annotated DN %s has naming conflict\n", olddn );
+ "Annotated DN %s has naming conflict\n", slapi_sdn_get_dn(oldsdn) );
newdn= NULL;
}
else
{
- slapi_rdn_add(rdn,SLAPI_ATTR_UNIQUEID,uniqueid);
- slapi_sdn_set_rdn(sdn, rdn);
- newdn= slapi_ch_strdup(slapi_sdn_get_dn(sdn));
+ char *parentdn = slapi_dn_parent(slapi_sdn_get_dn(oldsdn));
+ slapi_rdn_add(rdn, SLAPI_ATTR_UNIQUEID, uniqueid);
+ newdn = slapi_ch_smprintf("%s,%s", slapi_rdn_get_rdn(rdn), parentdn);
}
- slapi_sdn_free(&sdn);
slapi_rdn_free(&rdn);
return newdn;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 528693e..d80c54e 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -242,14 +242,12 @@ ldbm_back_delete( Slapi_PBlock *pb )
*/
is_tombstone_entry = slapi_entry_flag_is_set(e->ep_entry,
SLAPI_ENTRY_FLAG_TOMBSTONE);
if (delete_tombstone_entry) {
- PR_ASSERT(is_tombstone_entry);
if (!is_tombstone_entry) {
slapi_log_error(SLAPI_LOG_FATAL, "ldbm_back_delete",
"Attempt to delete a non-tombstone entry %s\n", dn);
delete_tombstone_entry = 0;
}
} else {
- PR_ASSERT(!is_tombstone_entry);
if (is_tombstone_entry) {
slapi_log_error(SLAPI_LOG_FATAL, "ldbm_back_delete",
"Attempt to Tombstone again a tombstone entry %s\n", dn);
@@ -328,12 +326,31 @@ ldbm_back_delete( Slapi_PBlock *pb )
if ( !slapi_sdn_isempty(&parentsdn) )
{
struct backentry *parent = NULL;
- entry_address parent_addr;
+ char *pid_str = slapi_entry_attr_get_charptr(e->ep_entry, LDBM_PARENTID_STR);
+ if (pid_str) {
+ /* First, try to get the direct parent. */
+ /*
+ * Although a rare case, multiple parents from repl conflict could exist.
+ * In such case, if a parent entry is found just by parentsdn
+ * (find_entry2modify_only_ext), a wrong parent could be found,
+ * and numsubordinate count could get confused.
+ */
+ ID pid = (ID)strtol(pid_str, (char **)NULL, 10);
+ parent = id2entry(be, pid ,NULL, &retval);
+ if (parent && cache_lock_entry(&inst->inst_cache, parent)) {
+ /* Failed to obtain parent entry's entry lock */
+ CACHE_RETURN(&(inst->inst_cache), &parent);
+ goto error_return;
+ }
+ }
+ if (NULL == parent) {
+ entry_address parent_addr;
- parent_addr.sdn = &parentsdn;
- parent_addr.uniqueid = NULL;
- parent = find_entry2modify_only_ext(pb, be, &parent_addr,
- TOMBSTONE_INCLUDED, &txn);
+ parent_addr.sdn = &parentsdn;
+ parent_addr.uniqueid = NULL;
+ parent = find_entry2modify_only_ext(pb, be, &parent_addr,
+ TOMBSTONE_INCLUDED, &txn);
+ }
if (NULL != parent) {
int isglue;
size_t haschildren = 0;
@@ -1171,9 +1188,9 @@ common_return:
}
diskfull_return:
- if(ldap_result_code!=-1)
+ if(ldap_result_code!=-1)
{
- slapi_send_ldap_result( pb, ldap_result_code, NULL, ldap_result_message, 0, NULL );
+ slapi_send_ldap_result( pb, ldap_result_code, NULL, ldap_result_message, 0, NULL );
}
modify_term(&parent_modify_c,be);
if(dblock_acquired)
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
index f3823cb..156461b 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
@@ -3152,6 +3152,7 @@ _entryrdn_index_read(backend *be,
/* Node might be a tombstone. */
rc = _entryrdn_get_tombstone_elem(cursor, tmpsrdn,
&key, nrdn, elem);
+ rdnidx--; /* consider nsuniqueid=..,<RDN> one RDN */
}
if (rc || NULL == *elem) {
slapi_log_error(SLAPI_LOG_BACKLDBM, ENTRYRDN_TAG,
@@ -3258,6 +3259,7 @@ _entryrdn_index_read(backend *be,
}
goto bail;
}
+ rdnidx--; /* consider nsuniqueid=..,<RDN> one RDN */
} else {
slapi_ch_free((void **)&tmpelem);
if (DB_NOTFOUND != rc) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c
b/ldap/servers/slapd/back-ldbm/ldbm_search.c
index c61dcce..e68b897 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_search.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c
@@ -1602,7 +1602,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension
)
* just forget about it, since we don't want to return anything at all.
*/
{
if ( slapi_uniqueIDCompareString(target_uniqueid,
e->ep_entry->e_uniqueid) ||
- slapi_sdn_scope_test( backentry_get_sdn(e), basesdn, scope ))
+ slapi_sdn_scope_test_ext( backentry_get_sdn(e), basesdn, scope,
e->ep_entry->e_flags ))
{
/* check size limit */
if ( slimit >= 0 )
diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c
index b40b6a0..dda439b 100644
--- a/ldap/servers/slapd/dn.c
+++ b/ldap/servers/slapd/dn.c
@@ -2114,10 +2114,7 @@ slapi_sdn_add_rdn(Slapi_DN *sdn, const Slapi_RDN *rdn)
{
/* NewDN= NewRDN + DN */
const char *dn= slapi_sdn_get_dn(sdn);
- char *newdn= slapi_ch_malloc(strlen(rawrdn)+1+strlen(dn)+1);
- strcpy( newdn, rawrdn );
- strcat( newdn, "," );
- strcat( newdn, dn );
+ char *newdn = slapi_ch_smprintf("%s,%s", rawrdn, dn);
slapi_sdn_set_dn_passin(sdn,newdn);
}
return sdn;
@@ -2345,7 +2342,7 @@ slapi_sdn_get_backend_parent(const Slapi_DN *sdn,Slapi_DN
*sdn_parent,const Slap
void
slapi_sdn_get_rdn(const Slapi_DN *sdn,Slapi_RDN *rdn)
{
- slapi_rdn_set_dn(rdn,sdn->dn);
+ slapi_rdn_set_dn(rdn, slapi_sdn_get_dn(sdn));
}
Slapi_DN *
@@ -2516,6 +2513,47 @@ slapi_sdn_scope_test( const Slapi_DN *dn, const Slapi_DN *base, int
scope )
return rc;
}
+/*
+ * Return non-zero if "dn" matches the scoping criteria
+ * given by "base" and "scope".
+ * If SLAPI_ENTRY_FLAG_TOMBSTONE is set to flags,
+ * DN without "nsuniqueid=...," is examined.
+ */
+int
+slapi_sdn_scope_test_ext( const Slapi_DN *dn, const Slapi_DN *base, int scope, int flags
)
+{
+ int rc = 0;
+
+ switch ( scope ) {
+ case LDAP_SCOPE_BASE:
+ if (flags & SLAPI_ENTRY_FLAG_TOMBSTONE) {
+ Slapi_DN parent;
+ slapi_sdn_init(&parent);
+ slapi_sdn_get_parent(dn, &parent);
+ rc = ( slapi_sdn_compare( dn, &parent ) == 0 );
+ slapi_sdn_done(&parent);
+ } else {
+ rc = ( slapi_sdn_compare( dn, base ) == 0 );
+ }
+ break;
+ case LDAP_SCOPE_ONELEVEL:
+ if (flags & SLAPI_ENTRY_FLAG_TOMBSTONE) {
+ Slapi_DN parent;
+ slapi_sdn_init(&parent);
+ slapi_sdn_get_parent(dn, &parent);
+ rc = ( slapi_sdn_isparent( base, &parent ) != 0 );
+ slapi_sdn_done(&parent);
+ } else {
+ rc = ( slapi_sdn_isparent( base, dn ) != 0 );
+ }
+ break;
+ case LDAP_SCOPE_SUBTREE:
+ rc = ( slapi_sdn_issuffix( dn, base ) != 0 );
+ break;
+ }
+ return rc;
+}
+
/*
* build the new dn of an entry for moddn operations
*/
@@ -2563,8 +2601,17 @@ size_t
slapi_sdn_get_size(const Slapi_DN *sdn)
{
size_t sz = sizeof(Slapi_DN);
+ /* slapi_sdn_get_ndn_len returns the normalized dn length
+ * if dn or ndn exists. If both does not exist, it
+ * normalizes udn and set it to dn and returns the length.
+ */
sz += slapi_sdn_get_ndn_len(sdn);
- sz += strlen(sdn->dn) + 1;
+ if (sdn->dn && sdn->ndn) {
+ sz += slapi_sdn_get_ndn_len(sdn);
+ }
+ if (sdn->udn) {
+ sz += strlen(sdn->udn) + 1;
+ }
return sz;
}
diff --git a/ldap/servers/slapd/rdn.c b/ldap/servers/slapd/rdn.c
index d408f0e..fe2fae0 100644
--- a/ldap/servers/slapd/rdn.c
+++ b/ldap/servers/slapd/rdn.c
@@ -479,25 +479,17 @@ slapi_rdn_add(Slapi_RDN *rdn, const char *type, const char *value)
PR_ASSERT(NULL != value);
if(rdn->rdn==NULL)
{
- /* type=value '\0' */
- rdn->rdn= slapi_ch_malloc(strlen(type)+1+strlen(value)+1);
- strcpy( rdn->rdn, type );
- strcat( rdn->rdn, "=" );
- strcat( rdn->rdn, value );
+ /* type=value '\0' */
+ rdn->rdn = slapi_create_dn_string("%s=%s", type, value);
}
else
{
- /* type=value+rdn '\0' */
- char *newrdn= slapi_ch_malloc(strlen(type)+1+strlen(value)+1+strlen(rdn->rdn)+1);
- strcpy( newrdn, type );
- strcat( newrdn, "=" );
- strcat( newrdn, value );
- strcat( newrdn, "+" );
- strcat( newrdn, rdn->rdn );
- slapi_ch_free((void**)&rdn->rdn);
- rdn->rdn= newrdn;
- }
- slapi_unsetbit_uchar(rdn->flag,FLAG_RDNS);
+ /* type=value+rdn '\0' */
+ char *newrdn = slapi_create_dn_string("%s=%s+%s", type, value, rdn->rdn);
+ slapi_ch_free_string(&rdn->rdn);
+ rdn->rdn = newrdn;
+ }
+ slapi_unsetbit_uchar(rdn->flag,FLAG_RDNS);
return 1;
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 4e7aae5..4a2544e 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -2698,6 +2698,24 @@ int slapi_sdn_get_ndn_len(const Slapi_DN *sdn);
int slapi_sdn_scope_test( const Slapi_DN *dn, const Slapi_DN *base, int scope );
/**
+ * Checks if a DN is within a specified scope under a specified base DN.
+ * This api adjusts tombstoned DN when comparing with the base dn.
+ *
+ * \param dn A pointer to the \c Slapi_DN structure to test.
+ * \param base The base DN against which \c dn is going to be tested.
+ * \param scope The scope tested. Valid scopes are:
+ * \arg \c LDAP_SCOPE_BASE
+ * \arg \c LDAP_SCOPE_ONELEVEL
+ * \arg \c LDAP_SCOPE_SUBTREE
+ * \param flags 0 or SLAPI_ENTRY_FLAG_TOMBSTONE
+ * \return non-zero if \c dn matches the scoping criteria given by \c base and \c scope.
+ * \see slapi_sdn_compare()
+ * \see slapi_sdn_isparent()
+ * \see slapi_sdn_issuffix()
+ */
+int slapi_sdn_scope_test_ext( const Slapi_DN *dn, const Slapi_DN *base, int scope, int
flags );
+
+/**
* Retreives the RDN from a given DN.
*
* This function takes the DN stored in the \c Slapi_DN structure pointed to