/*
* If any LDAP operation fails due to LDAP_SERVER_DOWN, control returns here.
*/
start_over:
if (failures++ > 10) {
return result;
}
if (LDAP_SUCCESS != (result = util_ldap_connection_open(r, ldc))) {
return result;
}
// do something with ldc and goto start_over if it returns LDAP_SERVER_DOWN
Now, util_ldap_connection_open either returns immediately, if the connection appears to be bound, or it tries to bind the connection, looping 10 times on LDAP_SERVER_DOWN (line 337, I agree with the comment at that point in the code).
So in order for the start_over loop to occur more than twice, it would be necessary that:
some operation returns LDAP_SERVER_DOWN;
util_ldap_connection_open succeeds in reopening the connection
with less than 10 retries;
but the operation returns LDAP_SERVER_DOWN again.Not only does this seem unlikely, it also seems like a situation where it would be better to return a 503 than to keep retrying. The correct logic would be something like:
1. if the connection is bound, try the operation
2. if the operation returns LDAP_SERVER_DOWN or the connection was not bound,
call util_ldap_connection_open and if it returns success, try the operation
I'm not sure under what circumstances various LDAP SDKs return LDAP_SERVER_DOWN, but I suppose it would generally either be because:
1) an existing connection was dropped, perhaps because of an idle time-out
2) there is no LDAP server listening on the interface at all and
the connection was refused
3) the LDAP server failed to accept the connection within some timeout.
In the first case, one or two retries are in order. In the second case, retrying is unlikely to change the situation at all. And in the third case, the user is likely to have long since gone somewhere else. So I think that 10 retries within connection_open is excessive as well; three is probably more than enough.
