Added check if a id in a valid format to the Collection::findById

CHANGELOG.md

@@ -109,6 +109,7 @@
 - Renamed `Phalcon\Http\Request::isSoapRequest` to `Phalcon\Http\Request::isSoap` and `Phalcon\Http\Request::isSecureRequest` to `Phalcon\Http\Request::isSecure`. Left the originals functions as aliases and marked them deprecated.
 - Fixed wildcard inheritance in `Phalcon\Acl\Adapter\Memory` [#12004](https://github.com/phalcon/cphalcon/issues/12004)
 - Dropped support of Oracle [#12008](https://github.com/phalcon/cphalcon/issues/12008)
+- Improved `Phalcon\Mvc\Collection::findById`. Added check if a `id` in a valid format [#12010](https://github.com/phalcon/cphalcon/issues/12010)
 
 # [2.0.13](https://github.com/phalcon/cphalcon/releases/tag/phalcon-v2.0.13) (2016-05-19)
 - Restored `Phalcon\Text::camelize` behavior [#11767](https://github.com/phalcon/cphalcon/issues/11767)

phalcon/mvc/collection.zep

@@ -1113,14 +1113,27 @@ abstract class Collection implements EntityInterface, CollectionInterface, Injec
 	/**
 	 * Find a document by its id (_id)
 	 *
-	 * @param string|\MongoId id
-	 * @return \Phalcon\Mvc\Collection
+	 * <code>
+	 * // Find user by using \MongoId object
+	 * $user = Users::findById(new \MongoId('545eb081631d16153a293a66'));
+	 *
+	 * // Find user by using id as sting
+	 * $user = Users::findById('45cbc4a0e4123f6920000002');
+	 *
+	 * // Validate input
+	 * if ($user = Users::findById($_POST['id'])) {
+	 *     // ...
+	 * }
+	 * </code>
 	 */
-	public static function findById(id) -> <Collection>
+	public static function findById(var id) -> <Collection> | null
 	{
 		var className, collection, mongoId;
 
 		if typeof id != "object" {
+			if !preg_match("/^[a-f\d]{24}$/i", id) {
+				return null;
+			}
 
 			let className = get_called_class();
 
@@ -1146,23 +1159,28 @@ abstract class Collection implements EntityInterface, CollectionInterface, Injec
 	 * Allows to query the first record that match the specified conditions
 	 *
 	 * <code>
-	 *
-	 * //What's the first robot in the robots table?
+	 * // What's the first robot in the robots table?
 	 * $robot = Robots::findFirst();
-	 * echo "The robot name is ", $robot->name, "\n";
+	 * echo 'The robot name is ', $robot->name, "\n";
 	 *
-	 * //What's the first mechanical robot in robots table?
-	 * $robot = Robots::findFirst(array(
-	 *     array("type" => "mechanical")
-	 * ));
-	 * echo "The first mechanical robot name is ", $robot->name, "\n";
+	 * // What's the first mechanical robot in robots table?
+	 * $robot = Robots::findFirst([
+	 *     ['type' => 'mechanical']
+	 * ]);
+	 * echo 'The first mechanical robot name is ', $robot->name, "\n";
 	 *
-	 * //Get first virtual robot ordered by name
-	 * $robot = Robots::findFirst(array(
-	 *     array("type" => "mechanical"),
-	 *     "order" => array("name" => 1)
-	 * ));
-	 * echo "The first virtual robot name is ", $robot->name, "\n";
+	 * // Get first virtual robot ordered by name
+	 * $robot = Robots::findFirst([
+	 *     ['type' => 'mechanical'],
+	 *     'order' => ['name' => 1]
+	 * ]);
+	 * echo 'The first virtual robot name is ', $robot->name, "\n";
+	 *
+	 * // Get first robot by id (_id)
+	 * $robot = Robots::findFirst([
+	 *     ['_id' => new \MongoId('45cbc4a0e4123f6920000002')]
+	 * ]);
+	 * echo 'The robot id is ', $robot->_id, "\n";
 	 * </code>
 	 */
 	public static function findFirst(array parameters = null) -> array

tests/_data/collections/Songs.php

@@ -7,8 +7,8 @@
 /**
  * Songs Collection
  *
- * @method static Songs[] find($parameters = null)
- * @method static Songs findFirst($parameters = null)
+ * @method static Songs[] find(array $parameters = null)
+ * @method static Songs findFirst(array $parameters = null)
  *
  * @property \MongoId _id
  * @property string artist

tests/_support/Helper/Collection.php → tests/_support/Helper/CollectionTrait.php

@@ -12,7 +12,7 @@
  *
  * @package Helper
  */
-trait Collection
+trait CollectionTrait
 {
     /**
      * Executed before each test

tests/integration/Mvc/Collection/BehaviorCest.php

@@ -2,7 +2,7 @@
 
 namespace Phalcon\Test\Integration\Mvc\Collection;
 
-use Helper\Collection;
+use Helper\CollectionTrait;
 use IntegrationTester;
 use Phalcon\Test\Collections\Subs;
 
@@ -25,7 +25,7 @@
  */
 class BehaviorCest
 {
-    use Collection;
+    use CollectionTrait;
 
     public function behaviors(IntegrationTester $I)
     {

tests/unit/Mvc/Collection/ClassChangeCest.php

@@ -3,7 +3,7 @@
 namespace Phalcon\Test\Unit\Mvc\Collection;
 
 use UnitTester;
-use Helper\Collection;
+use Helper\CollectionTrait;
 use Phalcon\Mvc\Collection\Exception;
 use Phalcon\Test\Collections\Bookshelf\Books;
 
@@ -26,7 +26,7 @@
  */
 class ClassChangeCest
 {
-    use Collection;
+    use CollectionTrait;
 
     public function change(UnitTester $I)
     {

tests/unit/Mvc/CollectionTest.php

@@ -2,7 +2,7 @@
 
 namespace Phalcon\Test\Unit\Mvc;
 
-use Helper\Collection;
+use Helper\CollectionTrait;
 use Phalcon\Test\Module\UnitTest;
 use Phalcon\Test\Collections\Songs;
 
@@ -25,7 +25,7 @@
  */
 class CollectionTest extends UnitTest
 {
-    use Collection;
+    use CollectionTrait;
 
     /**
      * executed before each test
@@ -296,6 +296,38 @@ function () {
         );
     }
 
+    /**
+     * Tests Collection::findById
+     *
+     * @author Serghei Iakovlev <serghei@phalconphp.com>
+     * @since  2016-07-24
+     * @issue  12010
+     */
+    public function testShouldUseCorrectMongoId()
+    {
+        $this->specify(
+            'The Collection::findById does not return expected result',
+            function () {
+                $song = new Songs();
+                $song->artist = 'Jo Blankenburg';
+                $song->name = 'Valkyrie';
+                $song->create();
+
+                $mongoIdObject = $song->_id;
+
+                expect(Songs::findById($mongoIdObject->__toString()))->isInstanceOf('\Phalcon\Mvc\CollectionInterface');
+                expect(Songs::findById($mongoIdObject))->isInstanceOf('\Phalcon\Mvc\CollectionInterface');
+
+                expect(Songs::findById('qwerty-1234'))->null();
+                expect(Songs::findById('s'))->null();
+                expect(Songs::findById(str_repeat('0', 25)))->null();
+                expect(Songs::findById(''))->null();
+                expect(Songs::findById(null))->null();
+                expect(Songs::findById(false))->null();
+            }
+        );
+    }
+
     protected function createDocument(array $fields)
     {
         $song = new Songs();