Perdition Users Mailing List ()

Simon Horman | 28 Oct 2008 01:47

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

On Mon, Oct 27, 2008 at 11:25:40AM +0000, Simon Fraser wrote:
> 
> I initially hit 'reply' rather than 'reply to all', so I'll re-send this
> to the list.
> 
> > I don't have an LDAP db handy to test against, so could
> > you please test the following change for me to see if
> > it helps your problem. If not, I'll dig deeper into the problem.
> 
> Many thanks for getting back to me so quickly.  I've patched a clean
> version of 1.17.1, and tested. 
> 
> > +     if (returns[0])
> > +             user_str = &returns[0];
> > +     if (returns[1])
> > +             server_str = &returns[1];
> > +     if (returns[2])
> > +             port_str = &returns[2];
> 
> The lookup was failing, and I've traced it to the lines above.  These
> pointers are directed to &returns, but a few lines later, all the values
> of returns are passed to free():
> 
>         for (count = 0; count < attrcount; count++)
>                 if (returns[count] != NULL)
>                         free(returns[count]);
>         free(returns);
> 
> I've replaced the lines quoted above with ones like:
>

(Continue reading)

headers

Simon Fraser | 28 Oct 2008 13:53

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.
>

(Continue reading)

headers

Simon Horman | 31 Oct 2008 03:25

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

On Tue, Oct 28, 2008 at 12:53:00PM +0000, Simon Fraser wrote:
> 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. 

Thanks, I have applied the change to hg.

http://hg.vergenet.net/perdition/perdition/rev/37c6aca9ee42

> > > 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

(Continue reading)