From f3900e89fdf85c06384aaab5c1cfa30200edf56f Mon Sep 17 00:00:00 2001 From: ADmad Date: Tue, 3 Dec 2013 22:14:26 +0530 Subject: [PATCH 1/9] Fixed bug where deleteAll tried to delete same id multiple times. Ensure find done in deleteAll only returns distinct ids. A wacky combination of association and conditions can sometimes generate multiple rows per id. --- lib/Cake/Model/Model.php | 2 +- lib/Cake/Test/Case/Model/ModelDeleteTest.php | 37 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 2c1b66ce1..ce6c13d2d 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -2699,7 +2699,7 @@ class Model extends Object implements CakeEventListener { } $ids = $this->find('all', array_merge(array( - 'fields' => "{$this->alias}.{$this->primaryKey}", + 'fields' => "DISTINCT {$this->alias}.{$this->primaryKey}", 'recursive' => 0), compact('conditions')) ); diff --git a/lib/Cake/Test/Case/Model/ModelDeleteTest.php b/lib/Cake/Test/Case/Model/ModelDeleteTest.php index 5cadf2cfa..9abcb5101 100644 --- a/lib/Cake/Test/Case/Model/ModelDeleteTest.php +++ b/lib/Cake/Test/Case/Model/ModelDeleteTest.php @@ -454,6 +454,43 @@ class ModelDeleteTest extends BaseModelTest { $this->assertFalse($result); } +/** + * testDeleteAllMultipleRowsPerId method + * + * Ensure find done in deleteAll only returns distinct ids. A wacky combination + * of association and conditions can sometimes generate multiple rows per id. + * + * @return void + */ + public function testDeleteAllMultipleRowsPerId() { + $this->loadFixtures('Article', 'User'); + + $TestModel = new Article(); + $TestModel->unbindModel(array( + 'belongsTo' => array('User'), + 'hasMany' => array('Comment'), + 'hasAndBelongsToMany' => array('Tag') + ), false); + $TestModel->bindModel(array( + 'belongsTo' => array( + 'User' => array( + 'foreignKey' => false, + 'conditions' => array( + 'Article.user_id = 1' + ) + ) + ) + ), false); + + $result = $TestModel->deleteAll( + array('Article.user_id' => array(1, 3)), + true, + true + ); + + $this->assertTrue($result); + } + /** * testRecursiveDel method * From e6f2c92005cfb5850159621da182f9b4f327fdd2 Mon Sep 17 00:00:00 2001 From: mark_story Date: Tue, 3 Dec 2013 22:39:14 -0500 Subject: [PATCH 2/9] Cast results of implementedEvents to an array. This makes using mocks much easier with event listeners as they don't need to provide a returnValue for implementedEvents. Refs #2333 --- lib/Cake/Event/CakeEventManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Event/CakeEventManager.php b/lib/Cake/Event/CakeEventManager.php index 77d283e76..3d6bf94f3 100644 --- a/lib/Cake/Event/CakeEventManager.php +++ b/lib/Cake/Event/CakeEventManager.php @@ -122,7 +122,7 @@ class CakeEventManager { * @return void */ protected function _attachSubscriber(CakeEventListener $subscriber) { - foreach ($subscriber->implementedEvents() as $eventKey => $function) { + foreach ((array)$subscriber->implementedEvents() as $eventKey => $function) { $options = array(); $method = $function; if (is_array($function) && isset($function['callable'])) { From 6ee5277c9fc57e33ba2d148519d113c72f659a33 Mon Sep 17 00:00:00 2001 From: Eliu Florez Date: Wed, 4 Dec 2013 14:26:02 -0430 Subject: [PATCH 3/9] Update CakeEventManager.php Cast resultados de implementedEvents a una matriz. https://github.com/cakephp/cakephp/commit/e6f2c92005cfb5850159621da182f9b4f327fdd2 --- lib/Cake/Event/CakeEventManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Event/CakeEventManager.php b/lib/Cake/Event/CakeEventManager.php index 3d6bf94f3..74d737b9d 100644 --- a/lib/Cake/Event/CakeEventManager.php +++ b/lib/Cake/Event/CakeEventManager.php @@ -197,7 +197,7 @@ class CakeEventManager { * @return void */ protected function _detachSubscriber(CakeEventListener $subscriber, $eventKey = null) { - $events = $subscriber->implementedEvents(); + $events = (array)$subscriber->implementedEvents(); if (!empty($eventKey) && empty($events[$eventKey])) { return; } elseif (!empty($eventKey)) { From 85a9132c9bb515ed83558bc08997572e6271c48a Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 4 Dec 2013 21:46:59 -0500 Subject: [PATCH 4/9] Append / to the start/end of the mapResources prefix. This makes the method easier to use and less error prone. Fixes #2431 --- lib/Cake/Routing/Router.php | 6 ++++++ lib/Cake/Test/Case/Routing/RouterTest.php | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/Cake/Routing/Router.php b/lib/Cake/Routing/Router.php index c5dbf660e..f5ae3847d 100644 --- a/lib/Cake/Routing/Router.php +++ b/lib/Cake/Routing/Router.php @@ -503,6 +503,12 @@ class Router { ), $options); $prefix = $options['prefix']; + if (strpos($prefix, '/') !== 0) { + $prefix = '/' . $prefix; + } + if (substr($prefix, -1) !== '/') { + $prefix .= '/'; + } foreach ((array)$controller as $name) { list($plugin, $name) = pluginSplit($name); diff --git a/lib/Cake/Test/Case/Routing/RouterTest.php b/lib/Cake/Test/Case/Routing/RouterTest.php index 19144f4e6..6a7cf05a7 100644 --- a/lib/Cake/Test/Case/Routing/RouterTest.php +++ b/lib/Cake/Test/Case/Routing/RouterTest.php @@ -215,6 +215,20 @@ class RouterTest extends CakeTestCase { ); $this->assertEquals($expected, $result); $this->assertEquals(array('test_plugin'), $resources); + + $resources = Router::mapResources('Posts', array('prefix' => 'api')); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + $result = Router::parse('/api/posts'); + $expected = array( + 'pass' => array(), + 'named' => array(), + 'plugin' => null, + 'controller' => 'posts', + 'action' => 'index', + '[method]' => 'GET' + ); + $this->assertEquals($expected, $result); } /** From 738d0e2277fcf16bca352a37fe7524e7b0828269 Mon Sep 17 00:00:00 2001 From: ADmad Date: Sat, 7 Dec 2013 18:40:08 +0530 Subject: [PATCH 5/9] Fixed edge case which allowed login with empty password. Ensure skipping call to FormAuthenticate::_checkFields() does not allow logging in with empty password. Closes #2441. --- .../Component/Auth/BaseAuthenticate.php | 4 +-- .../Component/Auth/FormAuthenticateTest.php | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Controller/Component/Auth/BaseAuthenticate.php b/lib/Cake/Controller/Component/Auth/BaseAuthenticate.php index 6541a15e3..65a0b80d3 100644 --- a/lib/Cake/Controller/Component/Auth/BaseAuthenticate.php +++ b/lib/Cake/Controller/Component/Auth/BaseAuthenticate.php @@ -1,7 +1,5 @@ passwordHasher()->check($password, $user[$fields['password']])) { return false; } diff --git a/lib/Cake/Test/Case/Controller/Component/Auth/FormAuthenticateTest.php b/lib/Cake/Test/Case/Controller/Component/Auth/FormAuthenticateTest.php index deacfdec2..90f5797c9 100644 --- a/lib/Cake/Test/Case/Controller/Component/Auth/FormAuthenticateTest.php +++ b/lib/Cake/Test/Case/Controller/Component/Auth/FormAuthenticateTest.php @@ -118,6 +118,40 @@ class FormAuthenticateTest extends CakeTestCase { $this->assertFalse($this->auth->authenticate($request, $this->response)); } +/** + * Test for password as empty string with _checkFields() call skipped + * Refs https://github.com/cakephp/cakephp/pull/2441 + * + * @return void + */ + public function testAuthenticatePasswordIsEmptyString() { + $request = new CakeRequest('posts/index', false); + $request->data = array( + 'User' => array( + 'user' => 'mariano', + 'password' => '' + )); + + $this->auth = $this->getMock( + 'FormAuthenticate', + array('_checkFields'), + array( + $this->Collection, + array( + 'fields' => array('username' => 'user', 'password' => 'password'), + 'userModel' => 'User' + ) + ) + ); + + // Simulate that check for ensuring password is not empty is missing. + $this->auth->expects($this->once()) + ->method('_checkFields') + ->will($this->returnValue(true)); + + $this->assertFalse($this->auth->authenticate($request, $this->response)); + } + /** * test authenticate field is not string * From 98e645a1eab4c1ae8a4ddf346b20aaba4c3842f5 Mon Sep 17 00:00:00 2001 From: Rachman Chavik Date: Sun, 8 Dec 2013 16:39:56 +0700 Subject: [PATCH 6/9] Add test to prove issues with deleteAll with $order set --- lib/Cake/Test/Case/Model/ModelDeleteTest.php | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/Cake/Test/Case/Model/ModelDeleteTest.php b/lib/Cake/Test/Case/Model/ModelDeleteTest.php index 9abcb5101..dc8f3107a 100644 --- a/lib/Cake/Test/Case/Model/ModelDeleteTest.php +++ b/lib/Cake/Test/Case/Model/ModelDeleteTest.php @@ -491,6 +491,33 @@ class ModelDeleteTest extends BaseModelTest { $this->assertTrue($result); } +/** + * testDeleteAllWithOrderProperty + * + * Ensure find done in deleteAll works with models that has $order property set + * + * @return void + */ + public function testDeleteAllWithOrderProperty() { + $this->loadFixtures('Article', 'User'); + + $TestModel = new Article(); + $TestModel->order = 'Article.published desc'; + $TestModel->unbindModel(array( + 'belongsTo' => array('User'), + 'hasMany' => array('Comment'), + 'hasAndBelongsToMany' => array('Tag') + ), false); + + $result = $TestModel->deleteAll( + array('Article.user_id' => array(1, 3)), + true, + true + ); + + $this->assertTrue($result); + } + /** * testRecursiveDel method * From 2d5a153c0de210c2e51618563208fdb8b9b9a5b0 Mon Sep 17 00:00:00 2001 From: Rachman Chavik Date: Sun, 8 Dec 2013 16:40:22 +0700 Subject: [PATCH 7/9] Set 'order' to false to prevent issues with postgres See: https://github.com/cakephp/cakephp/pull/2421#issuecomment-30074971 --- lib/Cake/Model/Model.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index ce6c13d2d..b79db9347 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -2700,6 +2700,7 @@ class Model extends Object implements CakeEventListener { $ids = $this->find('all', array_merge(array( 'fields' => "DISTINCT {$this->alias}.{$this->primaryKey}", + 'order' => false, 'recursive' => 0), compact('conditions')) ); From 530c95725f8b200b85b66104358f88efd07ae9bf Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 8 Dec 2013 11:46:17 -0500 Subject: [PATCH 8/9] Attempt to fix errors with postgres tests. --- lib/Cake/Test/Case/Model/ModelWriteTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index 0ef299814..07b892ce1 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -6262,7 +6262,7 @@ class ModelWriteTest extends BaseModelTest { 'Author' => array( 'user' => 'bob', 'password' => '5f4dcc3b5aa765d61d8327deb882cf90' - ))); + )), array('atomic' => false)); $result = $TestModel->find('first', array( 'order' => array('Post.id ' => 'DESC') From abf4af14a8414907d8092b653d30f6e3c87b10bf Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 8 Dec 2013 11:58:26 -0500 Subject: [PATCH 9/9] Change quotes to keep postgres happy. --- lib/Cake/Test/Case/Model/ModelWriteTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index 07b892ce1..3ad70fd84 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -6256,7 +6256,7 @@ class ModelWriteTest extends BaseModelTest { $TestModel->saveAssociated(array( 'Post' => array( - 'title' => $db->expression('(SELECT "Post with Author")'), + 'title' => $db->expression("(SELECT 'Post with Author')"), 'body' => 'This post will be saved with an author' ), 'Author' => array(