activeldap/activeldap — pull request #50 by pwillred (2013-02-02 03:08:22+00:00)
Title: SystemStackError during SASL bind to a unresponsive LDAP server

When attempting to bind using SASL to an LDAP server that is down (i.e., the server responds to the connection request with a TCP RST), activeldap causes a SystemStackError, rather than raising an ActiveLdap::ConnectionError as suggested by the documentation. The gem appears to enter a recursive cycle in calls to reconnect(), resulting in the error. A small portion of the stack trace follows at the end of this issue. I am using activeldap from HEAD on github, Ruby/LDAP 0.9.12, and ruby 1.9.3p327.

It appears that the initial call to schema() results in a search request that is sent to adapter/base.rb's operation(). (The following functions are all in adapter/base.rb) Operation() then detects that the server is not connected, and attempts to reconnect in a call to reconnect(). reconnect() is implemented with a loop that calls connect() until a fatal error is encountered, or the maximum number of attempts specified in retry_limit is reached. This all appears to behave as expected, without any major issues.

However, connect() is implemented with a call to bind() at the end. For servers that use SASL, bind() calls sasl_bind(), which needs to get the Root DSE to proceed with the bind. root_dse_values() is implemented with a call to root_dse(), which calls search(). As with the initial call to search() in schema(), search() calls operation(), which detects that the server is not connected, and calls reconnect(). This results in a recursive loop of calls to reconnect() that do not respect the number of previous attempts to connect specified in retry_limit, and eventually overflow the stack if the LDAP server is completely unresponsive.

----

I was able to prevent the SystemStackError and instead raise a ConnectionError with a couple of changes. First, reconnect() was changed to include the following value in the option hash passed to connect: "{:try_reconnect => false}". This should cause the calls from operation() to with_timeout() to raise a TimeoutError instead of making the recursive call to reconnect(). However, this option hash appears to be ignored in two other methods in the chain. The first is in sasl_bind(), which does not pass any of it's input options to root_dse_values, and instead relies on root_dse_values using the default "{}". The second is in root_dse, which ignores the input options in favor of a hard coded option hash passed into search().

I am very new to the activeldap code base, there may be external effects in the above code that I did not catch. However, it did not appear to introduce new failures in the supplied tests when I checked on my own testing setup. In addition, I was unable to add a new test to cover this issue; I didn't see a framework in place to handle SASL testing.

Thanks for your time!