From 9a63693227b3fd7a44fe3f89d2ab6149992f69e4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 19 Aug 2014 18:01:58 +0200 Subject: properly cancel a Paginated Results operation in order to avoid protocol errors, fixes #10526 --- apps/user_ldap/lib/access.php | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 570f445650d..392c0957d64 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -36,8 +36,16 @@ class Access extends LDAPUtility implements user\IUserTools { //never ever check this var directly, always use getPagedSearchResultState protected $pagedSearchedSuccessful; + /** + * @var string[] $cookies an array of returned Paged Result cookies + */ protected $cookies = array(); + /** + * @var string $lastCookie the last cookie returned from a Paged Results + * operation, defaults to an empty string + */ + protected $lastCookie = ''; public function __construct(Connection $connection, ILDAPWrapper $ldap, user\Manager $userManager) { @@ -84,7 +92,9 @@ class Access extends LDAPUtility implements user\IUserTools { \OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', \OCP\Util::DEBUG); return false; } - //all or nothing! otherwise we get in trouble with. + //Cancel possibly running Paged Results operation, otherwise we run in + //LDAP protocol errors + $this->abandonPagedSearch(); $dn = $this->DNasBaseParameter($dn); $rr = @$this->ldap->read($cr, $dn, $filter, array($attr)); if(!$this->ldap->isResource($rr)) { @@ -805,9 +815,6 @@ class Access extends LDAPUtility implements user\IUserTools { $linkResources = array_pad(array(), count($base), $cr); $sr = $this->ldap->search($linkResources, $base, $filter, $attr); $error = $this->ldap->errno($cr); - if ($pagedSearchOK) { - $this->ldap->controlPagedResult($cr, 999999, false, ""); - } if(!is_array($sr) || $error !== 0) { \OCP\Util::writeLog('user_ldap', 'Error when searching: '.$this->ldap->error($cr). @@ -1365,6 +1372,19 @@ class Access extends LDAPUtility implements user\IUserTools { return $belongsToBase; } + /** + * resets a running Paged Search operation + */ + private function abandonPagedSearch() { + if(count($this->cookies) > 0) { + $cr = $this->connection->getConnectionResource(); + $this->ldap->controlPagedResult($cr, 0, false, $this->lastCookie); + $this->getPagedSearchResultState(); + $this->lastCookie = ''; + $this->cookies = array(); + } + } + /** * get a cookie for the next LDAP paged search * @param string $base a string with the base DN for the search @@ -1403,6 +1423,7 @@ class Access extends LDAPUtility implements user\IUserTools { if(!empty($cookie)) { $cacheKey = 'lc' . crc32($base) . '-' . crc32($filter) . '-' .intval($limit) . '-' . intval($offset); $this->cookies[$cacheKey] = $cookie; + $this->lastCookie = $cookie; } } -- cgit v1.2.3 From 53ec32807abbde7854bcfe54b3c85797d62ec4a6 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 18 Sep 2014 17:00:51 +0200 Subject: abandon ongoing paged search before starting a new one --- apps/user_ldap/lib/access.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 392c0957d64..760d23bf44c 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1376,7 +1376,7 @@ class Access extends LDAPUtility implements user\IUserTools { * resets a running Paged Search operation */ private function abandonPagedSearch() { - if(count($this->cookies) > 0) { + if(!empty($this->lastCookie)) { $cr = $this->connection->getConnectionResource(); $this->ldap->controlPagedResult($cr, 0, false, $this->lastCookie); $this->getPagedSearchResultState(); @@ -1475,9 +1475,8 @@ class Access extends LDAPUtility implements user\IUserTools { } } if(!is_null($cookie)) { - if($offset > 0) { - \OCP\Util::writeLog('user_ldap', 'Cookie '.CRC32($cookie), \OCP\Util::INFO); - } + //since offset = 0, this is a new search. We abandon other searches that might be ongoing. + $this->abandonPagedSearch(); $pagedSearchOK = $this->ldap->controlPagedResult( $this->connection->getConnectionResource(), $limit, false, $cookie); -- cgit v1.2.3 From 2b9696efaea3029cfb20a6d9c931901e8aeaaef1 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 30 Sep 2014 13:13:52 +0200 Subject: abandond paged search only if PHP supports them --- apps/user_ldap/lib/access.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 760d23bf44c..50874ae7a15 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1376,7 +1376,7 @@ class Access extends LDAPUtility implements user\IUserTools { * resets a running Paged Search operation */ private function abandonPagedSearch() { - if(!empty($this->lastCookie)) { + if($this->connection->hasPagedResultSupport) { $cr = $this->connection->getConnectionResource(); $this->ldap->controlPagedResult($cr, 0, false, $this->lastCookie); $this->getPagedSearchResultState(); -- cgit v1.2.3 From f9e085b020e73b1cae350823b0d108b7b122cc56 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 30 Sep 2014 17:00:25 +0200 Subject: init a new paged search on read operations to satisfy OpenLDAP --- apps/user_ldap/lib/access.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 50874ae7a15..68641b7a298 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -95,6 +95,9 @@ class Access extends LDAPUtility implements user\IUserTools { //Cancel possibly running Paged Results operation, otherwise we run in //LDAP protocol errors $this->abandonPagedSearch(); + // openLDAP requires that we init a new Paged Search. Not needed by AD, + // but does not hurt either. + $this->initPagedSearch($filter, array($dn), $attr, 1, 0); $dn = $this->DNasBaseParameter($dn); $rr = @$this->ldap->read($cr, $dn, $filter, array($attr)); if(!$this->ldap->isResource($rr)) { -- cgit v1.2.3 From 6c502e11f8a199bfbcda833efb2e16497895ae42 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 1 Oct 2014 11:55:53 +0200 Subject: make scrutinizer happy, very minor changes --- apps/user_ldap/lib/access.php | 2 +- apps/user_ldap/lib/ildapwrapper.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 68641b7a298..159b0d73000 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -97,7 +97,7 @@ class Access extends LDAPUtility implements user\IUserTools { $this->abandonPagedSearch(); // openLDAP requires that we init a new Paged Search. Not needed by AD, // but does not hurt either. - $this->initPagedSearch($filter, array($dn), $attr, 1, 0); + $this->initPagedSearch($filter, array($dn), array($attr), 1, 0); $dn = $this->DNasBaseParameter($dn); $rr = @$this->ldap->read($cr, $dn, $filter, array($attr)); if(!$this->ldap->isResource($rr)) { diff --git a/apps/user_ldap/lib/ildapwrapper.php b/apps/user_ldap/lib/ildapwrapper.php index 590f6d7ac7a..a64bcd6b95b 100644 --- a/apps/user_ldap/lib/ildapwrapper.php +++ b/apps/user_ldap/lib/ildapwrapper.php @@ -51,7 +51,7 @@ interface ILDAPWrapper { * @param resource $link LDAP link resource * @param int $pageSize number of results per page * @param bool $isCritical Indicates whether the pagination is critical of not. - * @param array $cookie structure sent by LDAP server + * @param string $cookie structure sent by LDAP server * @return bool true on success, false otherwise */ public function controlPagedResult($link, $pageSize, $isCritical, $cookie); -- cgit v1.2.3