Merge pull request #203 from alcaeus/fix-nested-empty-keys

Fix detection of empty keys in arrays and objects

lib/Mongo/MongoCollection.php

@@ -282,9 +282,7 @@ class MongoCollection
             return;
         }
 
-        if (! count((array)$a)) {
-            throw new \MongoException('document must be an array or object');
-        }
+        $this->mustBeArrayOrObject($a);
 
         try {
             $result = $this->collection->insertOne(
@@ -368,16 +366,19 @@ class MongoCollection
      * Update records based on a given criteria
      *
      * @link http://www.php.net/manual/en/mongocollection.update.php
-     * @param array $criteria Description of the objects to update.
-     * @param array $newobj The object with which to update the matching records.
+     * @param array|object $criteria Description of the objects to update.
+     * @param array|object $newobj The object with which to update the matching records.
      * @param array $options
      * @return bool|array
      * @throws MongoException
      * @throws MongoWriteConcernException
      */
-    public function update(array $criteria, array $newobj, array $options = [])
+    public function update($criteria, $newobj, array $options = [])
     {
-        $this->checkKeys($newobj);
+        $this->mustBeArrayOrObject($criteria);
+        $this->mustBeArrayOrObject($newobj);
+
+        $this->checkKeys((array) $newobj);
 
         $multiple = isset($options['multiple']) ? $options['multiple'] : false;
         $isReplace = ! \MongoDB\is_first_key_operator($newobj);
@@ -979,12 +980,16 @@ class MongoCollection
         return $options;
     }
 
-    private function checkKeys($array)
+    private function checkKeys(array $array)
     {
-        foreach (array_keys($array) as $key) {
+        foreach ($array as $key => $value) {
             if (empty($key) && $key !== 0) {
                 throw new \MongoException('zero-length keys are not allowed, did you use $ with double quotes?');
             }
+
+            if (is_object($value) || is_array($value)) {
+                $this->checkKeys((array) $value);
+            }
         }
     }
 
@@ -1038,4 +1043,11 @@ class MongoCollection
     {
         return ['db', 'name'];
     }
+
+    private function mustBeArrayOrObject($a)
+    {
+        if (!is_array($a) && !is_object($a)) {
+            throw new \MongoException('document must be an array or object');
+        }
+    }
 }

tests/Alcaeus/MongoDbAdapter/Mongo/MongoCollectionTest.php

@@ -103,12 +103,22 @@ class MongoCollectionTest extends TestCase
         $this->assertSame(1, $this->getCollection()->count(['*' => 'foo']));
     }
 
-    public function testInsertWithEmptyKey()
+    public function getDocumentsWithEmptyKey()
+    {
+        return [
+            'array' => [['' => 'foo']],
+            'object' => [(object) ['' => 'foo']],
+        ];
+    }
+
+    /**
+     * @dataProvider getDocumentsWithEmptyKey
+     */
+    public function testInsertWithEmptyKey($document)
     {
         $this->expectException(\MongoException::class);
         $this->expectExceptionMessage('zero-length keys are not allowed, did you use $ with double quotes?');
 
-        $document = ['' => 'foo'];
         $this->getCollection()->insert($document);
     }
 
@@ -263,12 +273,15 @@ class MongoCollectionTest extends TestCase
         $this->assertSame(1, $this->getCollection()->count(['*' => 'foo']));
     }
 
-    public function testBatchInsertWithEmptyKey()
+    /**
+     * @dataProvider getDocumentsWithEmptyKey
+     */
+    public function testBatchInsertWithEmptyKey($document)
     {
         $this->expectException(\MongoException::class);
         $this->expectExceptionMessage('zero-length keys are not allowed, did you use $ with double quotes?');
 
-        $documents = [['' => 'foo']];
+        $documents = [$document];
         $this->getCollection()->batchInsert($documents);
     }
 
@@ -433,7 +446,10 @@ class MongoCollectionTest extends TestCase
         $this->assertSame(1, $this->getCollection()->count(['*' => 'foo']));
     }
 
-    public function testUpdateWithEmptyKey()
+    /**
+     * @dataProvider getDocumentsWithEmptyKey
+     */
+    public function testUpdateWithEmptyKey($updateDocument)
     {
         $document = ['foo' => 'bar'];
         $this->getCollection()->insert($document);
@@ -441,8 +457,21 @@ class MongoCollectionTest extends TestCase
         $this->expectException(\MongoException::class);
         $this->expectExceptionMessage('zero-length keys are not allowed, did you use $ with double quotes?');
 
-        $update_document = ['' => 'foo'];
-        $this->getCollection()->update($document, $update_document);
+        $this->getCollection()->update($document, $updateDocument);
+    }
+
+    /**
+     * @dataProvider getDocumentsWithEmptyKey
+     */
+    public function testAtomicUpdateWithEmptyKey($updateDocument)
+    {
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
+
+        $this->expectException(\MongoException::class);
+        $this->expectExceptionMessage('zero-length keys are not allowed, did you use $ with double quotes?');
+
+        $this->getCollection()->update($document, ['$set' => $updateDocument]);
     }
 
     public function testRemoveMultiple()

tests/Alcaeus/MongoDbAdapter/TestCase.php

@@ -124,7 +124,7 @@ abstract class TestCase extends BaseTestCase
         $result = $adminDb->command($doc);
         $arr = current($result->toArray());
         if (empty($arr->ok)) {
-            throw new RuntimeException("Failpoint failed");
+            throw new \RuntimeException("Failpoint failed");
         }
 
         return true;