Merge pull request #19489 from owncloud/fix-18297

fix possible infinite loop when reading groups in the wizard

3 files changed, 106 insertions, 1 deletions

diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index a2fdd88a240..0707b95013c 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -1468,6 +1468,30 @@ class Access extends LDAPUtility implements user\IUserTools {
 	}
 
 	/**
+	 * checks whether an LDAP paged search operation has more pages that can be
+	 * retrieved, typically when offset and limit are provided.
+	 *
+	 * Be very careful to use it: the last cookie value, which is inspected, can
+	 * be reset by other operations. Best, call it immediately after a search(),
+	 * searchUsers() or searchGroups() call. count-methods are probably safe as
+	 * well. Don't rely on it with any fetchList-method.
+	 * @return bool
+	 */
+	public function hasMoreResults() {
+		if(!$this->connection->hasPagedResultSupport) {
+			return false;
+		}
+
+		if(empty($this->lastCookie) && $this->lastCookie !== '0') {
+			// as in RFC 2696, when all results are returned, the cookie will
+			// be empty.
+			return false;
+		}
+
+		return true;
+	}
+
+	/**
 	 * set a cookie for LDAP paged search run
 	 * @param string $base a string with the base DN for the search
 	 * @param string $filter the search filter to identify the correct search
diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php
index 18a0412a63d..84a1225d6ec 100644
--- a/apps/user_ldap/lib/wizard.php
+++ b/apps/user_ldap/lib/wizard.php
@@ -439,7 +439,7 @@ class Wizard extends LDAPUtility {
 				$groupEntries[] = $item;
 			}
 			$offset += $limit;
-		} while (count($groupNames) > 0 && count($groupNames) % $limit === 0);
+		} while ($this->access->hasMoreResults());
 
 		if(count($groupNames) > 0) {
 			natsort($groupNames);
diff --git a/apps/user_ldap/tests/integration/lib/integrationtestpaging.php b/apps/user_ldap/tests/integration/lib/integrationtestpaging.php
new file mode 100644
index 00000000000..f8d008d065f
--- /dev/null
+++ b/apps/user_ldap/tests/integration/lib/integrationtestpaging.php
@@ -0,0 +1,81 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\user_ldap\tests\integration\lib;
+
+use OCA\user_ldap\lib\user\Manager as LDAPUserManager;
+use OCA\user_ldap\tests\integration\AbstractIntegrationTest;
+use OCA\User_LDAP\Mapping\UserMapping;
+use OCA\user_ldap\USER_LDAP;
+
+require_once __DIR__  . '/../../../../../lib/base.php';
+
+class IntegrationTestPaging extends AbstractIntegrationTest {
+	/** @var  UserMapping */
+	protected $mapping;
+
+	/** @var USER_LDAP */
+	protected $backend;
+
+	/**
+	 * prepares the LDAP environment and sets up a test configuration for
+	 * the LDAP backend.
+	 */
+	public function init() {
+		require(__DIR__ . '/../setup-scripts/createExplicitUsers.php');
+		parent::init();
+
+		$this->backend = new \OCA\user_ldap\USER_LDAP($this->access, \OC::$server->getConfig());
+	}
+
+	/**
+	 * tests that paging works properly against a simple example (reading all
+	 * of few users in smallest steps)
+	 *
+	 * @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);
+			foreach($result as $user) {
+				$users[] = $user['cn'];
+			}
+			$offset += $limit;
+		} while ($this->access->hasMoreResults());
+
+		if(count($users) === 2) {
+			return true;
+		}
+
+		return false;
+	}
+}
+
+require_once(__DIR__ . '/../setup-scripts/config.php');
+$test = new IntegrationTestPaging($host, $port, $adn, $apwd, $bdn);
+$test->init();
+$test->run();