From 6b3db0a3eb0ce3437b015b5ce07fc7afffc0602f Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 19 Dec 2010 01:42:23 -0500 Subject: [PATCH] Pulling out parameter merging logic into a method, this allows specific typing of parameter merging (named, querystring, route). Also simplifies whitelisting of request parameters. Tests added for merging and whitelisting. --- cake/libs/controller/components/paginator.php | 59 ++++++ .../controller/components/paginator.test.php | 190 +++++++++++++++--- 2 files changed, 216 insertions(+), 33 deletions(-) diff --git a/cake/libs/controller/components/paginator.php b/cake/libs/controller/components/paginator.php index 554e58d99..c0e237297 100644 --- a/cake/libs/controller/components/paginator.php +++ b/cake/libs/controller/components/paginator.php @@ -50,6 +50,17 @@ class PaginatorComponent extends Component { 'paramType' => 'named' ); +/** + * A list of request parameters users are allowed to set. Modifying + * this list will allow users to have more influence over pagination, + * be careful with what you permit. + * + * @var array + */ + public $whitelist = array( + 'limit', 'sort', 'page', 'direction' + ); + /** * Constructor * @@ -83,6 +94,9 @@ class PaginatorComponent extends Component { if (!is_object($object)) { throw new MissingModelException($object); } + + $options = $this->mergeOptions($object->alias, $scope, $whitelist); + $options = array_merge( $this->Controller->request->params, $this->Controller->request->query, @@ -279,4 +293,49 @@ class PaginatorComponent extends Component { } return $object; } + +/** + * Merges the various options that Pagination uses. + * Pulls settings together from the following places: + * + * - General pagination settings + * - Model specific settings. + * - Request parameters + * - $options argument. + * + * The result of this method is the aggregate of all the option sets combined together. + * + * @param string $alias Model alias being paginated, if the general settings has a key with this value + * that key's settings will be used for pagination instead of the general ones. + * @param string $options Per call options. + * @param string $whitelist A whitelist of options that are allowed from the request parameters. Modifying + * this array will allow you to permit more or less input from the user. + * @return array Array of merged options. + */ + public function mergeOptions($alias, $options, $whitelist = array()) { + if (isset($this->settings[$alias])) { + $defaults = $this->settings[$alias]; + } else { + $defaults = $this->settings; + } + if (empty($defaults['paramType'])) { + $defaults['paramType'] = 'named'; + } + switch ($defaults['paramType']) { + case 'named': + $request = $this->Controller->request->params['named']; + break; + case 'querystring': + $request = $this->Controller->request->query; + break; + case 'route': + $request = $this->Controller->request->params; + unset($request['pass'], $request['named']); + } + + $whitelist = array_flip(array_merge($this->whitelist, $whitelist)); + $request = array_intersect_key($request, $whitelist); + + return array_merge($defaults, $request, $options); + } } \ No newline at end of file diff --git a/cake/tests/cases/libs/controller/components/paginator.test.php b/cake/tests/cases/libs/controller/components/paginator.test.php index 096c523c5..25bf12a99 100644 --- a/cake/tests/cases/libs/controller/components/paginator.test.php +++ b/cake/tests/cases/libs/controller/components/paginator.test.php @@ -20,6 +20,7 @@ * @license MIT License (http://www.opensource.org/licenses/mit-license.php) */ App::import('Controller', 'Controller', false); +App::import('Component', 'Paginator'); App::import('Core', array('CakeRequest', 'CakeResponse')); /** @@ -210,6 +211,20 @@ class PaginatorTest extends CakeTestCase { */ public $fixtures = array('core.post', 'core.comment'); +/** + * setup + * + * @return void + */ + function setUp() { + parent::setUp(); + $this->request = new CakeRequest('controller_posts/index'); + $this->request->params['pass'] = $this->request->params['named'] = array(); + $this->Controller = new Controller($this->request); + $this->Paginator = new PaginatorComponent($this->getMock('ComponentCollection'), array()); + $this->Paginator->Controller = $this->Controller; + } + /** * testPaginate method * @@ -217,10 +232,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ function testPaginate() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment'); $Controller->passedArgs[] = '1'; $Controller->params['url'] = array(); @@ -306,10 +318,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ function testPaginateExtraParams() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment'); $Controller->passedArgs[] = '1'; @@ -352,7 +361,7 @@ class PaginatorTest extends CakeTestCase { $this->assertEqual($Controller->PaginatorControllerPost->lastQuery['limit'], 12); $this->assertEqual($paging['options']['limit'], 12); - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('ControllerPaginateModel'); $Controller->params['url'] = array(); $Controller->constructClasses(); @@ -400,10 +409,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ public function testPaginatePassedArgs() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('PaginatorControllerPost'); $Controller->passedArgs[] = array('1', '2', '3'); $Controller->params['url'] = array(); @@ -440,10 +446,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ function testPaginateSpecialType() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment'); $Controller->passedArgs[] = '1'; $Controller->params['url'] = array(); @@ -469,10 +472,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ function testDefaultPaginateParams() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->modelClass = 'PaginatorControllerPost'; $Controller->params['url'] = array(); $Controller->constructClasses(); @@ -491,10 +491,7 @@ class PaginatorTest extends CakeTestCase { * @return void */ function testPaginateOrderVirtualField() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment'); $Controller->params['url'] = array(); $Controller->constructClasses(); @@ -522,10 +519,7 @@ class PaginatorTest extends CakeTestCase { * @expectedException MissingModelException */ function testPaginateMissingModel() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new PaginatorTestController($request); + $Controller = new PaginatorTestController($this->request); $Controller->constructClasses(); $Controller->Paginator->paginate('MissingModel'); } @@ -537,10 +531,7 @@ class PaginatorTest extends CakeTestCase { * @access public */ function testPaginateMaxLimit() { - $request = new CakeRequest('controller_posts/index'); - $request->params['pass'] = $request->params['named'] = array(); - - $Controller = new Controller($request); + $Controller = new Controller($this->request); $Controller->uses = array('PaginatorControllerPost', 'ControllerComment'); $Controller->passedArgs[] = '1'; @@ -568,4 +559,137 @@ class PaginatorTest extends CakeTestCase { $result = $Controller->paginate('PaginatorControllerPost'); $this->assertEqual($Controller->params['paging']['PaginatorControllerPost']['options']['limit'], 2000); } + +/** + * test that option merging prefers specific models + * + * @return void + */ + function testMergeOptionsModelSpecific() { + $this->Paginator->settings = array( + 'page' => 1, + 'limit' => 20, + 'maxLimit' => 100, + 'paramType' => 'named', + 'Post' => array( + 'page' => 1, + 'limit' => 10, + 'maxLimit' => 50, + 'paramType' => 'named', + ) + ); + $result = $this->Paginator->mergeOptions('Silly', array()); + $this->assertEquals($this->Paginator->settings, $result); + + $result = $this->Paginator->mergeOptions('Silly', array('limit' => 10)); + $this->assertEquals(10, $result['limit']); + + $result = $this->Paginator->mergeOptions('Post', array('sort' => 'title')); + $expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'sort' => 'title', 'maxLimit' => 50); + $this->assertEquals($expected, $result); + } + +/** + * test mergeOptions with named params. + * + * @return void + */ + function testMergeOptionsNamedParams() { + $this->request->params['named'] = array( + 'page' => 10, + 'limit' => 10 + ); + $this->Paginator->settings = array( + 'page' => 1, + 'limit' => 20, + 'maxLimit' => 100, + 'paramType' => 'named', + ); + $result = $this->Paginator->mergeOptions('Post', array()); + $expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named'); + $this->assertEquals($expected, $result); + + $result = $this->Paginator->mergeOptions('Post', array('page' => 100)); + $this->assertEquals(100, $result['page'], 'Passed options should replace request params'); + } + +/** + * test merging options from the querystring. + * + * @return void + */ + function testMergeOptionsQueryString() { + $this->request->params['named'] = array( + 'page' => 10, + 'limit' => 10 + ); + $this->request->query = array( + 'page' => 99, + 'limit' => 75 + ); + $this->Paginator->settings = array( + 'page' => 1, + 'limit' => 20, + 'maxLimit' => 100, + 'paramType' => 'querystring', + ); + $result = $this->Paginator->mergeOptions('Post', array()); + $expected = array('page' => 99, 'limit' => 75, 'maxLimit' => 100, 'paramType' => 'querystring'); + $this->assertEquals($expected, $result); + + $result = $this->Paginator->mergeOptions('Post', array('page' => 100)); + $this->assertEquals(100, $result['page'], 'Passed options should replace request params'); + } + +/** + * test that the default whitelist doesn't let people screw with things they should not be allowed to. + * + * @return void + */ + function testMergeOptionsDefaultWhiteList() { + $this->request->params['named'] = array( + 'page' => 10, + 'limit' => 10, + 'fields' => array('bad.stuff'), + 'recursive' => 1000, + 'conditions' => array('bad.stuff'), + 'contain' => array('bad') + ); + $this->Paginator->settings = array( + 'page' => 1, + 'limit' => 20, + 'maxLimit' => 100, + 'paramType' => 'named', + ); + $result = $this->Paginator->mergeOptions('Post', array()); + $expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named'); + $this->assertEquals($expected, $result); + } + +/** + * test that modifying the whitelist works. + * + * @return void + */ + function testMergeOptionsExtraWhitelist() { + $this->request->params['named'] = array( + 'page' => 10, + 'limit' => 10, + 'fields' => array('bad.stuff'), + 'recursive' => 1000, + 'conditions' => array('bad.stuff'), + 'contain' => array('bad') + ); + $this->Paginator->settings = array( + 'page' => 1, + 'limit' => 20, + 'maxLimit' => 100, + 'paramType' => 'named', + ); + $result = $this->Paginator->mergeOptions('Post', array(), array('fields')); + $expected = array( + 'page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named', 'fields' => array('bad.stuff') + ); + $this->assertEquals($expected, $result); + } } \ No newline at end of file