From de8626694f0dffdc50f78bb9ae7aaf0168e8e43d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 11 Sep 2017 12:10:32 +0200 Subject: [PATCH] fix limit-flaw in search on paged results Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 16 ++++++++-------- .../Integration/Lib/IntegrationTestPaging.php | 13 +++++++++---- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 69e1f3c52f..e036f6ccb8 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -1149,9 +1149,9 @@ class Access extends LDAPUtility implements IUserTools { * @return array with the search result */ public function search($filter, $base, $attr = null, $limit = null, $offset = null, $skipHandling = false) { - if($limit <= 0) { - //otherwise search will fail - $limit = null; + $limitPerPage = intval($this->connection->ldapPagingSize); + if(!is_null($limit) && $limit < $limitPerPage && $limit > 0) { + $limitPerPage = $limit; } /* ++ Fixing RHDS searches with pages with zero results ++ @@ -1163,7 +1163,7 @@ class Access extends LDAPUtility implements IUserTools { $findings = array(); $savedoffset = $offset; do { - $search = $this->executeSearch($filter, $base, $attr, $limit, $offset); + $search = $this->executeSearch($filter, $base, $attr, $limitPerPage, $offset); if($search === false) { return array(); } @@ -1174,7 +1174,7 @@ class Access extends LDAPUtility implements IUserTools { //i.e. result do not need to be fetched, we just need the cookie //thus pass 1 or any other value as $iFoundItems because it is not //used - $this->processPagedSearchStatus($sr, $filter, $base, 1, $limit, + $this->processPagedSearchStatus($sr, $filter, $base, 1, $limitPerPage, $offset, $pagedSearchOK, $skipHandling); return array(); @@ -1185,10 +1185,10 @@ class Access extends LDAPUtility implements IUserTools { } $continue = $this->processPagedSearchStatus($sr, $filter, $base, $findings['count'], - $limit, $offset, $pagedSearchOK, + $limitPerPage, $offset, $pagedSearchOK, $skipHandling); - $offset += $limit; - } while ($continue && $pagedSearchOK && $findings['count'] < $limit); + $offset += $limitPerPage; + } while ($continue && $pagedSearchOK && ($limit === null || $findings['count'] < $limit)); // reseting offset $offset = $savedoffset; diff --git a/apps/user_ldap/tests/Integration/Lib/IntegrationTestPaging.php b/apps/user_ldap/tests/Integration/Lib/IntegrationTestPaging.php index 44ee1c3bb5..f9be6d6f67 100644 --- a/apps/user_ldap/tests/Integration/Lib/IntegrationTestPaging.php +++ b/apps/user_ldap/tests/Integration/Lib/IntegrationTestPaging.php @@ -47,6 +47,13 @@ class IntegrationTestPaging extends AbstractIntegrationTest { $this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager()); } + public function initConnection() { + parent::initConnection(); + $this->connection->setConfiguration([ + 'ldapPagingSize' => 1 + ]); + } + /** * tests that paging works properly against a simple example (reading all * of few users in smallest steps) @@ -54,20 +61,18 @@ class IntegrationTestPaging extends AbstractIntegrationTest { * @return bool */ protected function case1() { - $limit = 1; $offset = 0; $filter = 'objectclass=inetorgperson'; $attributes = ['cn', 'dn']; $users = []; do { - $result = $this->access->searchUsers($filter, $attributes, $limit, $offset); + $result = $this->access->searchUsers($filter, $attributes, null, $offset); foreach($result as $user) { $users[] = $user['cn']; } - $offset += $limit; + $offset += count($users); } while ($this->access->hasMoreResults()); - if(count($users) === 2) { return true; }