bug #19980 [Ldap] Fixed issue with legacy find() method not working as expected (csarrazi)
This PR was merged into the 3.1 branch.
Discussion
----------
[Ldap] Fixed issue with legacy find() method not working as expected
| Q | A
| ------------- | ---
| Branch? | 3.1
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #19804
| License | MIT
| Doc PR |
This PR fixes two bugs. The first, with the legacy `LdapClient` class' `find()` method not working as expected, sometimes throwing errors, which is an after-effect of missing Ldap attributes normalisation in the ResultIterator, and the second one being that the `find()` method does not return the expected output, which should be the same as PHP's `ldap_get_entries()` method.
As a reminder, this method should only be used by legacy software, which need to provide compatibility with Symfony 3.0 and Symfony 2.8.
Commits
-------
3bae5ea Fixed issue with legacy find() method not working as expected
This commit is contained in:
Fabien Potencier
commit
d79dedf876
@ -24,11 +24,10 @@ class Collection implements CollectionInterface
|
||||
private $search;
|
||||
private $entries;
|
||||
|
||||
public function __construct(Connection $connection, Query $search, array $entries = array())
|
||||
public function __construct(Connection $connection, Query $search)
|
||||
{
|
||||
$this->connection = $connection;
|
||||
$this->search = $search;
|
||||
$this->entries = array();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -36,78 +35,91 @@ class Collection implements CollectionInterface
|
||||
*/
|
||||
public function toArray()
|
||||
{
|
||||
$this->initialize();
|
||||
if (null === $this->entries) {
|
||||
$this->entries = iterator_to_array($this->getIterator(), false);
|
||||
}
|
||||
|
||||
return $this->entries;
|
||||
}
|
||||
|
||||
public function count()
|
||||
{
|
||||
$this->initialize();
|
||||
if (false !== $count = ldap_count_entries($this->connection->getResource(), $this->search->getResource())) {
|
||||
return $count;
|
||||
}
|
||||
|
||||
return count($this->entries);
|
||||
throw new LdapException(sprintf('Error while retrieving entry count: %s', ldap_error($this->connection->getResource())));
|
||||
}
|
||||
|
||||
public function getIterator()
|
||||
{
|
||||
return new ResultIterator($this->connection, $this->search);
|
||||
$con = $this->connection->getResource();
|
||||
$search = $this->search->getResource();
|
||||
$current = ldap_first_entry($con, $search);
|
||||
|
||||
if (0 === $this->count()) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (false === $current) {
|
||||
throw new LdapException(sprintf('Could not rewind entries array: %s', ldap_error($con)));
|
||||
}
|
||||
|
||||
yield $this->getSingleEntry($con, $current);
|
||||
|
||||
while (false !== $current = ldap_next_entry($con, $current)) {
|
||||
yield $this->getSingleEntry($con, $current);
|
||||
}
|
||||
}
|
||||
|
||||
public function offsetExists($offset)
|
||||
{
|
||||
$this->initialize();
|
||||
$this->toArray();
|
||||
|
||||
return isset($this->entries[$offset]);
|
||||
}
|
||||
|
||||
public function offsetGet($offset)
|
||||
{
|
||||
$this->toArray();
|
||||
|
||||
return isset($this->entries[$offset]) ? $this->entries[$offset] : null;
|
||||
}
|
||||
|
||||
public function offsetSet($offset, $value)
|
||||
{
|
||||
$this->initialize();
|
||||
$this->toArray();
|
||||
|
||||
$this->entries[$offset] = $value;
|
||||
}
|
||||
|
||||
public function offsetUnset($offset)
|
||||
{
|
||||
$this->initialize();
|
||||
$this->toArray();
|
||||
|
||||
unset($this->entries[$offset]);
|
||||
}
|
||||
|
||||
private function initialize()
|
||||
private function getSingleEntry($con, $current)
|
||||
{
|
||||
if (null === $this->entries) {
|
||||
return;
|
||||
$attributes = ldap_get_attributes($con, $current);
|
||||
|
||||
if (false === $attributes) {
|
||||
throw new LdapException(sprintf('Could not fetch attributes: %s', ldap_error($con)));
|
||||
}
|
||||
|
||||
$con = $this->connection->getResource();
|
||||
$attributes = $this->cleanupAttributes($attributes);
|
||||
|
||||
$entries = ldap_get_entries($con, $this->search->getResource());
|
||||
$dn = ldap_get_dn($con, $current);
|
||||
|
||||
if (false === $entries) {
|
||||
throw new LdapException(sprintf('Could not load entries: %s', ldap_error($con)));
|
||||
if (false === $dn) {
|
||||
throw new LdapException(sprintf('Could not fetch DN: %s', ldap_error($con)));
|
||||
}
|
||||
|
||||
if (0 === $entries['count']) {
|
||||
return array();
|
||||
}
|
||||
|
||||
unset($entries['count']);
|
||||
|
||||
$this->entries = array_map(function (array $entry) {
|
||||
$dn = $entry['dn'];
|
||||
$attributes = $this->cleanupAttributes($entry);
|
||||
|
||||
return new Entry($dn, $attributes);
|
||||
}, $entries);
|
||||
return new Entry($dn, $attributes);
|
||||
}
|
||||
|
||||
private function cleanupAttributes(array $entry = array())
|
||||
private function cleanupAttributes(array $entry)
|
||||
{
|
||||
$attributes = array_diff_key($entry, array_flip(range(0, $entry['count'] - 1)) + array(
|
||||
'count' => null,
|
||||
|
||||
@ -1,81 +0,0 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Ldap\Adapter\ExtLdap;
|
||||
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
|
||||
/**
|
||||
* @author Charles Sarrazin <charles@sarraz.in>
|
||||
*
|
||||
* @internal
|
||||
*/
|
||||
class ResultIterator implements \Iterator
|
||||
{
|
||||
private $connection;
|
||||
private $search;
|
||||
private $current;
|
||||
private $key;
|
||||
|
||||
public function __construct(Connection $connection, Query $search)
|
||||
{
|
||||
$this->connection = $connection->getResource();
|
||||
$this->search = $search->getResource();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches the current entry.
|
||||
*
|
||||
* @return Entry
|
||||
*/
|
||||
public function current()
|
||||
{
|
||||
$attributes = ldap_get_attributes($this->connection, $this->current);
|
||||
|
||||
if (false === $attributes) {
|
||||
throw new LdapException(sprintf('Could not fetch attributes: %s', ldap_error($this->connection)));
|
||||
}
|
||||
|
||||
$dn = ldap_get_dn($this->connection, $this->current);
|
||||
|
||||
if (false === $dn) {
|
||||
throw new LdapException(sprintf('Could not fetch DN: %s', ldap_error($this->connection)));
|
||||
}
|
||||
|
||||
return new Entry($dn, $attributes);
|
||||
}
|
||||
|
||||
public function next()
|
||||
{
|
||||
$this->current = ldap_next_entry($this->connection, $this->current);
|
||||
++$this->key;
|
||||
}
|
||||
|
||||
public function key()
|
||||
{
|
||||
return $this->key;
|
||||
}
|
||||
|
||||
public function valid()
|
||||
{
|
||||
return false !== $this->current;
|
||||
}
|
||||
|
||||
public function rewind()
|
||||
{
|
||||
$this->current = ldap_first_entry($this->connection, $this->search);
|
||||
|
||||
if (false === $this->current) {
|
||||
throw new LdapException(sprintf('Could not rewind entries array: %s', ldap_error($this->connection)));
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -64,24 +64,34 @@ final class LdapClient implements LdapClientInterface
|
||||
|
||||
$query = $this->ldap->query($dn, $query, array('filter' => $filter));
|
||||
$entries = $query->execute();
|
||||
$result = array();
|
||||
$result = array(
|
||||
'count' => 0,
|
||||
);
|
||||
|
||||
foreach ($entries as $entry) {
|
||||
$resultEntry = array();
|
||||
|
||||
foreach ($entry->getAttributes() as $attribute => $values) {
|
||||
$resultAttribute = $values;
|
||||
$resultAttribute = array(
|
||||
'count' => count($values),
|
||||
);
|
||||
|
||||
foreach ($values as $val) {
|
||||
$resultAttribute[] = $val;
|
||||
}
|
||||
$attributeName = strtolower($attribute);
|
||||
|
||||
$resultAttribute['count'] = count($values);
|
||||
$resultEntry[] = $resultAttribute;
|
||||
$resultEntry[$attribute] = $resultAttribute;
|
||||
$resultEntry[$attributeName] = $resultAttribute;
|
||||
$resultEntry[] = $attributeName;
|
||||
}
|
||||
|
||||
$resultEntry['count'] = count($resultEntry) / 2;
|
||||
$resultEntry['dn'] = $entry->getDn();
|
||||
$result[] = $resultEntry;
|
||||
}
|
||||
|
||||
$result['count'] = count($result);
|
||||
$result['count'] = count($result) - 1;
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
@ -47,4 +47,22 @@ class AdapterTest extends LdapTestCase
|
||||
$this->assertEquals(array('Fabien Potencier'), $entry->getAttribute('cn'));
|
||||
$this->assertEquals(array('fabpot@symfony.com', 'fabien@potencier.com'), $entry->getAttribute('mail'));
|
||||
}
|
||||
|
||||
/**
|
||||
* @group functional
|
||||
*/
|
||||
public function testLdapQueryIterator()
|
||||
{
|
||||
$ldap = new Adapter($this->getLdapConfig());
|
||||
|
||||
$ldap->getConnection()->bind('cn=admin,dc=symfony,dc=com', 'symfony');
|
||||
$query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))', array());
|
||||
$result = $query->execute();
|
||||
$iterator = $result->getIterator();
|
||||
$iterator->rewind();
|
||||
$entry = $iterator->current();
|
||||
$this->assertInstanceOf(Entry::class, $entry);
|
||||
$this->assertEquals(array('Fabien Potencier'), $entry->getAttribute('cn'));
|
||||
$this->assertEquals(array('fabpot@symfony.com', 'fabien@potencier.com'), $entry->getAttribute('mail'));
|
||||
}
|
||||
}
|
||||
|
||||
@ -20,7 +20,7 @@ use Symfony\Component\Ldap\LdapInterface;
|
||||
/**
|
||||
* @group legacy
|
||||
*/
|
||||
class LdapClientTest extends \PHPUnit_Framework_TestCase
|
||||
class LdapClientTest extends LdapTestCase
|
||||
{
|
||||
/** @var LdapClient */
|
||||
private $client;
|
||||
@ -72,13 +72,11 @@ class LdapClientTest extends \PHPUnit_Framework_TestCase
|
||||
->method('getIterator')
|
||||
->will($this->returnValue(new \ArrayIterator(array(
|
||||
new Entry('cn=qux,dc=foo,dc=com', array(
|
||||
'dn' => array('cn=qux,dc=foo,dc=com'),
|
||||
'cn' => array('qux'),
|
||||
'dc' => array('com', 'foo'),
|
||||
'givenName' => array('Qux'),
|
||||
)),
|
||||
new Entry('cn=baz,dc=foo,dc=com', array(
|
||||
'dn' => array('cn=baz,dc=foo,dc=com'),
|
||||
'cn' => array('baz'),
|
||||
'dc' => array('com', 'foo'),
|
||||
'givenName' => array('Baz'),
|
||||
@ -101,78 +99,44 @@ class LdapClientTest extends \PHPUnit_Framework_TestCase
|
||||
$expected = array(
|
||||
'count' => 2,
|
||||
0 => array(
|
||||
'count' => 4,
|
||||
0 => array(
|
||||
'count' => 1,
|
||||
0 => 'cn=qux,dc=foo,dc=com',
|
||||
),
|
||||
'dn' => array(
|
||||
'count' => 1,
|
||||
0 => 'cn=qux,dc=foo,dc=com',
|
||||
),
|
||||
1 => array(
|
||||
'count' => 1,
|
||||
0 => 'qux',
|
||||
),
|
||||
'count' => 3,
|
||||
0 => 'cn',
|
||||
'cn' => array(
|
||||
'count' => 1,
|
||||
0 => 'qux',
|
||||
),
|
||||
2 => array(
|
||||
'count' => 2,
|
||||
0 => 'com',
|
||||
1 => 'foo',
|
||||
),
|
||||
1 => 'dc',
|
||||
'dc' => array(
|
||||
'count' => 2,
|
||||
0 => 'com',
|
||||
1 => 'foo',
|
||||
),
|
||||
3 => array(
|
||||
'count' => 1,
|
||||
0 => 'Qux',
|
||||
),
|
||||
'givenName' => array(
|
||||
2 => 'givenname',
|
||||
'givenname' => array(
|
||||
'count' => 1,
|
||||
0 => 'Qux',
|
||||
),
|
||||
'dn' => 'cn=qux,dc=foo,dc=com',
|
||||
),
|
||||
1 => array(
|
||||
'count' => 4,
|
||||
0 => array(
|
||||
'count' => 1,
|
||||
0 => 'cn=baz,dc=foo,dc=com',
|
||||
),
|
||||
'dn' => array(
|
||||
'count' => 1,
|
||||
0 => 'cn=baz,dc=foo,dc=com',
|
||||
),
|
||||
1 => array(
|
||||
'count' => 1,
|
||||
0 => 'baz',
|
||||
),
|
||||
'count' => 3,
|
||||
0 => 'cn',
|
||||
'cn' => array(
|
||||
'count' => 1,
|
||||
0 => 'baz',
|
||||
),
|
||||
2 => array(
|
||||
'count' => 2,
|
||||
0 => 'com',
|
||||
1 => 'foo',
|
||||
),
|
||||
1 => 'dc',
|
||||
'dc' => array(
|
||||
'count' => 2,
|
||||
0 => 'com',
|
||||
1 => 'foo',
|
||||
),
|
||||
3 => array(
|
||||
'count' => 1,
|
||||
0 => 'Baz',
|
||||
),
|
||||
'givenName' => array(
|
||||
2 => 'givenname',
|
||||
'givenname' => array(
|
||||
'count' => 1,
|
||||
0 => 'Baz',
|
||||
),
|
||||
'dn' => 'cn=baz,dc=foo,dc=com',
|
||||
),
|
||||
);
|
||||
$this->assertEquals($expected, $this->client->find('dc=foo,dc=com', 'bar', 'baz'));
|
||||
@ -190,6 +154,25 @@ class LdapClientTest extends \PHPUnit_Framework_TestCase
|
||||
$this->assertEquals($expected, call_user_func_array(array($reflMethod, 'invoke'), $args));
|
||||
}
|
||||
|
||||
/**
|
||||
* @group functional
|
||||
* @requires extension ldap
|
||||
*/
|
||||
public function testLdapClientFunctional()
|
||||
{
|
||||
$config = $this->getLdapConfig();
|
||||
$ldap = new LdapClient($config['host'], $config['port']);
|
||||
$ldap->bind('cn=admin,dc=symfony,dc=com', 'symfony');
|
||||
$result = $ldap->find('dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))');
|
||||
|
||||
$con = @ldap_connect($config['host'], $config['port']);
|
||||
@ldap_bind($con, 'cn=admin,dc=symfony,dc=com', 'symfony');
|
||||
$search = @ldap_search($con, 'dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))', array('*'));
|
||||
$expected = @ldap_get_entries($con, $search);
|
||||
|
||||
$this->assertSame($expected, $result);
|
||||
}
|
||||
|
||||
public function provideConfig()
|
||||
{
|
||||
return array(
|
||||
|
||||
Reference in New Issue
Block a user