ITS#5210 Don't send cookie at end of refresh if nothing changed in refreshAndPersist

ITS#5211 spec says we must ignore invalid cookies

diff --git a/servers/slapd/overlays/syncprov.c b/servers/slapd/overlays/syncprov.c
index 49d0dd5db6ad0e174bab4ffe09dceed63ad4eaf6..f23aadc6a646d2195a5019191566a6e6c3af1f21 100644 (file)
--- a/servers/slapd/overlays/syncprov.c
+++ b/servers/slapd/overlays/syncprov.c
@@ -1814,7 +1814,9 @@ typedef struct searchstate {
        BerVarray ss_ctxcsn;
        int *ss_sids;
        int ss_numcsns;
-       int ss_present;
+#define        SS_PRESENT      0x01
+#define        SS_CHANGED      0x02
+       int ss_flags;
 } searchstate;
 
 static int
@@ -2003,26 +2005,32 @@ syncprov_search_response( Operation *op, SlapReply *rs )
        } else if ( rs->sr_type == REP_RESULT && rs->sr_err == LDAP_SUCCESS ) {
                struct berval cookie;
 
-               slap_compose_sync_cookie( op, &cookie, ss->ss_ctxcsn,
-                       srs->sr_state.rid, srs->sr_state.sid );
+               if ( ss->ss_flags & SS_CHANGED ) {
+                       slap_compose_sync_cookie( op, &cookie, ss->ss_ctxcsn,
+                               srs->sr_state.rid, srs->sr_state.sid );
 
-               Debug( LDAP_DEBUG_SYNC, "syncprov_search_response: cookie=%s\n", cookie.bv_val, 0, 0 );
+                       Debug( LDAP_DEBUG_SYNC, "syncprov_search_response: cookie=%s\n", cookie.bv_val, 0, 0 );
+               }
 
-               /* Is this a regular refresh? */
+               /* Is this a regular refresh?
+                * Note: refresh never gets here if there were no changes
+                */
                if ( !ss->ss_so ) {
                        rs->sr_ctrls = op->o_tmpalloc( sizeof(LDAPControl *)*2,
                                op->o_tmpmemctx );
                        rs->sr_ctrls[1] = NULL;
                        rs->sr_err = syncprov_done_ctrl( op, rs, rs->sr_ctrls,
-                               0, 1, &cookie, ss->ss_present ?  LDAP_SYNC_REFRESH_PRESENTS :
+                               0, 1, &cookie, ( ss->ss_flags & SS_PRESENT ) ?  LDAP_SYNC_REFRESH_PRESENTS :
                                        LDAP_SYNC_REFRESH_DELETES );
                        op->o_tmpfree( cookie.bv_val, op->o_tmpmemctx );
                } else {
                /* It's RefreshAndPersist, transition to Persist phase */
-                       syncprov_sendinfo( op, rs, ss->ss_present ?
+                       syncprov_sendinfo( op, rs, ( ss->ss_flags & SS_PRESENT ) ?
                                LDAP_TAG_SYNC_REFRESH_PRESENT : LDAP_TAG_SYNC_REFRESH_DELETE,
-                               &cookie, 1, NULL, 0 );
-                       op->o_tmpfree( cookie.bv_val, op->o_tmpmemctx );
+                               ( ss->ss_flags & SS_CHANGED ) ? &cookie : NULL,
+                               1, NULL, 0 );
+                       if ( ss->ss_flags & SS_CHANGED )
+                               op->o_tmpfree( cookie.bv_val, op->o_tmpmemctx );
 
                        /* Detach this Op from frontend control */
                        ldap_pvt_thread_mutex_lock( &ss->ss_so->s_mutex );
@@ -2050,7 +2058,7 @@ syncprov_op_search( Operation *op, SlapReply *rs )
        slap_overinst           *on = (slap_overinst *)op->o_bd->bd_info;
        syncprov_info_t         *si = (syncprov_info_t *)on->on_bi.bi_private;
        slap_callback   *cb;
-       int gotstate = 0, nochange = 0, do_present;
+       int gotstate = 0, changed = 0, do_present = 0;
        syncops *sop = NULL;
        searchstate *ss;
        sync_control *srs;
@@ -2065,8 +2073,6 @@ syncprov_op_search( Operation *op, SlapReply *rs )
                return rs->sr_err;
        }
 
-       do_present = si->si_nopres ? 0 : 1;
-
        srs = op->o_controls[slap_cids.sc_LDAPsync];
        op->o_managedsait = SLAP_CONTROL_NONCRITICAL;
 
@@ -2134,6 +2140,9 @@ syncprov_op_search( Operation *op, SlapReply *rs )
                if ( !numcsns )
                        goto no_change;
 
+               if ( !si->si_nopres )
+                       do_present = SS_PRESENT;
+
                /* If there are SIDs we don't recognize in the cookie, drop them */
                for (i=0; i<srs->sr_state.numcsns; ) {
                        for (j=0; j<numcsns; j++) {
@@ -2164,21 +2173,21 @@ syncprov_op_search( Operation *op, SlapReply *rs )
 
                /* If nothing has changed, shortcut it */
                if ( srs->sr_state.numcsns == numcsns ) {
-                       int i, j, changed = 0;
+                       int i, j;
                        for ( i=0; i<srs->sr_state.numcsns; i++ ) {
                                for ( j=0; j<numcsns; j++ ) {
                                        if ( srs->sr_state.sids[i] != sids[j] )
                                                continue;
                                        if ( !bvmatch( &srs->sr_state.ctxcsn[i], &ctxcsn[j] ))
-                                               changed = 1;
+                                               changed = SS_CHANGED;
                                        break;
                                }
                                if ( changed )
                                        break;
                        }
                        if ( !changed ) {
-no_change:             nochange = 1;
-                               if ( !(op->o_sync_mode & SLAP_SYNC_PERSIST) ) {
+                               do_present = 0;
+no_change:             if ( !(op->o_sync_mode & SLAP_SYNC_PERSIST) ) {
                                        LDAPControl     *ctrls[2];
 
                                        ctrls[0] = NULL;
@@ -2234,6 +2243,9 @@ no_change:                nochange = 1;
                                return rs->sr_err;
                        }
                }
+       } else {
+               /* No consumer state, assume something has changed */
+               changed = SS_CHANGED;
        }
 
 shortcut:
@@ -2245,7 +2257,7 @@ shortcut:
        }
 
        /* If something changed, find the changes */
-       if ( gotstate && !nochange ) {
+       if ( gotstate && changed ) {
                Filter *fand, *fava;
 
                fand = op->o_tmpalloc( sizeof(Filter), op->o_tmpmemctx );
@@ -2272,7 +2284,7 @@ shortcut:
        ss = (searchstate *)(cb+1);
        ss->ss_on = on;
        ss->ss_so = sop;
-       ss->ss_present = do_present;
+       ss->ss_flags = do_present | changed;
        ss->ss_ctxcsn = ctxcsn;
        ss->ss_numcsns = numcsns;
        ss->ss_sids = sids;
@@ -2286,7 +2298,7 @@ shortcut:
         * the refresh phase, just invoke the response callback to transition
         * us into persist phase
         */
-       if ( nochange ) {
+       if ( !changed ) {
                rs->sr_err = LDAP_SUCCESS;
                rs->sr_nentries = 0;
                send_ldap_result( op, rs );
@@ -2818,10 +2830,14 @@ static int syncprov_parseCtrl (
        sr->sr_rhint = rhint;
        if (!BER_BVISNULL(&cookie)) {
                ber_dupbv_x( &sr->sr_state.octet_str, &cookie, op->o_tmpmemctx );
+               /* If parse fails, pretend no cookie was sent */
                if ( slap_parse_sync_cookie( &sr->sr_state, op->o_tmpmemctx ) ||
                        sr->sr_state.rid == -1 ) {
-                       rs->sr_text = "Sync control : cookie parsing error";
-                       return LDAP_PROTOCOL_ERROR;
+                       if ( sr->sr_state.ctxcsn ) {
+                               ber_bvarray_free_x( sr->sr_state.ctxcsn, op->o_tmpmemctx );
+                               sr->sr_state.ctxcsn = NULL;
+                       }
+                       sr->sr_state.numcsns = 0;
                }
        }