From da12124ea1cc87187dcb664e5b3c07b0e6160afb Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Tue, 15 May 2018 16:45:56 +0200 Subject: [PATCH] Ticket 49658 - In repicated topology a single-valued attribute can diverge Bug Description: When deleting a specific value of a single valued attribute, the deleted value can be erronously resurrected. Fix Description: If the deleted value has not a vucsn, just zap the deleted value. additional fixes are: make sure to get the most recent deleted value (attr_most_recent_deleted_value). checks that resurrected deleted value had a more recent vucsn than current value https://pagure.io/389-ds-base/issue/49658 Reviewed by: ? Platforms tested: F26 Flag Day: no Doc impact: no --- ldap/servers/slapd/entrywsi.c | 85 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 11 deletions(-) diff --git a/ldap/servers/slapd/entrywsi.c b/ldap/servers/slapd/entrywsi.c index 58f7870ef..6bec8fc78 100644 --- a/ldap/servers/slapd/entrywsi.c +++ b/ldap/servers/slapd/entrywsi.c @@ -387,6 +387,20 @@ entry_add_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **b return retVal; } +/* Used for debug purpose, it dumps into the error log the + * entry with the replication stateinfo + */ +static void +entry_dump_stateinfo(char *msg, Slapi_Entry* e) +{ + char *s; + int len = 0; + + s = slapi_entry2str_with_options(e, &len, SLAPI_DUMP_STATEINFO); + slapi_log_err(SLAPI_LOG_ERR, msg, "%s\n", s); + slapi_ch_free((void **)&s); +} + static int entry_add_present_values_wsi_single_valued(Slapi_Entry *e, const char *type, struct berval **bervals, const CSN *csn, int urp, long flags) { @@ -467,7 +481,7 @@ entry_add_present_values_wsi_single_valued(Slapi_Entry *e, const char *type, str } valuearray_update_csn(valuestoadd, CSN_TYPE_VALUE_UPDATED, csn); retVal = attr_add_valuearray(a, valuestoadd, slapi_entry_get_dn_const(e)); - } + } a->a_flags = a_flags_orig; } done: @@ -705,6 +719,10 @@ entry_delete_present_values_wsi_single_valued(Slapi_Entry *e, const char *type, /* The attribute is single valued and the value was successful deleted */ /* but there could have been an add in the same operation, so double check */ if (valueset_isempty(&a->a_present_values)) { + /* A doubt here, a direct update deletes the last value + * of a single valued attribute. It will only contain deleted values. + * Why not setting the adcsn (attr_set_deletion_csn) ? + */ entry_present_attribute_to_deleted_attribute(e, a); } } else if (retVal != LDAP_SUCCESS) { @@ -1229,6 +1247,36 @@ resolve_attribute_state_present_to_deleted(Slapi_Entry *e, Slapi_Attr *a, Slapi_ } } +/* Retrieve from the deleted values the one that + * was the most recently deleted. Based on its vdcsn + */ +static Slapi_Value * +attr_most_recent_deleted_value(Slapi_Attr *a) +{ + Slapi_Value *v, *most_recent_v; + int i; + const CSN *vdcsn, *most_recent_vdcsn; + char csn_string[100]; + + memset(csn_string, 0, sizeof (csn_string)); + + vdcsn = NULL; + most_recent_vdcsn = NULL; + i = attr_first_deleted_value(a, &v); + most_recent_v = v; + + while (i != -1) { + vdcsn = value_get_csn(v, CSN_TYPE_VALUE_DELETED); + + if (csn_compare(most_recent_vdcsn, vdcsn) < 0) { + most_recent_v = v; + most_recent_vdcsn = vdcsn; + } + i = attr_next_deleted_value(a, i, &v); + } + return most_recent_v; +} + static void resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribute_state) { @@ -1249,7 +1297,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu if (i != -1) { slapi_attr_next_value(a, i, &new_value); } - attr_first_deleted_value(a, &pending_value); + pending_value = attr_most_recent_deleted_value(a); /* purge_attribute_state_single_valued */ adcsn = attr_get_deletion_csn(a); current_value_vucsn = value_get_csn(current_value, CSN_TYPE_VALUE_UPDATED); @@ -1310,15 +1358,30 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu } else if (new_value == NULL) { /* check if the pending value should become the current value */ if (pending_value != NULL) { - if (!value_distinguished_at_csn(e, a, current_value, pending_value_vucsn)) { - /* attribute.current_value = attribute.pending_value; */ - /* attribute.pending_value = NULL; */ - entry_present_value_to_zapped_value(a, current_value); - entry_deleted_value_to_present_value(a, pending_value); - current_value = pending_value; + if (pending_value_vucsn) { + /* "resurrect the pending value at the condition the current one is not distinguished + * and the pending value vucsn > current value vucsn + */ + if (!value_distinguished_at_csn(e, a, current_value, pending_value_vucsn) && + (csn_compare(current_value_vucsn, pending_value_vucsn) < 0)) { + /* attribute.current_value = attribute.pending_value; */ + /* attribute.pending_value = NULL; */ + entry_present_value_to_zapped_value(a, current_value); + entry_deleted_value_to_present_value(a, pending_value); + current_value = pending_value; + current_value_vucsn = pending_value_vucsn; + + pending_value = NULL; + pending_value_vucsn = NULL; + pending_value_vdcsn = NULL; + } + } else { + /* the pending value vucsn is NULL and this value is pending (aka deleted) -> zap it */ + entry_deleted_value_to_zapped_value(a, pending_value); pending_value = NULL; - current_value_vucsn = pending_value_vucsn; + pending_value_vdcsn = NULL; pending_value_vucsn = NULL; + } } /* check if the current value should be deleted */ @@ -1360,7 +1423,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu } } else /* new value is after the current value */ { - if (!value_distinguished_at_csn(e, a, current_value, new_value_vucsn)) { + if (!value_distinguished_at_csn(e, a, current_value, new_value_vucsn)) { /* attribute.current_value = new_value */ entry_present_value_to_zapped_value(a, current_value); current_value = new_value; @@ -1387,7 +1450,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu /* purge_attribute_state_single_valued */ if ((pending_value != NULL && (csn_compare(adcsn, pending_value_vucsn) < 0)) || (pending_value == NULL && (csn_compare(adcsn, current_value_vucsn) < 0))) { - attr_set_deletion_csn(a, NULL); + attr_set_deletion_csn(a, NULL); adcsn = NULL; } -- 2.13.6