Fixed CS and hardened some code

src/Symfony/Component/Ldap/Adapter/AdapterInterface.php

@@ -26,9 +26,9 @@ interface AdapterInterface
     /**
      * Creates a new Query.
      *
-     * @param $dn
+     * @param string $dn
-     * @param $query
+     * @param string $query
-     * @param array $options
+     * @param array  $options
      *
      * @return QueryInterface
      */

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

@@ -13,6 +13,7 @@ namespace Symfony\Component\Ldap\Adapter\ExtLdap;
 
 use Symfony\Component\Ldap\Adapter\CollectionInterface;
 use Symfony\Component\Ldap\Entry;
+use Symfony\Component\Ldap\Exception\LdapException;
 
 /**
  * @author Charles Sarrazin <charles@sarraz.in>
@@ -84,7 +85,13 @@ class Collection implements CollectionInterface
             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();
@@ -94,15 +101,22 @@ class Collection implements CollectionInterface
 
         $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);
-                'count' => null,
-                'dn' => null,
-            ));
-            array_walk($attributes, function (&$value) {
-                unset($value['count']);
-            });
 
             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']);
+        });
+
+        return $attributes;
+    }
 }

src/Symfony/Component/Ldap/Adapter/ExtLdap/Connection.php

@@ -13,6 +13,7 @@ namespace Symfony\Component\Ldap\Adapter\ExtLdap;
 
 use Symfony\Component\Ldap\Adapter\AbstractConnection;
 use Symfony\Component\Ldap\Exception\ConnectionException;
+use Symfony\Component\Ldap\Exception\LdapException;
 
 /**
  * @author Charles Sarrazin <charles@sarraz.in>
@@ -30,6 +31,9 @@ class Connection extends AbstractConnection
         $this->disconnect();
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function isBound()
     {
         return $this->bound;
@@ -55,6 +59,8 @@ class Connection extends AbstractConnection
      * Returns a link resource.
      *
      * @return resource
+     *
+     * @internal
      */
     public function getResource()
     {
@@ -66,6 +72,7 @@ class Connection extends AbstractConnection
         if ($this->connection) {
             return;
         }
+
         $host = $this->config['host'];
 
         ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, $this->config['version']);
@@ -73,8 +80,12 @@ class Connection extends AbstractConnection
 
         $this->connection = ldap_connect($host, $this->config['port']);
 
-        if ($this->config['useStartTls']) {
+        if (false === $this->connection) {
-            ldap_start_tls($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)));
         }
     }
 
@@ -85,5 +96,6 @@ class Connection extends AbstractConnection
         }
 
         $this->connection = null;
+        $this->bound = false;
     }
 }

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

@@ -30,32 +30,51 @@ class Query extends AbstractQuery
         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 (null === $this->search) {
-        if (!$this->connection->isBound()) {
+            // If the connection is not bound, then we try an anonymous bind.
-            $this->connection->bind();
+            if (!$this->connection->isBound()) {
+                $this->connection->bind();
+            }
+
+            $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']
+            );
         }
 
-        $con = $this->connection->getResource();
+        if (false === $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 (!$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);
     }
@@ -64,6 +83,8 @@ class Query extends AbstractQuery
      * Returns a LDAP search resource.
      *
      * @return resource
+     *
+     * @internal
      */
     public function getResource()
     {

src/Symfony/Component/Ldap/Adapter/ExtLdap/ResultIterator.php

@@ -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
 {
@@ -37,45 +40,42 @@ class ResultIterator implements \Iterator
     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)));
+        }
     }
 }