From 46a8eab772f82923cab6e158f7b71df5415081d9 Mon Sep 17 00:00:00 2001 From: Mario Kolling Date: Thu, 16 Jul 2015 17:17:57 -0300 Subject: [PATCH] Fix RHDS ldap paged search, Issue #17173 Change-Id: Ic804ea95401a9b49cb2b0866af722aa0d3ee58c5 --- apps/user_ldap/lib/access.php | 110 +++++++++++++++++++++------------- 1 file changed, 69 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index d4f4eaddcb..614afca462 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -865,14 +865,13 @@ class Access extends LDAPUtility implements user\IUserTools { * @param bool $pagedSearchOK whether a paged search has been executed * @param bool $skipHandling required for paged search when cookies to * prior results need to be gained - * @return array|false array with the search result as first value and pagedSearchOK as - * second | false if not successful + * @return bool cookie validity, true if we have more pages, false otherwise. */ private function processPagedSearchStatus($sr, $filter, $base, $iFoundItems, $limit, $offset, $pagedSearchOK, $skipHandling) { + $cookie = null; if($pagedSearchOK) { $cr = $this->connection->getConnectionResource(); foreach($sr as $key => $res) { - $cookie = null; if($this->ldap->controlPagedResultResponse($cr, $res, $cookie)) { $this->setPagedResultCookie($base[$key], $filter, $limit, $offset, $cookie); } @@ -893,6 +892,12 @@ class Access extends LDAPUtility implements user\IUserTools { \OCP\Util::writeLog('user_ldap', 'Paged search was not available', \OCP\Util::INFO); } } + /* ++ Fixing RHDS searches with pages with zero results ++ + * Return cookie status. If we don't have more pages, with RHDS + * cookie is null, with openldap cookie is an empty string and + * to 386ds '0' is a valid cookie. Even if $iFoundItems == 0 + */ + return !empty($cookie) || $cookie === '0'; } /** @@ -921,7 +926,6 @@ class Access extends LDAPUtility implements user\IUserTools { $this->connection->getConnectionResource(); do { - $continue = false; $search = $this->executeSearch($filter, $base, $attr, $limitPerPage, $offset); if($search === false) { @@ -929,12 +933,20 @@ class Access extends LDAPUtility implements user\IUserTools { } list($sr, $pagedSearchOK) = $search; - $count = $this->countEntriesInSearchResults($sr, $limitPerPage, $continue); + /* ++ Fixing RHDS searches with pages with zero results ++ + * countEntriesInSearchResults() method signature changed + * by removing $limit and &$hasHitLimit parameters + */ + $count = $this->countEntriesInSearchResults($sr); $counter += $count; - $this->processPagedSearchStatus($sr, $filter, $base, $count, $limitPerPage, + $hasMorePages = $this->processPagedSearchStatus($sr, $filter, $base, $count, $limitPerPage, $offset, $pagedSearchOK, $skipHandling); $offset += $limitPerPage; + /* ++ Fixing RHDS searches with pages with zero results ++ + * Continue now depends on $hasMorePages value + */ + $continue = $pagedSearchOK && $hasMorePages; } while($continue && (is_null($limit) || $limit <= 0 || $limit > $counter)); return $counter; @@ -942,20 +954,15 @@ class Access extends LDAPUtility implements user\IUserTools { /** * @param array $searchResults - * @param int $limit - * @param bool $hasHitLimit * @return int */ - private function countEntriesInSearchResults($searchResults, $limit, &$hasHitLimit) { + private function countEntriesInSearchResults($searchResults) { $cr = $this->connection->getConnectionResource(); $counter = 0; foreach($searchResults as $res) { $count = intval($this->ldap->countEntries($cr, $res)); $counter += $count; - if($count > 0 && $count === $limit) { - $hasHitLimit = true; - } } return $counter; @@ -976,38 +983,53 @@ class Access extends LDAPUtility implements user\IUserTools { //otherwise search will fail $limit = null; } - $search = $this->executeSearch($filter, $base, $attr, $limit, $offset); - if($search === false) { - return array(); - } - list($sr, $pagedSearchOK) = $search; - $cr = $this->connection->getConnectionResource(); - - if($skipHandling) { - //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, - $offset, $pagedSearchOK, - $skipHandling); - return array(); - } - - // Do the server-side sorting - foreach(array_reverse($attr) as $sortAttr){ - foreach($sr as $searchResource) { - $this->ldap->sort($cr, $searchResource, $sortAttr); - } - } + /* ++ Fixing RHDS searches with pages with zero results ++ + * As we can have pages with zero results and/or pages with less + * than $limit results but with a still valid server 'cookie', + * loops through until we get $continue equals true and + * $findings['count'] < $limit + */ $findings = array(); - foreach($sr as $res) { - $findings = array_merge($findings, $this->ldap->getEntries($cr , $res )); - } + $savedoffset = $offset; + do { + $continue = false; + $search = $this->executeSearch($filter, $base, $attr, $limit, $offset); + if($search === false) { + return array(); + } + list($sr, $pagedSearchOK) = $search; + $cr = $this->connection->getConnectionResource(); - $this->processPagedSearchStatus($sr, $filter, $base, $findings['count'], - $limit, $offset, $pagedSearchOK, + if($skipHandling) { + //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, + $offset, $pagedSearchOK, + $skipHandling); + return array(); + } + + // Do the server-side sorting + foreach(array_reverse($attr) as $sortAttr){ + foreach($sr as $searchResource) { + $this->ldap->sort($cr, $searchResource, $sortAttr); + } + } + + + foreach($sr as $res) { + $findings = array_merge($findings, $this->ldap->getEntries($cr , $res )); + } + + $continue = $this->processPagedSearchStatus($sr, $filter, $base, $findings['count'], + $limit, $offset, $pagedSearchOK, $skipHandling); + $offset += $limit; + } while ($continue && $pagedSearchOK && $findings['count'] < $limit); + // reseting offset + $offset = $savedoffset; // if we're here, probably no connection resource is returned. // to make ownCloud behave nicely, we simply give back an empty array. @@ -1595,7 +1617,13 @@ class Access extends LDAPUtility implements user\IUserTools { } } - } else if($this->connection->hasPagedResultSupport && $limit === 0) { + /* ++ Fixing RHDS searches with pages with zero results ++ + * We coudn't get paged searches working with our RHDS for login ($limit = 0), + * due to pages with zero results. + * So we added "&& !empty($this->lastCookie)" to this test to ignore pagination + * if we don't have a previous paged search. + */ + } else if($this->connection->hasPagedResultSupport && $limit === 0 && !empty($this->lastCookie)) { // a search without limit was requested. However, if we do use // Paged Search once, we always must do it. This requires us to // initialize it with the configured page size.