From 88d6e045b84170ae51929f3ca72f263f96e5eda1 Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkinder@redhat.com>
Date: Apr 28 2011 20:57:51 +0000
Subject: Bug 695779 - windows sync can lose old values when a new value is added


When a single modify operation with multiple mods is played back to
AD, it is possible for some old values to be lost.  We check if a
mod is necessary to send to AD to detect loops, and skip sending
operations or values that are not necessary.  This doesn't take
prior mods in the same modify operation into account though, which
can make the results incorrect.

This patch makes the sync code keep a running copy of the AD entry
that the mods are applied to.  We use this copy to check which mods
can be skipped and which we must send.
(cherry picked from commit 2be27d35da9fdf6d5f35ff4fb94906f4fe193133)

---

diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index 428f5f1..daa35cc 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -2051,12 +2051,11 @@ windows_delete_local_entry(Slapi_DN *sdn){
   error message to that effect.
 */
 static int
-mod_already_made(Private_Repl_Protocol *prp, Slapi_Mod *smod)
+mod_already_made(Private_Repl_Protocol *prp, Slapi_Mod *smod, Slapi_Entry *ad_entry)
 {
 	int retval = 0;
 	int op = 0;
 	const char *type = NULL;
-	const Slapi_Entry *ad_entry = windows_private_get_raw_entry(prp->agmt);
 
 	if (!slapi_mod_isvalid(smod)) { /* bogus */
 		slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
@@ -2143,6 +2142,15 @@ mod_already_made(Private_Repl_Protocol *prp, Slapi_Mod *smod)
 						agmt_get_long_name(prp->agmt), op);
 	}
 
+	/* If the mod shouldn't be skipped, we should
+	 * apply it to the entry that was passed in.  This
+	 * allows a single modify operation with multiple
+	 * mods to take prior mods into account when
+	 * determining what can be skipped. */
+	if (retval == 0) {
+		slapi_entry_apply_mod(ad_entry, slapi_mod_get_ldapmod_byref(smod));
+	}
+
 	return retval;
 }
 
@@ -2426,10 +2434,18 @@ windows_map_mods_for_replay(Private_Repl_Protocol *prp,LDAPMod **original_mods, 
 	LDAPMod *mod = NULL;
 	int is_nt4 = windows_private_get_isnt4(prp->agmt);
 	Slapi_Mod *mysmod = NULL;
+	const Slapi_Entry *ad_entry = NULL;
+	Slapi_Entry *ad_entry_copy = NULL;
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "=> windows_map_mods_for_replay\n", 0, 0, 0 );
 
 	/* Iterate through the original mods, looking each attribute type up in the maps for either user or group */
+
+	/* Make a copy of the AD entry. */
+	ad_entry = windows_private_get_raw_entry(prp->agmt);
+	if (ad_entry) {
+		ad_entry_copy = slapi_entry_dup(ad_entry);
+	}
 	
 	slapi_mods_init_byref(&smods, original_mods);
 	slapi_mods_init(&mapped_smods,10);
@@ -2591,7 +2607,7 @@ windows_map_mods_for_replay(Private_Repl_Protocol *prp,LDAPMod **original_mods, 
 			}
 		}
 		/* Otherwise we do not copy this mod at all */
-		if (mysmod && !mod_already_made(prp, mysmod)) { /* make sure this mod is still valid to send */
+		if (mysmod && !mod_already_made(prp, mysmod, ad_entry_copy)) { /* make sure this mod is still valid to send */
 			slapi_mods_add_ldapmod(&mapped_smods, slapi_mod_get_ldapmod_passout(mysmod));
 		}
 		if (mysmod) {
@@ -2600,6 +2616,8 @@ windows_map_mods_for_replay(Private_Repl_Protocol *prp,LDAPMod **original_mods, 
 			
 		mod = slapi_mods_get_next_mod(&smods);
 	}
+
+	slapi_entry_free(ad_entry_copy);
 	slapi_mods_done (&smods);
 	/* Extract the mods for the caller */
 	*returned_mods = slapi_mods_get_ldapmods_passout(&mapped_smods);
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
index 9e0b57f..a945485 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -3176,6 +3176,14 @@ slapi_entry_apply_mods( Slapi_Entry *e, LDAPMod **mods )
 	return entry_apply_mods(e, mods);
 }
 
+/*
+ * Apply a single mod to an entry
+ */ 
+int slapi_entry_apply_mod( Slapi_Entry *e, LDAPMod *mod )
+{
+	return entry_apply_mod(e, mod);
+}
+
 int
 entry_apply_mods( Slapi_Entry *e, LDAPMod **mods )
 {
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 8531fbe..898e0f8 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -1978,7 +1978,7 @@ int slapi_entry_delete_string(Slapi_Entry *e, const char *type, const char *valu
 void slapi_entry_diff(Slapi_Mods *smods, Slapi_Entry *e1, Slapi_Entry *e2, int diff_ctrl);
 
 /**
- * Applies an array of \c LDAPMod modifications a Slapi_Entry.
+ * Applies an array of \c LDAPMod modifications to a Slapi_Entry.
  *
  * \param e Entry to which you want to apply the modifications.
  * \param mods \c NULL terminated array of \c LDAPMod modifications that you
@@ -1991,6 +1991,19 @@ void slapi_entry_diff(Slapi_Mods *smods, Slapi_Entry *e1, Slapi_Entry *e2, int d
 int slapi_entry_apply_mods(Slapi_Entry *e, LDAPMod **mods);
 
 /**
+ * Applies a single \c LDAPMod modification to a Slapi_Entry.
+ *
+ * \param e Entry to which you want to apply the modification.
+ * \param mod A pointer to the \c LDAPMod modification that you
+ *             want to apply to the specified entry.
+ * \return \c LDAP_SUCCESS if the mod applied to the entry cleanly, otherwise
+ *         an LDAP error is returned.
+ * \warning It is up to the caller to free the \c LDAPMod after the mod has
+ *          been applied.
+ */
+int slapi_entry_apply_mod(Slapi_Entry *e, LDAPMod *mod);
+
+/**
  * Renames a Slapi_Entry.
  *
  * This function will rename an existing \c Slapi_Entry, similar to what