From f292f98060b8f19893f6d4c311bf044428e72c04 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 11 Jan 2018 13:20:17 +0100 Subject: [PATCH 1/4] when paged results are turned off, all (max possible) users are returned thus hasMoreResult should return false Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Jobs/Sync.php | 2 +- apps/user_ldap/tests/Jobs/SyncTest.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php index b78a1947e2..dc8a14d111 100644 --- a/apps/user_ldap/lib/Jobs/Sync.php +++ b/apps/user_ldap/lib/Jobs/Sync.php @@ -177,7 +177,7 @@ class Sync extends TimedJob { ); if($connection->ldapPagingSize === 0) { - return true; + return false; } return count($results) >= $connection->ldapPagingSize; } diff --git a/apps/user_ldap/tests/Jobs/SyncTest.php b/apps/user_ldap/tests/Jobs/SyncTest.php index f8852a4666..c6cb9549b0 100644 --- a/apps/user_ldap/tests/Jobs/SyncTest.php +++ b/apps/user_ldap/tests/Jobs/SyncTest.php @@ -158,7 +158,8 @@ class SyncTest extends TestCase { return [ [ 3, 3, true ], [ 3, 5, true ], - [ 3, 2, false] + [ 3, 2, false], + [ 0, 4, false] ]; } From 15a3f4659f7e6a84039cd8ef859d1dfb3eaedf9a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 11 Jan 2018 14:17:14 +0100 Subject: [PATCH 2/4] enrich log message with backtrace, but level it down to DEBUG The message is not helpful anyway for an admin, and oftentimes is just valid (e.g. when searching with an offset beyond users in LDAP). Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 14d5b826f6..d07a8a030a 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -52,6 +52,7 @@ use OCA\User_LDAP\Mapping\AbstractMapping; use OC\ServerNotAvailableException; use OCP\IConfig; +use OCP\Util; /** * Class Access @@ -1937,9 +1938,8 @@ class Access extends LDAPUtility implements IUserTools { } \OCP\Util::writeLog('user_ldap', 'Ready for a paged search', \OCP\Util::DEBUG); } else { - \OCP\Util::writeLog('user_ldap', - 'No paged search for us, Cpt., Limit '.$limit.' Offset '.$offset, - \OCP\Util::INFO); + $e = new \Exception('No paged search possible, Limit '.$limit.' Offset '.$offset); + \OC::$server->getLogger()->logException($e, ['level' => Util::DEBUG]); } } From 9031ae02816d5cf357fce714909ea8ca8d3b8066 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 11 Jan 2018 14:47:51 +0100 Subject: [PATCH 3/4] fix return value when ldapPagingSize returns null Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Jobs/Sync.php | 4 ++-- apps/user_ldap/tests/Jobs/SyncTest.php | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php index dc8a14d111..0cc0be7d3c 100644 --- a/apps/user_ldap/lib/Jobs/Sync.php +++ b/apps/user_ldap/lib/Jobs/Sync.php @@ -176,10 +176,10 @@ class Sync extends TimedJob { true ); - if($connection->ldapPagingSize === 0) { + if((int)$connection->ldapPagingSize === 0) { return false; } - return count($results) >= $connection->ldapPagingSize; + return count($results) >= (int)$connection->ldapPagingSize; } /** diff --git a/apps/user_ldap/tests/Jobs/SyncTest.php b/apps/user_ldap/tests/Jobs/SyncTest.php index c6cb9549b0..75ffd0e828 100644 --- a/apps/user_ldap/tests/Jobs/SyncTest.php +++ b/apps/user_ldap/tests/Jobs/SyncTest.php @@ -159,7 +159,8 @@ class SyncTest extends TestCase { [ 3, 3, true ], [ 3, 5, true ], [ 3, 2, false], - [ 0, 4, false] + [ 0, 4, false], + [ null, 4, false] ]; } From f84ec9256367758b65f37db5f0bc81a9ed2b8d13 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 11 Jan 2018 15:17:18 +0100 Subject: [PATCH 4/4] revert resolving of recursion (3628d4d65d67d12afc93d969de61cb06a33179fd) without recursion we have issues with internal states. paged search status are set to false, cookies are not being set. In the end we have endless requests which pile up enormously with a high initial offset. Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index d07a8a030a..b84f5a38b3 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -1914,11 +1914,8 @@ class Access extends LDAPUtility implements IUserTools { // no cookie known from a potential previous search. We need // to start from 0 to come to the desired page. cookie value // of '0' is valid, because 389ds - $reOffset = 0; - while($reOffset < $offset) { - $this->search($filter, array($base), $attr, $limit, $reOffset, true); - $reOffset += $limit; - } + $reOffset = ($offset - $limit) < 0 ? 0 : $offset - $limit; + $this->search($filter, array($base), $attr, $limit, $reOffset, true); $cookie = $this->getPagedResultCookie($base, $filter, $limit, $offset); //still no cookie? obviously, the server does not like us. Let's skip paging efforts. // '0' is valid, because 389ds