Backport of PR #7815, correct LDAP user count on setup with many users

(occ user:report)

LDAP: make sure cache key for paged result cookie matches when limit or offset is null or 0

LDAP: fix user report i.e. count for LDAP servers with really many users

initialize variable

var count is assigned in the inner loop so it must be checked inside there to be properly used as part of the exit condition of the outer loop

Put inner loop into own method, let's see whether it makes scrutinizer happier

1 files changed, 40 insertions, 15 deletions

diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index 43c0b1b8e77..ae25918af89 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -693,7 +693,7 @@ class Access extends LDAPUtility {
 	}
 
 	public function countUsers($filter, $attr = array('dn'), $limit = null, $offset = null) {
-		return $this->count($filter, $this->connection->ldapBaseGroups, $attr, $limit, $offset);
+		return $this->count($filter, $this->connection->ldapBaseUsers, $attr, $limit, $offset);
 	}
 
 	/**
@@ -807,22 +807,47 @@ class Access extends LDAPUtility {
 	 */
 	private function count($filter, $base, $attr = null, $limit = null, $offset = null, $skipHandling = false) {
 		\OCP\Util::writeLog('user_ldap', 'Count filter:  '.print_r($filter, true), \OCP\Util::DEBUG);
-		$search = $this->executeSearch($filter, $base, $attr, $limit, $offset);
-		if($search === false) {
-			return false;
+
+		if(is_null($limit)) {
+			$limit = 400;
 		}
-		list($sr, $pagedSearchOK) = $search;
-		$cr = $this->connection->getConnectionResource();
+
 		$counter = 0;
-		foreach($sr as $key => $res) {
-			$count = $this->ldap->countEntries($cr, $res);
-		    if($count !== false) {
-				$counter += $count;
+		$count = null;
+		$cr = $this->connection->getConnectionResource();
+
+		do {
+			$continue = false;
+			$search = $this->executeSearch($filter, $base, $attr,
+										   $limit, $offset);
+			if($search === false) {
+				return $counter > 0 ? $counter : false;
 			}
-		}
+			list($sr, $pagedSearchOK) = $search;
+
+			$count = $this->countEntriesInSearchResults($sr, $limit, $continue);
+			$counter += $count;
 
-		$this->processPagedSearchStatus($sr, $filter, $base, $counter, $limit,
+			$this->processPagedSearchStatus($sr, $filter, $base, $count, $limit,
 										$offset, $pagedSearchOK, $skipHandling);
+			$offset += $limit;
+		} while($continue);
+
+		return $counter;
+	}
+
+	private function countEntriesInSearchResults($searchResults, $limit,
+																&$hasHitLimit) {
+		$cr = $this->connection->getConnectionResource();
+		$count = 0;
+
+		foreach($searchResults as $res) {
+			$count = intval($this->ldap->countEntries($cr, $res));
+			$counter += $count;
+			if($count === $limit) {
+				$hasHitLimit = true;
+			}
+		}
 
 		return $counter;
 	}
@@ -923,7 +948,7 @@ class Access extends LDAPUtility {
 		//we slice the findings, when
 		//a) paged search insuccessful, though attempted
 		//b) no paged search, but limit set
-		if((!$this->pagedSearchedSuccessful
+		if((!$this->getPagedSearchResultState()
 			&& $pagedSearchOK)
 			|| (
 				!$pagedSearchOK
@@ -1213,7 +1238,7 @@ class Access extends LDAPUtility {
 		}
 		$offset -= $limit;
 		//we work with cache here
-		$cachekey = 'lc' . crc32($base) . '-' . crc32($filter) . '-' . $limit . '-' . $offset;
+		$cachekey = 'lc' . crc32($base) . '-' . crc32($filter) . '-' . intval($limit) . '-' . intval($offset);
 		$cookie = '';
 		if(isset($this->cookies[$cachekey])) {
 			$cookie = $this->cookies[$cachekey];
@@ -1235,7 +1260,7 @@ class Access extends LDAPUtility {
 	 */
 	private function setPagedResultCookie($base, $filter, $limit, $offset, $cookie) {
 		if(!empty($cookie)) {
-			$cachekey = 'lc' . crc32($base) . '-' . crc32($filter) . '-' .$limit . '-' . $offset;
+			$cachekey = 'lc' . crc32($base) . '-' . crc32($filter) . '-' .intval($limit) . '-' . intval($offset);
 			$this->cookies[$cachekey] = $cookie;
 		}
 	}