Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ldap] Fixed CS and hardened some code #17833

Merged
merged 1 commit into from Feb 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if getResource() returns null (maybe I simply miss the point where we ensure that the connection is always established before this code is executed)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection is always established before this code is executed.

Either the Query class creates the connection if non-existent (with default values), or it uses the existing connection.


$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)));
}
}
}