Re: bug: Partial results from DB lookup causing parsing problems

On Tue, 2008-10-28 at 11:47 +1100, Simon Horman wrote:

> Sorry about that. I think that your fix is ok, though
> I think I will go with the change below that just makes
> sure the strings aren't freed if we make it to the bottom
> cleanly.
> 
> The key is the following snippet
> 
>        leave:
> -       if (returns) {
> +       if (returns && status) {
>                 for (count = 0; count < attrcount; count++)

That does seem a lot cleaner - thank you. 

> > Incidentally, I've ventured into nasty kludge territory to work around a
> > bug that appears not to be in your code, but either in libldap or
> > libldap's interaction with Active Directory.  ldap_init, ldap_set_option
> > and ldap_bind_s are all successful, but ldap_search_s does not return
> > LDAP_SUCCESS.  Instead, it returns LDAP_SERVER_DOWN, _even if_ the
> > search was successful. It seems insane, but if I comment out the check
> > for (err != LDAP_SUCCESS) and just check for returned attributes, all
> > the data is there.  I can replicate this in my own test program using
> > libldap both from debian-etch (2.1.30) and the openldap-2.4.11 source
> > package, so I'm confident the problem isn't in your code, but it's
> > probably worth knowing about.
> 
> That does sound bizarre - but believable.
> 
> Could you send a patch just to confirm the change / workaround that you are
> suggesting?

With regards Active Directory, I'm currently uncertain.  I've been
examining the openldap source, and as best I can determine it's a
prematurely closed connection.  There are a few odd conditions in the
openldap source where a LDAP_SUCCESS is replaced with LDAP_SERVER_DOWN,
but it's getting to be heavy going working it all out. Experimenting
with with ldap_search_st/ldap_search_ext_s hasn't provided any results.

The only ideas I've had that don't involve a lot of further testing is
to either perform an additional query to determine the server type, or
to assume that if the init and bind succeed, then a response of
LDAP_SERVER_DOWN immediately afterwards is bogus, and only report on
other error conditions.   While I'm happy to run with this patch on my
local installation, it seems like the sort of thing that will come back
to bite later on. 

Simon.


--- perditiondb_ldap.c  2008-10-28 11:50:28.263292881 +0000
+++ perditiondb_ldap.c.new      2008-10-28 11:51:53.342721470 +0000
@@ -391,7 +391,7 @@
        /* Perform the search */
        err = ldap_search_s(connection, lud->lud_dn, lud->lud_scope,
                           lud->lud_filter, lud->lud_attrs, 0, &res);
-       if (err != LDAP_SUCCESS) {
+       if (err != LDAP_SUCCESS && err != LDAP_SERVER_DOWN) {
                VANESSA_LOGGER_DEBUG_UNSAFE("ldap_search_s: %s",
                                ldap_err2string(err));
                goto leave;




-- 
 The Wellcome Trust Sanger Institute is operated by Genome Research 
 Limited, a charity registered in England with number 1021457 and a 
 company registered in England with number 2742969, whose registered 
 office is 215 Euston Road, London, NW1 2BE. 
______________________________________________
Perdition-users mailing list
[email protected]
http://lists.vergenet.net/listinfo/perdition-users