Bug 592397 - Upgrade tool dn2rdn: it does not clean up

the entrydn in id2entry

https://bugzilla.redhat.com/show_bug.cgi?id=592397

Description: If entries created by the 389 v1.2.5 or older,
the primary db (id2entry.db4) contains "entrydn: <normalized dn>".
Upgrading from the old version to v1.2.6 keeps the entrydn
attribute type and its value even though v1.2.6 is not supposed
to store the entrydn in the database.
1) This patch drops the entrydn attribute and value in upgrading
   the db.
2) If an ldif file contains entrydn attribute type and value,
   import (ldif2db[.pl]) ignores it.
3) A leak was found in the export (db2ldif[.pl]) which is fixed.
4) When nsslapd-subtree-rename-switch configuration attribute has
   the value "on", entrydn is not used nor created.  But the
   server accepted reindexing entrydn request and generated an
   entrydn index file.  This patch rejects it.
5) Entry and dn cache clear calls (cache_clear) are added to
   dblayer_instance_close in "#if defined(_USE_VALGRIND)", which
   is not defined.  To enable the code, the server needs to be
   rebuilt with defining the macro.  This is purely for debugging.

7 files changed, 101 insertions, 28 deletions

diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c
index 95d57a04..5c1f6e6c 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -2520,6 +2520,17 @@ int dblayer_instance_close(backend *be)
     if (NULL == inst)
         return -1;
 
+#if defined(_USE_VALGRIND)
+    /* When running a memory leak checking tool (e.g., valgrind),
+       it reduces the noise by enabling this code. */
+    LDAPDebug1Arg(LDAP_DEBUG_ANY, "%s: Cleaning up entry cache\n",
+                                  inst->inst_name);
+    cache_clear(&inst->inst_cache, CACHE_TYPE_ENTRY);
+    LDAPDebug1Arg(LDAP_DEBUG_ANY, "%s: Cleaning up dn cache\n",
+                                  inst->inst_name);
+    cache_clear(&inst->inst_dncache, CACHE_TYPE_DN);
+#endif
+
     if (attrcrypt_cleanup_private(inst)) {
         LDAPDebug(LDAP_DEBUG_ANY,
                   "Error: failed to clean up attrcrypt system for %s\n",
diff --git a/ldap/servers/slapd/back-ldbm/id2entry.c b/ldap/servers/slapd/back-ldbm/id2entry.c
index b58591e8..71ea940e 100644
--- a/ldap/servers/slapd/back-ldbm/id2entry.c
+++ b/ldap/servers/slapd/back-ldbm/id2entry.c
@@ -359,7 +359,7 @@ id2entry( backend *be, ID id, back_txn *txn, int *err  )
         rc = get_value_from_string((const char *)data.dptr, "rdn", &rdn);
         if (rc) {
             /* data.dptr may not include rdn: ..., try "dn: ..." */
-            ee = slapi_str2entry( data.dptr, 0 );
+            ee = slapi_str2entry( data.dptr, SLAPI_STR2ENTRY_NO_ENTRYDN );
         } else {
             char *dn = NULL;
             struct backdn *bdn = dncache_find_id(&inst->inst_dncache, id);
@@ -380,13 +380,19 @@ id2entry( backend *be, ID id, back_txn *txn, int *err  )
                 }
                 sdn = slapi_sdn_new_dn_byval((const char *)dn);
                 bdn = backdn_init(sdn, id, 0);
-                CACHE_ADD( &inst->inst_dncache, bdn, NULL );
-                CACHE_RETURN(&inst->inst_dncache, &bdn);
-                slapi_log_error(SLAPI_LOG_CACHE, ID2ENTRY,
-                                "entryrdn_lookup_dn returned: %s, "
-                                "and set to dn cache (id %d)\n", dn, id);
+                if (CACHE_ADD( &inst->inst_dncache, bdn, NULL )) {
+                    backdn_free(&bdn);
+                    slapi_log_error(SLAPI_LOG_CACHE, ID2ENTRY,
+                                    "%s is already in the dn cache\n", dn);
+                } else {
+                    CACHE_RETURN(&inst->inst_dncache, &bdn);
+                    slapi_log_error(SLAPI_LOG_CACHE, ID2ENTRY,
+                                    "entryrdn_lookup_dn returned: %s, "
+                                    "and set to dn cache (id %d)\n", dn, id);
+                }
             }
-            ee = slapi_str2entry_ext( (const char *)dn, data.dptr, 0 );
+            ee = slapi_str2entry_ext( (const char *)dn, data.dptr, 
+                                      SLAPI_STR2ENTRY_NO_ENTRYDN );
             slapi_ch_free_string(&rdn);
             slapi_ch_free_string(&dn);
         }
diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c
index d8153d7d..a8f5f5dd 100644
--- a/ldap/servers/slapd/back-ldbm/import-threads.c
+++ b/ldap/servers/slapd/back-ldbm/import-threads.c
@@ -535,7 +535,7 @@ import_producer(void *param)
                 FREE(estr);
                 continue;
             }
-            e = slapi_str2entry_ext(dn, estr, flags);
+            e = slapi_str2entry_ext(dn, estr, flags|SLAPI_STR2ENTRY_NO_ENTRYDN);
             slapi_ch_free_string(&dn);
         } else {
             e = slapi_str2entry(estr, flags);
@@ -1004,7 +1004,7 @@ index_producer(void *param)
             rc = get_value_from_string((const char *)data.dptr, "rdn", &rdn);
             if (rc) {
                 /* data.dptr may not include rdn: ..., try "dn: ..." */
-                e = slapi_str2entry( data.dptr, 0 );
+                e = slapi_str2entry( data.dptr, SLAPI_STR2ENTRY_NO_ENTRYDN );
                 if (job->flags & FLAG_DN2RDN) {
                     int len = 0;
                     int options = SLAPI_DUMP_STATEINFO | SLAPI_DUMP_UNIQUEID |
@@ -1092,7 +1092,8 @@ index_producer(void *param)
                                     "entryrdn_lookup_dn returned: %s, "
                                     "and set to dn cache\n", dn);
                 }
-                e = slapi_str2entry_ext( dn, data.dptr, 0 );
+                e = slapi_str2entry_ext(dn, data.dptr, 
+                                        SLAPI_STR2ENTRY_NO_ENTRYDN);
                 slapi_ch_free_string(&rdn);
             }
         } else {
@@ -3391,7 +3392,7 @@ import_get_and_add_parent_rdns(ImportWorkerInfo *info,
                                 "from Slapi_RDN\n", rdn, id);
             goto bail;
         }
-        e = slapi_str2entry_ext( dn, data.dptr, 0 );
+        e = slapi_str2entry_ext(dn, data.dptr, SLAPI_STR2ENTRY_NO_ENTRYDN);
         (*curr_entry)++;
         rc = index_set_entry_to_fifo(info, e, id, total_id, *curr_entry);
         if (rc) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
index 94312911..6698d835 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
@@ -1145,7 +1145,7 @@ bail:
         if (0 != myrc) {
             slapi_log_error(SLAPI_LOG_FATAL, ENTRYRDN_TAG,
                   "entryrdn_lookup_dn: Failed to close cursor: %s(%d)\n",
-                  dblayer_strerror(rc), rc);
+                  dblayer_strerror(myrc), myrc);
         }
     }
     if (db) {
diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
index e0b09fb4..0a13c256 100644
--- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
+++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
@@ -1358,7 +1358,8 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb )
             rc = get_value_from_string((const char *)data.dptr, "rdn", &rdn);
             if (rc) {
                 /* data.dptr may not include rdn: ..., try "dn: ..." */
-                ep->ep_entry = slapi_str2entry( data.dptr, str2entry_options );
+                ep->ep_entry = slapi_str2entry( data.dptr, 
+                               str2entry_options | SLAPI_STR2ENTRY_NO_ENTRYDN );
             } else {
                 char *pid_str = NULL;
                 char *pdn = NULL;
@@ -1447,10 +1448,10 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb )
                         slapi_ch_free_string(&pdn);
                     }
                     slapi_rdn_done(&psrdn);
-                    /* dn is not dup'ed in slapi_sdn_new_dn_byref.
+                    /* dn is not dup'ed in slapi_sdn_new_dn_passin.
                      * It's set to bdn and put in the dn cache. */
                     /* don't free dn */
-                    sdn = slapi_sdn_new_dn_byref(dn);
+                    sdn = slapi_sdn_new_dn_passin(dn);
                     bdn = backdn_init(sdn, temp_id, 0);
                     myrc = CACHE_ADD( &inst->inst_dncache, bdn, NULL );
                     if (myrc) {
@@ -1465,8 +1466,8 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb )
                                         "and set to dn cache\n", dn);
                     }
                 }
