From 353a8e442f895500b8f324c2dc4afd389ac52425 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 1 Oct 2015 00:30:05 +0200 Subject: [PATCH] fix possible infinite loop when reading groups in the wizard --- apps/user_ldap/lib/access.php | 24 ++++++ apps/user_ldap/lib/wizard.php | 2 +- .../integration/lib/integrationtestpaging.php | 81 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 apps/user_ldap/tests/integration/lib/integrationtestpaging.php diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 2a605a2a0f..e5bb4fc10f 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1466,6 +1466,30 @@ class Access extends LDAPUtility implements user\IUserTools { return $cookie; } + /** + * 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 diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 8d96e7de91..8dd7bc5abf 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -438,7 +438,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 0000000000..f8d008d065 --- /dev/null +++ b/apps/user_ldap/tests/integration/lib/integrationtestpaging.php @@ -0,0 +1,81 @@ + + * + * @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 + * + */ + +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();