LDAP: PHP Warning should be catched

[rosali]

[31-Okt-2008 00:16:03] PHP Warning: ldap_search() [<a href='function.ldap-search'>function.ldap-search</a>]: Partial search results returned: Sizelimit exceeded. in E:\xampp_161\htdocs\webmail\program\include\rcube_ldap.php on line 566

Currently this warning fills up my logs ?

[hayalci]

#1485531 would be a solution to this problem.

To make the LDAP address book work, we have increased the "number of returned results" on the LDAP server and changed the limit in roundcube code with the following patch.

diff -Naur roundcubemail-0.2-beta-orig/program/steps/mail/compose.inc roundcubemail-0.2-beta-modif/program/steps/mail/compose.inc
--- roundcubemail-0.2-beta-orig/program/steps/mail/compose.inc  2008-10-05 21:39:28.000000000 +0300
+++ roundcubemail-0.2-beta-modif/program/steps/mail/compose.inc 2008-10-05 21:42:00.000000000 +0300
@@ -925,7 +925,7 @@

     $LDAP = new rcube_ldap($ldapserv_config);
     $LDAP->connect();
-    $LDAP->set_pagesize(1000);
+    $LDAP->set_pagesize(20000);

     $results = $LDAP->search($ldapserv_config['mail_field'], "");

[alec]

Not sure, but for some LDAP servers it may be fixed by checking LDAP_OPT_SIZELIMIT with ldap_get_option() and then setting sizelimit to such value in ldap_search() call. For servers not supported by ldap_get_option() we'll need probably config option for sizelimit value.

[alec]

Attached not tested patch, which checks sizelimit on LDAP server if is supported by PHP (ldap_get_option) or/and allows to set 'sizelimit' in roundcube 'ldap_public' config option.

[alec]

Looks like the only solution is to disable error with '@' sign. Fixed in r2941.

[till]

This doesn't exactly fix the original warning, but what I noticed:

http://trac.roundcube.net/browser/trunk/roundcubemail/program/include/rcube_ldap.php?rev=2941#L391

ldap_read() can return false, so, I'm guessing it shouldn't be used in ldap_first_entry() without being tested.

rcube_ldap::_exec_search() should also return true if the previous really returned true (or not false).

What do you think?

[alec]

Replying to till:

This doesn't exactly fix the original warning, but what I noticed:

It does.

ldap_read() can return false, so, I'm guessing it shouldn't be used in ldap_first_entry() without being tested. rcube_ldap::_exec_search() should also return true if the previous really returned true (or not false).

_exec_search() result is not check anywhere, but I've made some changes in r2942-2943.

[till]

Replying to alec:

Replying to till:

This doesn't exactly fix the original warning, but what I noticed:

It does.

I really meant that my "observations" were not related to the warning in this ticket.

Till

ldap_read() can return false, so, I'm guessing it shouldn't be used in ldap_first_entry() without being tested. rcube_ldap::_exec_search() should also return true if the previous really returned true (or not false).

_exec_search() result is not check anywhere, but I've made some changes in r2942-2943.

Thanks. IMHO, it didn't make any sense to always return true. And maybe we should check the return value (maybe later ;-)).