10000 feature #29495 [Ldap] Implement pagination (kevans91) · symfony/symfony@2bd27ff · GitHub
[go: up one dir, main page]
Skip to content

Commit 2bd27ff

Browse files
fabpotfabpot
committed
feature #29495 [Ldap] Implement pagination (kevans91)
This PR was squashed before being merged into the 4.3-dev branch (closes #29495). Discussion ---------- [Ldap] Implement pagination | Q | A | ------------- | --- | Branch? | master | Bug fix? | yno | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | N/A (cannot test at the moment) | Fixed tickets | N/A | License | MIT | Doc PR | N/A Implement pagination support in the ExtLdap adapter. In a more abstract sense, other adapters should use a query's pageSize option to determine if pagination is being needed. Pagination is required in some environments that frequently query for more results than the remote server is willing to allow. BC break was avoided by having Query->getResource() return the first result, if available. A small hack is included to work around php-ldap failing to reset pagination properly; the LDAP_OPT_SERVER_CONTROLS are sent with every request, whether pagination has been 'reset' by sending a 0-page request or not. This appears to be a php-ldap bug that will need to be addressed there, but we can work-around it for now by doing both: setting the 0-page option *and* unsetting the OID directly. This was resulting in odd results, like queries returning 0 results or returning < server_page_size results for a query that should have returned >= server_page_size. Commits ------- b963474 [Ldap] Implement pagination
2 parents 5e07953 + b963474 commit 2bd27ff

File tree

7 files changed

+277
-44
lines changed

7 files changed

+277
-44
lines changed

‎.travis.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ before_install:
6262
set -e
6363
stty cols 120
6464
mkdir /tmp/slapd
65+
if [ ! -e /tmp/slapd-modules ]; then
66+
[ -d /usr/lib/openldap ] && ln -s /usr/lib/openldap /tmp/slapd-modules || ln -s /usr/lib/ldap /tmp/slapd-modules
67+
fi
6568
slapd -f src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf -h ldap://localhost:3389 &
6669
[ -d ~/.composer ] || mkdir ~/.composer
6770
cp .composer/* ~/.composer/

‎src/Symfony/Component/Ldap/Adapter/AbstractQuery.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public function __construct(ConnectionInterface $connection, string $dn, string
3535
'deref' => static::DEREF_NEVER,
3636
'attrsOnly' => 0,
3737
'scope' => static::SCOPE_SUB,
38+
'pageSize' => 0,
3839
]);
3940
$resolver->setAllowedValues('deref', [static::DEREF_ALWAYS, static::DEREF_NEVER, static::DEREF_FINDING, static::DEREF_SEARCHING]);
4041
$resolver->setAllowedValues('scope', [static::SCOPE_BASE, static::SCOPE_ONE, static::SCOPE_SUB]);

‎src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php

Lines changed: 21 additions & 12 deletions
< 67E6 /tr>
Original file line numberDiff line numberDiff line change
@@ -44,31 +44,40 @@ public function toArray()
4444

4545
public function count()
4646
{
47-
if (false !== $count = ldap_count_entries($this->connection->getResource(), $this->search->getResource())) {
48-
return $count;
47+
$con = $this->connection->getResource();
48+
$searches = $this->search->getResources();
49+
$count = 0;
50+
foreach ($searches as $search) {
51+
$searchCount = ldap_count_entries($con, $search);
52+
if (false === $searchCount) {
53+
throw new LdapException(sprintf('Error while retrieving entry count: %s.', ldap_error($con)));
54+
}
55+
$count += $searchCount;
4956
}
5057

51-
throw new LdapException(sprintf('Error while retrieving entry count: %s.', ldap_error($this->connection->getResource())));
58+
return $count;
5259
}
5360

5461
public function getIterator()
5562
{
56-
$con = $this->connection->getResource();
57-
$search = $this->search->getResource();
58-
$current = ldap_first_entry($con, $search);
59-
6063
if (0 === $this->count()) {
6164
return;
6265
}
6366

64-
if (false === $current) {
65-
throw new LdapException(sprintf('Could not rewind entries array: %s.', ldap_error($con)));
66-
}
67+
$con = $this->connection->getResource();
68+
$searches = $this->search->getResources();
69+
foreach ($searches as $search) {
70+
$current = ldap_first_entry($con, $search);
6771

68-
yield $this->getSingleEntry($con, $current);
72+
if (false === $current) {
73+
throw new LdapException(sprintf('Could not rewind entries array: %s.', ldap_error($con)));
74+
}
6975

70-
while (false !== $current = ldap_next_entry($con, $current)) {
7176
yield $this->getSingleEntry($con, $current);
77+
78+
while (false !== $current = ldap_next_entry($con, $current)) {
79+
yield $this->getSingleEntry($con, $current);
80+
}
7281
}
7382
}
7483

‎src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php

Lines changed: 124 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@
2121
*/
2222
class Query extends AbstractQuery
2323
{
24+
// As of PHP 7.2, we can use LDAP_CONTROL_PAGEDRESULTS instead of this
25+
const PAGINATION_OID = '1.2.840.113556.1.4.319';
26+
2427
/** @var Connection */
2528
protected $connection;
2629

27-
/** @var resource */
28-
private $search;
30+
/** @var resource[] */
31+
private $results;
2932

3033
public function __construct(Connection $connection, string $dn, string $query, array $options = [])
3134
{
@@ -37,29 +40,33 @@ public function __destruct()
3740
$con = $this->connection->getResource();
3841
$this->connection = null;
3942

40-
if (null === $this->search || false === $this->search) {
43+
if (null === $this->results) {
4144
return;
4245
}
4346

44-
$success = ldap_free_result($this->search);
45-
$this->search = null;
46-
47-
if (!$success) {
48-
throw new LdapException(sprintf('Could not free results: %s.', ldap_error($con)));
47+
foreach ($this->results as $result) {
48+
if (false === $result || null === $result) {
49+
continue;
50+
}
51+
if (!ldap_free_result($result)) {
52+
throw new LdapException(sprintf('Could not free results: %s.', ldap_error($con)));
53+
}
4954
}
55+
$this->results = null;
5056
}
5157

5258
/**
5359
* {@inheritdoc}
5460
*/
5561
public function execute()
5662
{
57-
if (null === $this->search) {
63+
if (null === $this->results) {
5864
// If the connection is not bound, throw an exception. Users should use an explicit bind call first.
5965
if (!$this->connection->isBound()) {
6066
throw new NotBoundException('Query execution is not possible without binding the connection first.');
6167
}
6268

69+
$this->results = [];
6370
$con = $this->connection->getResource();
6471

6572
switch ($this->options['scope']) {
@@ -76,39 +83,126 @@ public function execute()
7683
throw new LdapException(sprintf('Could not search in scope "%s".', $this->options['scope']));
7784
}
7885

79-
$this->search = @$func(
80-
$con,
81-
$this->dn,
82-
$this->query,
83-
$this->options['filter'],
84-
$this->options['attrsOnly'],
85-
$this->options['maxItems'],
86-
$this->options['timeout'],
87-
$this->options['deref']
88-
);
89-
}
90-
91-
if (false === $this->search) {
92-
$ldapError = '';
93-
if ($errno = ldap_errno($con)) {
94-
$ldapError = sprintf(' LDAP error was [%d] %s', $errno, ldap_error($con));
86+
$itemsLeft = $maxItems = $this->options['maxItems'];
87+
$pageSize = $this->options['pageSize'];
88+
// Deal with the logic to handle maxItems properly. If we can satisfy it in
89+
// one request based on pageSize, we don't need to bother sending page control
90+
// to the server so that it can determine what we already know.
91+
if (0 !== $maxItems && $pageSize > $maxItems) {
92+
$pageSize = 0;
93+
} elseif (0 !== $maxItems) {
94+
$pageSize = min($maxItems, $pageSize);
95+
}
96+
$pageControl = $this->options['scope'] != static::SCOPE_BASE && $pageSize > 0;
97+
$cookie = '';
98+
do {
99+
if ($pageControl) {
100+
ldap_control_paged_result($con, $pageSize, true, $cookie);
101+
}
102+
$sizeLimit = $itemsLeft;
103+
if ($pageSize > 0 && $sizeLimit >= $pageSize) {
104+
$sizeLimit = 0;
105+
}
106+
$search = @$func(
107+
$con,
108+
$this->dn,
109+
$this->query,
110+
$this->options['filter'],
111+
$this->options['attrsOnly'],
112+
$sizeLimit,
113+
$this->options['timeout'],
114+
$this->options['deref']
115+
);
116+
117+
if (false === $search) {
118+
$ldapError = '';
119+
if ($errno = ldap_errno($con)) {
120+
$ldapError = sprintf(' LDAP error was [%d] %s', $errno, ldap_error($con));
121+
}
122+
if ($pageControl) {
123+
$this->resetPagination();
124+
}
125+
126+
throw new LdapException(sprintf('Could not complete search with dn "%s", query "%s" and filters "%s".%s', $this->dn, $this->query, implode(',', $this->options['filter']), $ldapError));
127+
}
128+
129+
$this->results[] = $search;
130+
$itemsLeft -= min($itemsLeft, $pageSize);
131+
132+
if (0 !== $maxItems && 0 === $itemsLeft) {
133+
break;
134+
}
135+
if ($pageControl) {
136+
ldap_control_paged_result_response($con, $search, $cookie);
137+
}
138+
} while (null !== $cookie && '' !== $cookie);
139+
140+
if ($pageControl) {
141+
$this->resetPagination();
95142
}
96-
97-
throw new LdapException(sprintf('Could not complete search with dn "%s", query "%s" and filters "%s".%s', $this->dn, $this->query, implode(',', $this->options['filter']), $ldapError));
98143
}
99144

100145
return new Collection($this->connection, $this);
101146
}
102147

103148
/**
104-
* Returns a LDAP search resource.
149+
* Returns a LDAP search resource. If this query resulted in multiple searches, only the first
150+
* page will be returned.
105151
*
106152
* @return resource
107153
*
108154
* @internal
109155
*/
110-
public function getResource()
156+
public function getResource($idx = 0)
111157
{
112-
return $this->search;
158+
if (null === $this->results || $idx >= \count($this->results)) {
159+
return null;
160+
}
161+
162+
return $this->results[$idx];
163+
}
164+
165+
/**
166+
* Returns all LDAP search resources.
167+
*
168+
* @return resource[]
169+
*
170+
* @internal
171+
*/
172+
public function getResources()
173+
{
174+
return $this->results;
175+
}
176+
177+
/**
178+
* Resets pagination on the current connection.
179+
*
180+
* @internal
181+
*/
182+
private function resetPagination()
183+
{
184+
$con = $th 10A16 is->connection->getResource();
185+
ldap_control_paged_result($con, 0);
186+
187+
// This is a workaround for a bit of a bug in the above invocation
188+
// of ldap_control_paged_result. Instead of indicating to extldap that
189+
// we no longer wish to page queries on this link, this invocation sets
190+
// the LDAP_CONTROL_PAGEDRESULTS OID with a page size of 0. This isn't
191+
// well defined by RFC 2696 if there is no cookie present, so some servers
192+
// will interpret it differently and do the wrong thing. Forcefully remove
193+
// the OID for now until a fix can make its way through the versions of PHP
194+
// the we support.
195+
//
196+
// This is not supported in PHP < 7.2, so these versions will remain broken.
197+
$ctl = [];
198+
ldap_get_option($con, LDAP_OPT_SERVER_CONTROLS, $ctl);
199+
if (!empty($ctl)) {
200+
foreach ($ctl as $idx => $info) {
201+
if (static::PAGINATION_OID == $info['oid']) {
202+
unset($ctl[$idx]);
203+
}
204+
}
205+
ldap_set_option($con, LDAP_OPT_SERVER_CONTROLS, $ctl);
206+
}
113207
}
114208
}

‎src/Symfony/Component/Ldap/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
-----
66

77
* added `EntryManager::move`, not implementing it is deprecated
8+
* Added pagination support to the ExtLdap adapter with the pageSize query option
89

910
4.2.0
1011
-----

0 commit comments

Comments
 (0)
0