Merge pull request #747 from DirectoryTree/BUG-746

Bug 746 - Infinite loop in LdapRecord\Models\Collection::contains function

src/Models/Collection.php

@@ -21,8 +21,10 @@ public function modelDns(): static
 
     /**
      * Determine if the collection contains all the given models, or any models.
+     *
+     * @param  QueryCollection|Model|array|string|null  $models
      */
-    public function exists($models = null): bool
+    public function exists(mixed $models = null): bool
     {
         $models = $this->getArrayableModels($models);
 
@@ -56,10 +58,7 @@ public function exists($models = null): bool
     public function contains($key, $operator = null, $value = null): bool
     {
         if (func_num_args() > 1 || $key instanceof Closure) {
-            // If we are supplied with more than one argument, or
-            // we were passed a closure, we will utilize the
-            // parents contains method, for compatibility.
-            return parent::contains($key, $operator, $value);
+            return parent::contains(...func_get_args());
         }
 
         foreach ($this->getArrayableModels($key) as $model) {
@@ -78,7 +77,7 @@ public function contains($key, $operator = null, $value = null): bool
     /**
      * Get the provided models as an array.
      */
-    protected function getArrayableModels($models = null): array
+    protected function getArrayableModels(mixed $models = null): array
     {
         if ($models instanceof QueryCollection) {
             return $models->all();

src/Models/Model.php

@@ -557,6 +557,8 @@ public function fresh(): static|false
     public function is(?Model $model = null): bool
     {
         return ! is_null($model)
+            && ! empty($this->dn)
+            && ! empty($model->getDn())
             && $this->dn == $model->getDn()
             && $this->getConnectionName() == $model->getConnectionName();
     }

tests/Unit/Models/CollectionTest.php

@@ -0,0 +1,194 @@
+<?php
+
+namespace LdapRecord\Tests\Unit\Models;
+
+use LdapRecord\Models\ActiveDirectory\User;
+use LdapRecord\Models\Collection;
+use LdapRecord\Models\Model;
+use LdapRecord\Tests\TestCase;
+
+class CollectionTest extends TestCase
+{
+    public function test_exists_with_no_parameters()
+    {
+        $collection = new Collection;
+
+        $this->assertFalse(
+            $collection->exists()
+        );
+
+        $collection = new Collection([new User]);
+
+        $this->assertTrue(
+            $collection->exists()
+        );
+    }
+
+    public function test_exists_with_dn()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->exists('cn=John Doe')
+        );
+
+        $this->assertFalse(
+            $collection->exists('cn=Jane Doe')
+        );
+    }
+
+    public function test_exists_with_multiple_dns()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->exists(['cn=John Doe'])
+        );
+
+        $this->assertFalse(
+            $collection->exists(['cn=Jane Doe', 'cn=Foo Bar'])
+        );
+    }
+
+    public function test_exists_with_model()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->exists($user)
+        );
+
+        $this->assertFalse(
+            $collection->exists(new User(['dn' => 'cn=Jane Doe']))
+        );
+    }
+
+    public function test_exists_with_model_collection()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->exists(new Collection([$user]))
+        );
+
+        $this->assertFalse(
+            $collection->exists(new Collection([
+                new User(['dn' => 'cn=Jane Doe']),
+                new User(['dn' => 'cn=Foo Bar']),
+            ]))
+        );
+    }
+
+    public function test_contains_with_closure()
+    {
+        $user = new User(['cn' => 'John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->contains(function (Model $model) {
+                return $model->getFirstAttribute('cn') === 'John Doe';
+            })
+        );
+
+        $this->assertFalse(
+            $collection->contains(function (Model $model) {
+                return $model->getFirstAttribute('cn') === 'Jane Doe';
+            })
+        );
+    }
+
+    public function test_contains_with_key_operator_and_value()
+    {
+        $user = new User(['cn' => 'John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->contains('cn', '=', ['John Doe'])
+        );
+
+        $this->assertFalse(
+            $collection->contains('cn', '=', ['Jane Doe'])
+        );
+    }
+
+    public function test_contains_with_model()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $otherUser = new User(['dn' => 'cn=Jane Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertTrue(
+            $collection->contains($user)
+        );
+
+        $this->assertFalse(
+            $collection->contains($otherUser)
+        );
+    }
+
+    public function test_contains_with_model_without_dn()
+    {
+        $user = new User(['cn' => 'John Doe']);
+
+        $collection = new Collection([$user]);
+
+        $this->assertFalse(
+            $collection->contains(new User)
+        );
+    }
+
+    public function test_contains_with_multiple_models()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $otherUser = new User(['dn' => 'cn=Jane Doe']);
+
+        $collection = new Collection([$user, $otherUser]);
+
+        $this->assertTrue(
+            $collection->contains([
+                $user,
+                $otherUser,
+            ])
+        );
+
+        $this->assertFalse(
+            $collection->contains([
+                new User(['dn' => 'cn=Foo Bar']),
+                new User(['dn' => 'cn=Bar Baz']),
+            ])
+        );
+    }
+
+    public function test_contains_with_model_collection()
+    {
+        $user = new User(['dn' => 'cn=John Doe']);
+
+        $otherUser = new User(['dn' => 'cn=Jane Doe']);
+
+        $collection = new Collection([$user, $otherUser]);
+
+        $this->assertTrue(
+            $collection->contains(new Collection([$user]))
+        );
+
+        $this->assertFalse(
+            $collection->contains(new Collection([
+                new User(['dn' => 'cn=Foo Bar']),
+                new User(['dn' => 'cn=Bar Baz']),
+            ]))
+        );
+    }
+}

tests/Unit/Models/ModelSerializationTest.php

@@ -19,6 +19,7 @@ public function testModelWithTimestampsCanBeSerializedAndEncoded()
 
         $model = (new User)->setRawAttributes([
             'cn' => 'René',
+            'dn' => 'cn=René',
             'whenchanged' => [(string) $whenchanged],
             'lastlogon' => [(string) $lastlogon],
         ]);
@@ -48,6 +49,7 @@ public function testModelWithBinaryGuidAndSidCanBeSerializedAndEncoded()
         $sid = new Sid('S-1-5-21-1004336348-1177238915-682003330-512');
 
         $model = (new Entry)->setRawAttributes([
+            'dn' => 'cn=Foo Bar',
             'objectguid' => [$guid->getBinary()],
             'objectsid' => [$sid->getBinary()],
         ]);