bug #20605 [Ldap] Always have a valid connection when using the EntryManager (bobvandevijver)
This PR was submitted for the 3.2 branch but it was merged into the 3.1 branch instead (closes #20605).
Discussion
----------
[Ldap] Always have a valid connection when using the EntryManager
| Q | A
| ------------- | ---
| Branch? | 3.2
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | They should
| Fixed tickets |
| License | MIT
| Doc PR |
The connection might be `null` when calling the `getEntryManager` function as it uses the `$this->connection` instead of retrieved it from it's own method which checks (and constructs if necessary) it first.
Commits
-------
7775ec210f
[Ldap] Always have a valid connection when using the EntryManager
This commit is contained in:
commit
5c68c69da1
|
@ -12,11 +12,14 @@
|
|||
namespace Symfony\Component\Ldap\Adapter;
|
||||
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
|
||||
/**
|
||||
* Entry manager interface.
|
||||
*
|
||||
* @author Charles Sarrazin <charles@sarraz.in>
|
||||
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
|
||||
*/
|
||||
interface EntryManagerInterface
|
||||
{
|
||||
|
@ -24,6 +27,9 @@ interface EntryManagerInterface
|
|||
* Adds a new entry in the Ldap server.
|
||||
*
|
||||
* @param Entry $entry
|
||||
*
|
||||
* @throws NotBoundException
|
||||
* @throws LdapException
|
||||
*/
|
||||
public function add(Entry $entry);
|
||||
|
||||
|
@ -31,6 +37,9 @@ interface EntryManagerInterface
|
|||
* Updates an entry from the Ldap server.
|
||||
*
|
||||
* @param Entry $entry
|
||||
*
|
||||
* @throws NotBoundException
|
||||
* @throws LdapException
|
||||
*/
|
||||
public function update(Entry $entry);
|
||||
|
||||
|
@ -38,6 +47,9 @@ interface EntryManagerInterface
|
|||
* Removes an entry from the Ldap server.
|
||||
*
|
||||
* @param Entry $entry
|
||||
*
|
||||
* @throws NotBoundException
|
||||
* @throws LdapException
|
||||
*/
|
||||
public function remove(Entry $entry);
|
||||
}
|
||||
|
|
|
@ -50,7 +50,7 @@ class Adapter implements AdapterInterface
|
|||
public function getEntryManager()
|
||||
{
|
||||
if (null === $this->entryManager) {
|
||||
$this->entryManager = new EntryManager($this->connection);
|
||||
$this->entryManager = new EntryManager($this->getConnection());
|
||||
}
|
||||
|
||||
return $this->entryManager;
|
||||
|
|
|
@ -14,9 +14,11 @@ namespace Symfony\Component\Ldap\Adapter\ExtLdap;
|
|||
use Symfony\Component\Ldap\Adapter\EntryManagerInterface;
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
|
||||
/**
|
||||
* @author Charles Sarrazin <charles@sarraz.in>
|
||||
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
|
||||
*/
|
||||
class EntryManager implements EntryManagerInterface
|
||||
{
|
||||
|
@ -32,7 +34,7 @@ class EntryManager implements EntryManagerInterface
|
|||
*/
|
||||
public function add(Entry $entry)
|
||||
{
|
||||
$con = $this->connection->getResource();
|
||||
$con = $this->getConnectionResource();
|
||||
|
||||
if (!@ldap_add($con, $entry->getDn(), $entry->getAttributes())) {
|
||||
throw new LdapException(sprintf('Could not add entry "%s": %s', $entry->getDn(), ldap_error($con)));
|
||||
|
@ -46,7 +48,7 @@ class EntryManager implements EntryManagerInterface
|
|||
*/
|
||||
public function update(Entry $entry)
|
||||
{
|
||||
$con = $this->connection->getResource();
|
||||
$con = $this->getConnectionResource();
|
||||
|
||||
if (!@ldap_modify($con, $entry->getDn(), $entry->getAttributes())) {
|
||||
throw new LdapException(sprintf('Could not update entry "%s": %s', $entry->getDn(), ldap_error($con)));
|
||||
|
@ -58,10 +60,23 @@ class EntryManager implements EntryManagerInterface
|
|||
*/
|
||||
public function remove(Entry $entry)
|
||||
{
|
||||
$con = $this->connection->getResource();
|
||||
$con = $this->getConnectionResource();
|
||||
|
||||
if (!@ldap_delete($con, $entry->getDn())) {
|
||||
throw new LdapException(sprintf('Could not remove entry "%s": %s', $entry->getDn(), ldap_error($con)));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the connection resource, but first check if the connection is bound.
|
||||
*/
|
||||
private function getConnectionResource()
|
||||
{
|
||||
// If the connection is not bound, throw an exception. Users should use an explicit bind call first.
|
||||
if (!$this->connection->isBound()) {
|
||||
throw new NotBoundException('Query execution is not possible without binding the connection first.');
|
||||
}
|
||||
|
||||
return $this->connection->getResource();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -13,13 +13,15 @@ namespace Symfony\Component\Ldap\Adapter\ExtLdap;
|
|||
|
||||
use Symfony\Component\Ldap\Adapter\AbstractQuery;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
|
||||
/**
|
||||
* @author Charles Sarrazin <charles@sarraz.in>
|
||||
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
|
||||
*/
|
||||
class Query extends AbstractQuery
|
||||
{
|
||||
/** @var Connection */
|
||||
/** @var Connection */
|
||||
protected $connection;
|
||||
|
||||
/** @var resource */
|
||||
|
@ -53,9 +55,9 @@ class Query extends AbstractQuery
|
|||
public function execute()
|
||||
{
|
||||
if (null === $this->search) {
|
||||
// If the connection is not bound, then we try an anonymous bind.
|
||||
// If the connection is not bound, throw an exception. Users should use an explicit bind call first.
|
||||
if (!$this->connection->isBound()) {
|
||||
$this->connection->bind();
|
||||
throw new NotBoundException('Query execution is not possible without binding the connection first.');
|
||||
}
|
||||
|
||||
$con = $this->connection->getResource();
|
||||
|
|
|
@ -12,9 +12,12 @@
|
|||
namespace Symfony\Component\Ldap\Adapter;
|
||||
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
|
||||
/**
|
||||
* @author Charles Sarrazin <charles@sarraz.in>
|
||||
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
|
||||
*/
|
||||
interface QueryInterface
|
||||
{
|
||||
|
@ -27,6 +30,9 @@ interface QueryInterface
|
|||
* Executes a query and returns the list of Ldap entries.
|
||||
*
|
||||
* @return CollectionInterface|Entry[]
|
||||
*
|
||||
* @throws NotBoundException
|
||||
* @throws LdapException
|
||||
*/
|
||||
public function execute();
|
||||
}
|
||||
|
|
|
@ -0,0 +1,21 @@
|
|||
<?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\Exception;
|
||||
|
||||
/**
|
||||
* NotBoundException is thrown if the connection with the LDAP server is not yet bound.
|
||||
*
|
||||
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
|
||||
*/
|
||||
class NotBoundException extends \RuntimeException implements ExceptionInterface
|
||||
{
|
||||
}
|
|
@ -14,6 +14,7 @@ namespace Symfony\Component\Ldap\Tests;
|
|||
use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter;
|
||||
use Symfony\Component\Ldap\Adapter\ExtLdap\Collection;
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
use Symfony\Component\Ldap\LdapInterface;
|
||||
|
||||
/**
|
||||
|
@ -65,4 +66,15 @@ 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 testLdapQueryWithoutBind()
|
||||
{
|
||||
$ldap = new Adapter($this->getLdapConfig());
|
||||
$this->setExpectedException(NotBoundException::class);
|
||||
$query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))', array());
|
||||
$query->execute();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -15,6 +15,7 @@ use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter;
|
|||
use Symfony\Component\Ldap\Adapter\ExtLdap\Collection;
|
||||
use Symfony\Component\Ldap\Entry;
|
||||
use Symfony\Component\Ldap\Exception\LdapException;
|
||||
use Symfony\Component\Ldap\Exception\NotBoundException;
|
||||
|
||||
/**
|
||||
* @requires extension ldap
|
||||
|
@ -98,6 +99,39 @@ class LdapManagerTest extends LdapTestCase
|
|||
$this->assertNull($entry->getAttribute('email'));
|
||||
}
|
||||
|
||||
/**
|
||||
* @group functional
|
||||
*/
|
||||
public function testLdapUnboundAdd()
|
||||
{
|
||||
$this->adapter = new Adapter($this->getLdapConfig());
|
||||
$this->setExpectedException(NotBoundException::class);
|
||||
$em = $this->adapter->getEntryManager();
|
||||
$em->add(new Entry(''));
|
||||
}
|
||||
|
||||
/**
|
||||
* @group functional
|
||||
*/
|
||||
public function testLdapUnboundRemove()
|
||||
{
|
||||
$this->adapter = new Adapter($this->getLdapConfig());
|
||||
$this->setExpectedException(NotBoundException::class);
|
||||
$em = $this->adapter->getEntryManager();
|
||||
$em->remove(new Entry(''));
|
||||
}
|
||||
|
||||
/**
|
||||
* @group functional
|
||||
*/
|
||||
public function testLdapUnboundUpdate()
|
||||
{
|
||||
$this->adapter = new Adapter($this->getLdapConfig());
|
||||
$this->setExpectedException(NotBoundException::class);
|
||||
$em = $this->adapter->getEntryManager();
|
||||
$em->update(new Entry(''));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return Collection|Entry[]
|
||||
*/
|
||||
|
|
Reference in New Issue