223603 - if the LDAP host is specified as multiple space-delimited hosts, ldap fails

Status: NEW

(MailNews Core :: LDAP Integration, defect)

Description

[David :Bienvenu]

If the user/client specifies multiple space delimited ldap hosts, ldap lookups
fail. 4.x would use the first host, and then try the second if the dns lookup
failed on the first...

Comment 1

[David :Bienvenu]

Attachment: proposed fix

this fix makes it so we'll at least connect to the first host. Down the road, I
need to fix this code to try multiple hosts. But for the near term, I need this
to work at least a little.

Comment 2

[David :Bienvenu]

Comment on attachment 134068 [details] [diff] [review]
proposed fix

Dan, this is just temporary, but we need it for now.

Comment 3

[Scott MacGregor]

two more small comments, before you checkin:

1) "ldap c-dk" should be "ldap c-sdk"
2) -1 should be kNotFound

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 134068 [details] [diff] [review]
proposed fix

sr=dmose, but please adjust the comment a bit: "but now it fails completely"
will actually refer to pre-bugfix code, which is confusing for the reader.

Comment 5

[David :Bienvenu]

Attachment: fix addressing comments

Comment 6

[David :Bienvenu]

Comment on attachment 134091 [details] [diff] [review]
fix addressing comments

carrying forward reviews

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 134091 [details] [diff] [review]
fix addressing comments

a=asa (on behalf of drivers) for checkin to 1.6alpha

Comment 8

[David :Bienvenu]

fix to make it not fail checked in - I'll leave this open to make multiple hosts
actually work.

Comment 9

[David :Bienvenu]

Here's the plan for making multiple hosts work:

Because we don't pass the host name directly into the ldap c-sdk ldap_init
function, but rather, do a dns lookup on the host and pass the resulting ip
address(es) into ldap_init(), we need to extend the code to look up multiple dns
hosts, and wait for the dns lookups to complete before calling ldap_init with
the ip addresses of all the hosts. We do all this because the ldap c-sdk does
blocking dns lookups, which blocks the UI. We believe the current trunk c-sdk
does async dns lookups, but we're using an older version which does not. The
long term plan should be to use the newer version of the c-sdk, but that seems
risky in the short term.

The draw back of doing multiple dns lookups is that we'll likely end up doing
more dns lookups than we need, if you assume the first host is up and running.
My guess is that the ldap c-sdk would only do one dns lookup in that case, but
we'll end up doing them all. But I think that's the price they're going to have
to pay until we can get to the newer version of the c-sdk.