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.