From a68952f9a8e6d3ff9fea1b4b485acc63f77b3ce6 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Apr 29 2013 16:59:27 +0000 Subject: Ticket #529 - dn normalization must handle multiple space characters in attributes Bug Description: Commit 69ff83598d517bed84922b1c7dd67cab023b4d99 had a flaw -- handling normdn in upgradedn_producer had a problem. The string was passed to the Slapi_DN in the entry using slapi_sdn_init_dn_passin, while the string could be modified at other places. Fix Description: This patch manages the normdn string more carefully. https://fedorahosted.org/389/ticket/529 Reviewed by Rich (Thank you!!) --- diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index 3cb25d3..79a5984 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -1523,6 +1523,7 @@ upgradedn_producer(void *param) finished = 0; while (!finished) { ID temp_id; + int dn_in_cache; if (job->flags & FLAG_ABORT) { goto error; @@ -1580,6 +1581,7 @@ upgradedn_producer(void *param) do_dn_norm = 0; do_dn_norm_sp = 0; rdn_has_spaces = 0; + dn_in_cache = 0; if (entryrdn_get_switch()) { /* original rdn is allocated in get_value_from_string */ @@ -1591,10 +1593,12 @@ upgradedn_producer(void *param) } else { bdn = dncache_find_id(&inst->inst_dncache, temp_id); if (bdn) { - /* don't free dn */ + /* don't free normdn */ normdn = (char *)slapi_sdn_get_dn(bdn->dn_sdn); CACHE_RETURN(&inst->inst_dncache, &bdn); + dn_in_cache = 1; } else { + /* free normdn */ rc = entryrdn_lookup_dn(be, rdn, temp_id, (char **)&normdn, NULL, NULL); if (rc) { @@ -1639,6 +1643,7 @@ upgradedn_producer(void *param) continue; } } + /* free normdn */ normdn = slapi_ch_smprintf("%s%s%s", rdn, pdn?",":"", pdn?pdn:""); slapi_ch_free_string(&pdn); @@ -1648,14 +1653,18 @@ upgradedn_producer(void *param) * we need to put the new value to cache.*/ /* dn is dup'ed in slapi_sdn_new_dn_byval. * It's set to bdn and put in the dn cache. */ - sdn = slapi_sdn_new_normdn_byref(normdn); + /* normdn is allocated in this scope. + * Thus, we can just passin. */ + sdn = slapi_sdn_new_normdn_passin(normdn); bdn = backdn_init(sdn, temp_id, 0); CACHE_ADD( &inst->inst_dncache, bdn, NULL ); CACHE_RETURN(&inst->inst_dncache, &bdn); + /* don't free this normdn */ normdn = slapi_sdn_get_dn(sdn); slapi_log_error(SLAPI_LOG_CACHE, "uptradedn", "entryrdn_lookup_dn returned: %s, " "and set to dn cache\n", normdn); + dn_in_cache = 1; } } e = slapi_str2entry_ext(normdn, NULL, data.dptr, @@ -1691,11 +1700,7 @@ upgradedn_producer(void *param) * -- normalize it with the new format */ if (!normdn) { - get_value_from_string((const char *)ecopy, "dn", (char **)&normdn); - } - if (normdn) { - slapi_sdn_done(&(e->e_sdn)); - slapi_sdn_init_dn_passin(&(e->e_sdn), normdn); + /* No rdn in id2entry or entrydn */ normdn = slapi_sdn_get_dn(&(e->e_sdn)); } @@ -1807,13 +1812,16 @@ upgradedn_producer(void *param) normdn, temp_id, alt_id); LDAPDebug2Args(LDAP_DEBUG_ANY, "Renaming \"%s\" to \"%s\"\n", rdn, newrdn); + if (!dn_in_cache) { + /* If not in dn cache, normdn needs to be freed. */ + slapi_ch_free_string(&normdn); + } normdn = slapi_ch_smprintf("%s,%s", newrdn, parentdn); slapi_ch_free_string(&newrdn); slapi_ch_free_string(&parentdn); /* Reset DN and RDN in the entry */ slapi_sdn_done(&(e->e_sdn)); - slapi_sdn_init_normdn_passin(&(e->e_sdn), normdn); - /* normdn = slapi_sdn_get_dn(&(e->e_sdn)); */ + slapi_sdn_init_normdn_byval(&(e->e_sdn), normdn); } info_state |= DN_NORM_SP; upgradedn_add_to_list(&ud_list, @@ -1834,12 +1842,14 @@ upgradedn_producer(void *param) * It's set to bdn and put in the dn cache. */ /* Waited to put normdn into dncache until it could be modified in * chk_dn_norm_sp. */ - sdn = slapi_sdn_new_normdn_byref(normdn); - bdn = backdn_init(sdn, temp_id, 0); - CACHE_ADD(&inst->inst_dncache, bdn, NULL); - CACHE_RETURN(&inst->inst_dncache, &bdn); - slapi_log_error(SLAPI_LOG_CACHE, "uptradedn", - "set dn %s to dn cache\n", normdn); + if (!dn_in_cache) { + sdn = slapi_sdn_new_normdn_passin(normdn); + bdn = backdn_init(sdn, temp_id, 0); + CACHE_ADD(&inst->inst_dncache, bdn, NULL); + CACHE_RETURN(&inst->inst_dncache, &bdn); + slapi_log_error(SLAPI_LOG_CACHE, "uptradedn", + "set dn %s to dn cache\n", normdn); + } /* Check DN syntax attr values if it contains '\\' or not */ /* Start from the rdn */ if (chk_dn_norm) { diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c index 3043ee6..9261bd8 100644 --- a/ldap/servers/slapd/dn.c +++ b/ldap/servers/slapd/dn.c @@ -1915,6 +1915,22 @@ slapi_sdn_init_normdn_byref(Slapi_DN *sdn, const char *dn) } else { sdn->ndn_len = strlen(dn); sdn->dn = dn; + sdn->flag = slapi_unsetbit_uchar(sdn->flag, FLAG_DN); + } + return sdn; +} + +/* use when dn is already normalized (but case is yet touched) */ +Slapi_DN * +slapi_sdn_init_normdn_byval(Slapi_DN *sdn, const char *dn) +{ + slapi_sdn_init(sdn); + if(dn == NULL) { + sdn->ndn_len = 0; + } else { + sdn->ndn_len = strlen(dn); + sdn->dn= slapi_ch_strdup(dn); + sdn->flag = slapi_setbit_uchar(sdn->flag, FLAG_DN); } return sdn; } diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index a77c9f4..49ecb52 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -384,6 +384,7 @@ Slapi_DN *slapi_sdn_init_dn_passin(Slapi_DN *sdn,const char *dn); Slapi_DN *slapi_sdn_init_ndn_byref(Slapi_DN *sdn,const char *dn); Slapi_DN *slapi_sdn_init_ndn_byval(Slapi_DN *sdn,const char *dn); Slapi_DN *slapi_sdn_init_normdn_byref(Slapi_DN *sdn, const char *dn); +Slapi_DN *slapi_sdn_init_normdn_byval(Slapi_DN *sdn, const char *dn); Slapi_DN *slapi_sdn_init_normdn_ndn_passin(Slapi_DN *sdn, const char *dn); Slapi_DN *slapi_sdn_init_normdn_passin(Slapi_DN *sdn, const char *dn); char *slapi_dn_normalize_original( char *dn );