[python-ldap] segfault/Abort for operations with unbound connection

[Space One]

Sorry but that change SUCKS!

Now we received tracebacks from our customers with this error:

  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 918,
in search_ext_s
    return
self._apply_method_s(SimpleLDAPObject.search_ext_s,*args,**kwargs)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 865,
in _apply_method_s
    self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 843,
in reconnect
    SimpleLDAPObject.unbind_s(self)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 600,
in unbind_s
    return self.unbind_ext_s(None,None)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 591,
in unbind_ext_s
    msgid = self.unbind_ext(serverctrls,clientctrls)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 588,
in unbind_ext
    return
self._ldap_call(self._l.unbind_ext,RequestControlTuples(serverctrls),RequestControlTuples(clientctrls))
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 136,
in __getattr__
    self.__class__.__name__,repr(name)
AttributeError: ReconnectLDAPObject has no attribute '_l'

We can't handle all AttributeError's - 1. it could hide a lot of other
errors. 2. It wouldn't be future-proof if the code changes somewhen and
raise something else (e.g. TypeError due to NoneType) instead or the
variable '_l' changes so that one cannot inspect the frame/str(exc).

Also the question here is why the reconnection did fail but didn't raise
anything (our code doesn't call unbind() anywhere!).

Best regards
spaceone

Am 26.01.2016 um 19:11 schrieb Petr Viktorin:
> On 01/26/2016 04:40 PM, Michael Ströder wrote:
>> HI!
>>
>> (please stay on python-ldap list so others comment as well)
>>
>> Space One wrote:
>>> Am 26.01.2016 um 16:21 schrieb Michael Ströder:
>>>> Space One wrote:
>>>>> AttributeError is IMHO not good, please better raise ldap.SERVER_DOWN so
>>>>> that one can handle it.
>>>> I consider ldap.SERVER_DOWN to be inappropriate in this case because it
>>>> indicates an error the application can recover from by proper reconnect.
>>>>
>>>> But in case the application calls some LDAPObject method *after* unbinding then
>>>> the application's code has to be fixed. This is a different case!
>>> We are speaking about ldap.ldapobject.ReconnectLDAPObject, right?
>> Not limited to ReconnectLDAPObject.
>>
>>> This class somehow unbind()s itself.
>> I never experienced such a seg fault when using ReconnectLDAPObject and I'm
>> using it a lot. Therefore I assume there's no problem inside this class.
>> Especially it won't call any LDAPObject method after internal unbind without
>> internally setting up new connection object.
>>
>>> If you aren't okay with ldap.SERVER_DOWN please don't use AttributeError
>>> but some new exception like ldap.UNBOUND_CONNECTION.
>>>
>>> As the intial thread says I want a way to clearly be able to
>>> differentitate if the connection is still valid and handle things if
>>> not. AttributeError would not be nice in that scenario as I don't want
>>> to catch all AttributeErrors.
>> Your application MUST NOT catch this exception and simply re-try!
>>
>> Your application MUST avoid to use an LDAPObject instance after calling
>> LDAPObject.unbind_ext() and friends! So if this (unhandled) exception ever
>> happens it indicates a bug in your application logic and you have to fix your
>> calling code.
>>
>> The only thing python-ldap did wrong is the seg fault.
>>
>> Ciao, Michael.
> Indeed. The only code that should be catching this exception is the test
> that verifies python-ldap doesn't segfault. That test could be a bit
> more specific if the exception was not a generic AttributeError.
>
> Also, a generic exception means any code that expects AttributeError
> will now catch this as well. Not too likely in good code, but one more
> reason to use a specific exception.
>
> I'm not insisting though.
>