44017 - document that ldap_urlsearch() isn't necessarily async

Status: ASSIGNED

(Directory :: LDAP C SDK, defect, P3)

Description

[Dan Mosedale (:dmosedale, :dmose)]

ldap_urlsearch() is asynchronous only in the case where a connection already 
exists and is bound.  otherwise, ldap_bind_s() is called down in 
nsldapi_new_connection().  The documentation that comes with the 4.0 C SDK 
doesn't seem to mention this; it would be nice if it did.

Comment 1

[Mark Smith [:mcs]]

Arguably, this is a bug in the sense that the bind is not required if you are 
using LDAPv3 (it is required with LDAPv2).  The ldap_urlsearch() function and 
related ones could really use some work in general since not all of the LDAP URL 
features are supported today (specifically, there options can appear at the end 
of the URL and one option for the bind DN has been defined).

In the short run you could use ldap_url_parse() and then call 
ldap_init() followed by ldap_search_ext() yourself.  But this does require 
creating a new LDAP session handle for each URL followed.

Another alternative would be to change the code in ldap_url_search() to skip the 
bind if LDAPv3 is in used.  E.g., change this line:

        err = nsldapi_send_server_request( ld, ber, msgid, NULL, srv,
            NULL, NULL, 1 );

to something like this:

        err = nsldapi_send_server_request( ld, ber, msgid, NULL, srv,
            NULL, NULL, ( NSLDAPI_LDAP_VERSION( ld ) < LDAP_VERSION3 ));

or a more readable choice:

        int  bind_required;

        bind_required = ( NSLDAPI_LDAP_VERSION( ld ) < LDAP_VERSION3 );

        ...

        err = nsldapi_send_server_request( ld, ber, msgid, NULL, srv,
            NULL, NULL, bind_required );

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I've actually already migrated to using ldap_init(), ldap_bind(), and 
ldap_search_ext() directly, and just checked that into directory/xpcom.

But if you consider the bug to be about the bind not being required, doesn't
that really mean that there is also a more general bug: the async API isn't 
really async unless ASYNC_CONNECT is used?

Comment 3

[Mark Smith [:mcs]]

"Async" is a slippery term.  For the LDAP API, historically it has meant "Don't
block waiting for a response from the LDAP server."  Of course ldap_url_search()
is broken even by this definition, and referral chasing may be as well.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Fair enough.  FWIW, when I saw the API referred to as "asynchronous", I assumed 
that meant I could call those functions from a UI thread and not be hung up in a 
networking function causing the UI to stall out.

Anyway, semantics aside, I'd love to hear your thoughts on where I should go 
with the XPCOM wrapper.  Specifically, I posted something in mozilla.directory 
yesterday which talks about the changes I checked in and this issue in 
particular.  If you'd care to comment on that, that'd be very helpful.

Comment 5

[Mark Smith [:mcs]]

TM = 5.14

Comment 6

[Jonathan]

rg ldap_urlsearch returns no results