Fix RHDS ldap paged search, Issue #17173

Change-Id: Ic804ea95401a9b49cb2b0866af722aa0d3ee58c5

diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index d4f4eaddcb3ffa622f859c8aab785559c096124d..614afca462debfef7d5acfdcb3101c5e9160130b 100644 (file)
--- 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();
-               }
+               /* ++ 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();
+               $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();
 
-               // Do the server-side sorting
-               foreach(array_reverse($attr) as $sortAttr){
-                       foreach($sr as $searchResource) {
-                               $this->ldap->sort($cr, $searchResource, $sortAttr);
+                       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();
                        }
-               }
 
-               $findings = array();
-               foreach($sr as $res) {
-                       $findings = array_merge($findings, $this->ldap->getEntries($cr  , $res ));
-               }
+                       // 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 ));
+                       }
 
-               $this->processPagedSearchStatus($sr, $filter, $base, $findings['count'],
-                                                                               $limit, $offset, $pagedSearchOK,
+                       $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.