Ldap::addAttributes() fails without throwing if bind() has not been called

[mbaynton]

This one is bad. Other methods that require an ldap bind to have been made, like Ldap::search(), check if the connection is already bound and call Ldap::bind() if not, so users may be accustomed to skipping calling Ldap::bind() directly in user code. However, Ldap::addAttributes() does not do this, so if the Ldap instance doesn't already have a bound connection, we end up calling ldap_mod_add() with null as the ldap resource. Obviously, that doesn't work, but worse, ldap_mod_add returns null, not false, when it is given a null resource, and we only throw an exception if it returns false. So calling code gets no indication that something is wrong.

#68 and #73 as currently written are also subject to this bug.

I'll submit two PRs for this, one with only new tests that cover this case, and one with tests plus fix.

[mbaynton]

@heiglandreas current tests pass because they check that ldap_mod_add() is being passed null?!?

zend-ldap/test/OfflineTest.php

Line 200 in 0234654

$this->isNull(),

That should never be. Please advise how you'd like to proceed. You've not approved of any changes to existing tests in the past.

[heiglandreas]

thanks for noting and raising that issue! And I'd rather see that ldap_mod_add behaves the same as all the other methods and tries to bind when no connection is available.

I'm not yet sure how to handle that test but I'll check that today. Perhaps adding a new test for the fixed (right) behaviour and mark the current test as incomplete or skipped.

[mbaynton]

updateAttributes and deleteAttributes are also affected fwiw. I have subclassed Zend\Ldap locally to fix this for me anyways for now as I'm unsure what an acceptable patch would look like due to the broken existing test and @heiglandreas has assigned this to him as well.

[weierophinney]

This repository has been closed and moved to laminas/laminas-ldap; a new issue has been opened at laminas/laminas-ldap#2.