Re: (ITS#6785) slapo-chain mishandles sr_text/sr_matched

[masarati@aero.polimi.it]

> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS:
> URL:
> Submission from: (NULL) (193.157.198.89)
> Submitted by: hallvard
>
>
> ldap_chain_response() can send sr_text/sr_matched which refers to a
> different error than sr_err and rc.
>
> It saves rs-> sr_err, sr_text, sr_matched to three local variables and
> can restore them later, but not maintain the text and matched variables
> in parallel with err.  Also it does not track/reset/obey
> REP_MATCHED_MUSTBEFREED along with sr_matched.
>
> Fails test032-chain with the asserts below.  However there may be
> non-success result with the wrong text/matched too, that cannot be
> assert()ed.
>
> Come to think of it, maybe the last issues applies to ITS#6774 too?
> REP_MATCHED_MUSTBEFREED, mismatch between failure code and text/matched.
>
> Index: servers/slapd/back-ldap/chain.c
> @@ -1019,4 +1019,14 @@ ldap_chain_response( Operation *op, SlapReply *rs )
>  	rs->sr_ref = NULL;
>
> +	const char *bad_incoming_matched = NULL, *bad_incoming_text = NULL;
> +	switch ( sr_err ) {
> +	case LDAP_SUCCESS:
> +	case LDAP_COMPARE_TRUE:
> +	case LDAP_COMPARE_FALSE:
> +		bad_incoming_matched = matched;
> +	case LDAP_REFERRAL:
> +		bad_incoming_text = text;
> +	}
> +
>  	/* we need this to know if back-ldap returned any result */
>  	lb.lb_lc = lc;
> @@ -1169,4 +1179,15 @@ cannot_chain:;
>
>  dont_chain:;
> +	switch ( sr_err ) {
> +	case LDAP_SUCCESS:
> +	case LDAP_COMPARE_TRUE:
> +	case LDAP_COMPARE_FALSE:
> +		assert( !matched || bad_incoming_matched );
> +	case LDAP_REFERRAL:
> +		assert( !text || bad_incoming_text );
> +	}
> +	assert( rc != LDAP_SUCCESS ||
> +			(( !text    || bad_incoming_text) &&
> +			 ( !matched || bad_incoming_matched )));
>  	rs->sr_err = sr_err;
>  	rs->sr_type = sr_type;

What do you suggest?  To use a local SlapReply instead of hijacking the
one passed as argument?

p.