Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
minor #17833 [Ldap] Fixed CS and hardened some code (csarrazi)
This PR was merged into the 3.1-dev branch.

Discussion
----------

[Ldap] Fixed CS and hardened some code

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16665
| License       | MIT
| Doc PR        | no

This PR takes into account remaining comments from #16665

Commits
-------

eefa70d Fixed CS and hardened some code
  • Loading branch information
fabpot committed Feb 18, 2016
2 parents 1f80683 + eefa70d commit 45caae4
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 46 deletions.
6 changes: 3 additions & 3 deletions src/Symfony/Component/Ldap/Adapter/AdapterInterface.php
Expand Up @@ -26,9 +26,9 @@ public function getConnection();
/**
* Creates a new Query.
*
* @param $dn
* @param $query
* @param array $options
* @param string $dn
* @param string $query
* @param array $options
*
* @return QueryInterface
*/
Expand Down
28 changes: 21 additions & 7 deletions src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Ldap\Adapter\CollectionInterface;
use Symfony\Component\Ldap\Entry;
use Symfony\Component\Ldap\Exception\LdapException;

/**
* @author Charles Sarrazin <charles@sarraz.in>
Expand Down Expand Up @@ -84,7 +85,13 @@ private function initialize()
return;
}

$entries = ldap_get_entries($this->connection->getResource(), $this->search->getResource());
$con = $this->connection->getResource();

$entries = ldap_get_entries($con, $this->search->getResource());

if (false === $entries) {
throw new LdapException(sprintf('Could not load entries: %s', ldap_error($con)));
}

if (0 === $entries['count']) {
return array();
Expand All @@ -94,15 +101,22 @@ private function initialize()

$this->entries = array_map(function (array $entry) {
$dn = $entry['dn'];
$attributes = array_diff_key($entry, array_flip(range(0, $entry['count'] - 1)) + array(
$attributes = $this->cleanupAttributes($entry);

return new Entry($dn, $attributes);
}, $entries);
}

private function cleanupAttributes(array $entry = array())
{
$attributes = array_diff_key($entry, array_flip(range(0, $entry['count'] - 1)) + array(
'count' => null,
'dn' => null,
));
array_walk($attributes, function (&$value) {
unset($value['count']);
});
array_walk($attributes, function (&$value) {
unset($value['count']);
});

return new Entry($dn, $attributes);
}, $entries);
return $attributes;
}
}
16 changes: 14 additions & 2 deletions src/Symfony/Component/Ldap/Adapter/ExtLdap/Connection.php
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Ldap\Adapter\AbstractConnection;
use Symfony\Component\Ldap\Exception\ConnectionException;
use Symfony\Component\Ldap\Exception\LdapException;

/**
* @author Charles Sarrazin <charles@sarraz.in>
Expand All @@ -30,6 +31,9 @@ public function __destruct()
$this->disconnect();
}

/**
* {@inheritdoc}
*/
public function isBound()
{
return $this->bound;
Expand All @@ -55,6 +59,8 @@ public function bind($dn = null, $password = null)
* Returns a link resource.
*
* @return resource
*
* @internal
*/
public function getResource()
{
Expand All @@ -66,15 +72,20 @@ private function connect()
if ($this->connection) {
return;
}

$host = $this->config['host'];

ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, $this->config['version']);
ldap_set_option($this->connection, LDAP_OPT_REFERRALS, $this->config['optReferrals']);

$this->connection = ldap_connect($host, $this->config['port']);

if ($this->config['useStartTls']) {
ldap_start_tls($this->connection);
if (false === $this->connection) {
throw new LdapException(sprintf('Could not connect to Ldap server: %s', ldap_error($this->connection)));
}

if ($this->config['useStartTls'] && false === ldap_start_tls($this->connection)) {
throw new LdapException(sprintf('Could not initiate TLS connection: %s', ldap_error($this->connection)));
}
}

Expand All @@ -85,5 +96,6 @@ private function disconnect()
}

$this->connection = null;
$this->bound = false;
}
}
57 changes: 39 additions & 18 deletions src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php
Expand Up @@ -30,32 +30,51 @@ public function __construct(Connection $connection, $dn, $query, array $options
parent::__construct($connection, $dn, $query, $options);
}

public function __destruct()
{
$con = $this->connection->getResource();
$this->connection = null;

if (null === $this->search) {
return;
}

$success = ldap_free_result($this->search);
$this->search = null;

if (!$success) {
throw new LdapException(sprintf('Could not free results: %s', ldap_error($con)));
}
}

/**
* {@inheritdoc}
*/
public function execute()
{
// If the connection is not bound, then we try an anonymous bind.
if (!$this->connection->isBound()) {
$this->connection->bind();
}
if (null === $this->search) {
// If the connection is not bound, then we try an anonymous bind.
if (!$this->connection->isBound()) {
$this->connection->bind();
}

$con = $this->connection->getResource();
$con = $this->connection->getResource();

$this->search = ldap_search(
$con,
$this->dn,
$this->query,
$this->options['filter'],
$this->options['attrsOnly'],
$this->options['maxItems'],
$this->options['timeout'],
$this->options['deref']
);

if (!$this->search) {
$this->search = ldap_search(
$con,
$this->dn,
$this->query,
$this->options['filter'],
$this->options['attrsOnly'],
$this->options['maxItems'],
$this->options['timeout'],
$this->options['deref']
);
}

if (false === $this->search) {
throw new LdapException(sprintf('Could not complete search with dn "%s", query "%s" and filters "%s"', $this->dn, $this->query, implode(',', $this->options['filter'])));
};
}

return new Collection($this->connection, $this);
}
Expand All @@ -64,6 +83,8 @@ public function execute()
* Returns a LDAP search resource.
*
* @return resource
*
* @internal
*/
public function getResource()
{
Expand Down
32 changes: 16 additions & 16 deletions src/Symfony/Component/Ldap/Adapter/ExtLdap/ResultIterator.php
Expand Up @@ -12,9 +12,12 @@
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
{
Expand All @@ -37,45 +40,42 @@ public function __construct(Connection $connection, Query $search)
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);
}

/**
* Sets the cursor to the next entry.
*/
public function next()
{
$this->current = ldap_next_entry($this->connection, $this->current);
++$this->key;
}

/**
* Returns the current key.
*
* @return int
*/
public function key()
{
return $this->key;
}

/**
* Checks whether the current entry is valid or not.
*
* @return bool
*/
public function valid()
{
return false !== $this->current;
}

/**
* Rewinds the iterator to the first entry.
*/
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)));
}
}
}

0 comments on commit 45caae4

Please sign in to comment.