Re: (ITS#6103) canceled operations do not respond

[h.b.furuseth@usit.uio.no]

hyc@symas.com writes:
>Hallvard B Furuseth wrote:
>> The send_ldap_response() fix is still incomplete:

[Reversing the quoteed message]

>> Also, this:
>> 	rc = rs->sr_err;
>> 	if ( rc == SLAPD_ABANDON&&  op->o_cancel )
>> 		rc = LDAP_CANCELLED;
>> will send a negative result code (SLAPD_ABANDON) if !o_cancel.
> 
> No, it won't. That is all bypassed at the top of the function.

If so, the quoted op->o_cancel test could be skipped.  But an overlay
could set rs->sr_err = SLAPD_ABANDON during slap_response_play().

Though that ought to mean o_abandon is set, so send_ldap_ber() will drop
the message.  So you're right after all that it won't be sent.

>> Note, not just when the function itself sets rc = LDAP_CANCELLED,
>> since syncprov can set rs->sr_err = LDAP_CANCELLED too.
>> Which also suggests the assert( rs->sr_err == LDAP_REFERRAL ) when
>> sr_ref != NULL is rather dubious.  Maybe sr_ref should just
>> be ignored when sending something else than LDAP_REFERRAL.
> 
> Leave the assert() in place until you have proof one way or the other.

Haven't tested with an example of an overlay setting SLAPD_ABANDON;
could check syncprov or retcode I suppose.  Faked it with
	nanosleep(&((struct timespec){0,2E8}), 0); /*gcc or C99 code*/
	if ( op->o_abandon ) rs->sr_err = SLAPD_ABANDON;
after the op->o_callback->sc_response in slap_response_play().
Then, with this slapd.conf:
    include	servers/slapd/schema/core.schema
    referral	ldap://elsewhere/
    database	frontend
    overlay	valsort
send
    Delete "cn=urgle"
    <wait 0.1 second>
    Abandon <previous request>
lt-slapd: result.c:479: send_ldap_response:
    Assertion `rs->sr_err == 0x0a' failed.


One more detail, looking at result.c rev 1.330 send_ldap_response():
I presume the LDAP_CONNECTIONLESS branch should also ber_printf rc
rather than rs->sr_err to the response.  Not sure if Cancel makes sense
in cldap (and after V2 Bind which has no extended operation at that),
but OpenLDAP does let through Cancel after a V2 Bind.

>> When rc == LDAP_CANCELLED, the function should not send sr_matched,
>> sr_text, sr_ref, and sr_ctrls.  Looking at how Abandon works, it seems
>> slap_cleanup_play() should still see them if they are not sent?
> 
> Dunno what you mean.

I thought that was obvious, but since it isn't maybe I should ask
ldapext instead.  Cancel does seem underspecified in this regard.

Anyway, I meant: Either an operation gets cancelled/abandoned, or it
doesn't and comletes.  If these fields are set, they most likely relate
to a completed operation.  Nowhere SLAPD_ABANDON gets set, are the
others set - possibly except with back-meta/back-ldap, I haven't looked
too closely.  OTOH, code setting SLAPD_ABANDON rarely _clears_ those
fields, so I imagine they can convey information which would have been
removed (access controls maybe?)  or updated before they were sent, if
the operation had completed.

Clearing a critical control can be a bit doubtful, I'll grant.  But for
not obeying critical controls it's a choice between which error code to
return - unavailableCriticalExtension, cancelled, or something else.  Or
some controls could mark the operation as "uncancellable" (another flag
needed...)

But come to think of it, an Extended Response has some of the same
problem.  By the above reasoning, responseName and responseValue should
be dropped in a Cancelled response, as when sending protocolError to an
unrecognized reqest.  Might be a bit extreme.

-- 
Hallvard