-                ep->ep_entry =
-                        slapi_str2entry_ext( dn, data.dptr, str2entry_options );
+                ep->ep_entry = slapi_str2entry_ext( dn, data.dptr, 
+                               str2entry_options | SLAPI_STR2ENTRY_NO_ENTRYDN );
                 slapi_ch_free_string(&rdn);
             }
         } else {
@@ -1751,11 +1752,36 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                                 CONFIG_ENTRYRDN_SWITCH);
                         }
                         LDAPDebug(LDAP_DEBUG_ANY, 
-                                "%s: Requested to index %s, but %s is off",
+                                "%s: Requested to index %s, but %s is off\n",
                                 inst->inst_name, LDBM_ENTRYRDN_STR,
                                 CONFIG_ENTRYRDN_SWITCH);
                         goto err_out;
                     }
+                } else if (strcasecmp(attrs[i]+1, LDBM_ENTRYDN_STR) == 0) {
+                    if (entryrdn_get_switch()) { /* subtree-rename: on */
+                        if (task) {
+                            slapi_task_log_notice(task, 
+                                "%s: Requested to index %s, but %s is on",
+                                inst->inst_name, LDBM_ENTRYDN_STR,
+                                CONFIG_ENTRYRDN_SWITCH);
+                        }
+                        LDAPDebug(LDAP_DEBUG_ANY,
+                                "%s: Requested to index %s, but %s is on\n",
+                                inst->inst_name, LDBM_ENTRYDN_STR,
+                                CONFIG_ENTRYRDN_SWITCH);
+                        goto err_out;
+                    } else {
+                        charray_add(&indexAttrs, attrs[i]+1);
+                        ai->ai_indexmask |= INDEX_OFFLINE;
+                        if (task) {
+                            slapi_task_log_notice(task,
+                                                  "%s: Indexing attribute: %s",
+                                                  inst->inst_name, attrs[i]+1);
+                        }
+                        LDAPDebug2Args(LDAP_DEBUG_ANY, 
+                                       "%s: Indexing attribute: %s\n",
+                                       inst->inst_name, attrs[i] + 1);
+                    }
                 } else {
                     charray_add(&indexAttrs, attrs[i]+1);
                     ai->ai_indexmask |= INDEX_OFFLINE;
@@ -1763,8 +1789,9 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                       slapi_task_log_notice(task, "%s: Indexing attribute: %s",
                                             inst->inst_name, attrs[i]+1);
                     }
-                    LDAPDebug(LDAP_DEBUG_ANY, "%s: Indexing attribute: %s\n",
-                              inst->inst_name, attrs[i]+1, 0);
+                    LDAPDebug2Args(LDAP_DEBUG_ANY, 
+                                   "%s: Indexing attribute: %s\n",
+                                   inst->inst_name, attrs[i]+1);
                 }
                 dblayer_erase_index_file(be, ai, i/* chkpt; 1st time only */);
                 break;
@@ -1789,8 +1816,8 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                         slapi_task_log_notice(task, "%s: Indexing VLV: %s",
                                               inst->inst_name, attrs[i]+1);
                     }
-                    LDAPDebug(LDAP_DEBUG_ANY, "%s: Indexing VLV: %s\n",
-                              inst->inst_name, attrs[i]+1, 0);
+                    LDAPDebug2Args(LDAP_DEBUG_ANY, "%s: Indexing VLV: %s\n",
+                                   inst->inst_name, attrs[i]+1);
                 }
                 break;
             }
@@ -1911,7 +1938,8 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
             rc = get_value_from_string((const char *)data.dptr, "rdn", &rdn);
             if (rc) {
                 /* data.dptr may not include rdn: ..., try "dn: ..." */
-                ep->ep_entry = slapi_str2entry( data.dptr, 0 );
+                ep->ep_entry = slapi_str2entry( data.dptr, 
+                                                SLAPI_STR2ENTRY_NO_ENTRYDN );
             } else {
                 char *pid_str = NULL;
                 char *pdn = NULL;
@@ -1991,10 +2019,10 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                                                rdn, pdn?",":"", pdn?pdn:"");
                         slapi_ch_free_string(&pdn);
                     }
-                    /* dn is not dup'ed in slapi_sdn_new_dn_byref.
+                    /* dn is not dup'ed in slapi_sdn_new_dn_passin.
                      * It's set to bdn and put in the dn cache. */
                     /* don't free dn */
-                    sdn = slapi_sdn_new_dn_byref(dn);
+                    sdn = slapi_sdn_new_dn_passin(dn);
                     bdn = backdn_init(sdn, temp_id, 0);
                     myrc = CACHE_ADD( &inst->inst_dncache, bdn, NULL );
                     if (myrc) {
@@ -2010,7 +2038,8 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                     }
                 }
                 slapi_rdn_done(&psrdn);
-                ep->ep_entry = slapi_str2entry_ext( dn, data.dptr, 0 );
+                ep->ep_entry = slapi_str2entry_ext( dn, data.dptr, 
+                                                   SLAPI_STR2ENTRY_NO_ENTRYDN );
                 slapi_ch_free_string(&rdn);
             }
         } else {
@@ -3162,7 +3191,8 @@ _get_and_add_parent_rdns(backend *be,
                            "(rdn: %s, ID: %d) from Slapi_RDN\n", rdn, id);
             goto bail;
         }
-        ep->ep_entry = slapi_str2entry_ext( dn, data.dptr, 0 );
+        ep->ep_entry = slapi_str2entry_ext( dn, data.dptr, 
+                                            SLAPI_STR2ENTRY_NO_ENTRYDN );
         ep->ep_id = id;
         slapi_ch_free_string(&dn);
     }
@@ -3296,7 +3326,7 @@ _export_or_index_parents(ldbm_instance *inst,
                     int myrc = 0;
                     /* pdn is put in DN cache.  No need to free it here,
                      * since it'll be free'd when evicted from the cache. */
-                    psdn = slapi_sdn_new_dn_byref(pdn);
+                    psdn = slapi_sdn_new_dn_passin(pdn);
                     bdn = backdn_init(psdn, pid, 0);
                     myrc = CACHE_ADD(&inst->inst_dncache, bdn, NULL);
                     if (myrc) {
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
index 82fbbb2c..22f4ae0f 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -351,6 +351,12 @@ str2entry_fast( const char *rawdn, char *s, int flags, int read_stateinfo )
 			continue;
 		}
 
+		/* If SLAPI_STR2ENTRY_NO_ENTRYDN is set, skip entrydn */
+		if ( (flags & SLAPI_STR2ENTRY_NO_ENTRYDN) &&
+		     PL_strncasecmp( type.bv_val, "entrydn", type.bv_len ) == 0 ) {
+			if (freeval) slapi_ch_free_string(&value.bv_val);
+			continue;
+		}
 
 		/* retrieve uniqueid */
 		if ( PL_strncasecmp (type.bv_val, SLAPI_ATTR_UNIQUEID, type.bv_len) == 0 ){
@@ -854,6 +860,13 @@ str2entry_dupcheck( const char *rawdn, char *s, int flags, int read_stateinfo )
 		    continue;
 		}
 
+		/* If SLAPI_STR2ENTRY_NO_ENTRYDN is set, skip entrydn */
+		if ( (flags & SLAPI_STR2ENTRY_NO_ENTRYDN) &&
+		     strcasecmp( type, "entrydn" ) == 0 ) {
+			if (freeval) slapi_ch_free_string(&bvvalue.bv_val);
+			continue;
+		}
+
 		/* retrieve uniqueid */
 		if ( strcasecmp (type, SLAPI_ATTR_UNIQUEID) == 0 ){
 
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 8df6ec06..266fb931 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -786,6 +786,9 @@ void slapi_pblock_destroy( Slapi_PBlock *pb );
  *        \arg #SLAPI_STR2ENTRY_EXPAND_OBJECTCLASSES
  *        \arg #SLAPI_STR2ENTRY_NOT_WELL_FORMED_LDIF
  *        \arg #SLAPI_STR2ENTRY_NO_SCHEMA_LOCK
+ *        \arg #SLAPI_STR2ENTRY_USE_OBSOLETE_DNFORMAT
+ *        \arg #SLAPI_STR2ENTRY_NO_ENTRYDN
+ *
  * \return A pointer to the #Slapi_Entry structure representing the entry.
  * \return \c NULL if the string cannot be converted; for example, if no DN is
  *         specified in the string.
@@ -897,6 +900,15 @@ Slapi_Entry *slapi_str2entry_ext( const char *dn, char *s, int flags );
 #define SLAPI_STR2ENTRY_USE_OBSOLETE_DNFORMAT 512
 
 /**
+ * Do not include entrydn attribute value in the in-memory entry
+ *
+ * If this flag is set, entrydn in the source string is ignored.
+ *
+ * \see slapi_str2entry()
+ */
+#define SLAPI_STR2ENTRY_NO_ENTRYDN 1024
+
+/**
  * Generates a description of an entry as an LDIF string.
  *
  * This function behaves much like slapi_entry2str(); however, you can specify