Skip to content

Commit

Permalink
Pulling out parameter merging logic into a method, this allows specif…
Browse files Browse the repository at this point in the history 
…ic typing of parameter merging (named, querystring, route). Also simplifies whitelisting of request parameters.

Tests added for merging and whitelisting.
  • Loading branch information
@markstory
markstory committed Dec 19, 2010
1 parent 7585b29 commit 6b3db0a
Show file tree
Hide file tree
Showing 2 changed files with 216 additions and 33 deletions.
59 changes: 59 additions & 0 deletions cake/libs/controller/components/paginator.php
View file
Expand Up @@ -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
*
Expand Down Expand Up @@ -83,6 +94,9 @@ public function paginate($object = null, $scope = array(), $whitelist = array())
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,
Expand Down Expand Up @@ -279,4 +293,49 @@ protected function _getObject($object) {
}
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);
}
}
190 changes: 157 additions & 33 deletions cake/tests/cases/libs/controller/components/paginator.test.php
View file
Expand Up @@ -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'));

/**
Expand Down Expand Up @@ -210,17 +211,28 @@ 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
*
* @access public
* @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();
Expand Down Expand Up @@ -306,10 +318,7 @@ function testPaginate() {
* @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';
Expand Down Expand Up @@ -352,7 +361,7 @@ function testPaginateExtraParams() {
$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();
Expand Down Expand Up @@ -400,10 +409,7 @@ function testPaginateExtraParams() {
* @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();
Expand Down Expand Up @@ -440,10 +446,7 @@ public function testPaginatePassedArgs() {
* @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();
Expand All @@ -469,10 +472,7 @@ function testPaginateSpecialType() {
* @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();
Expand All @@ -491,10 +491,7 @@ function testDefaultPaginateParams() {
* @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();
Expand Down Expand Up @@ -522,10 +519,7 @@ function testPaginateOrderVirtualField() {
* @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');
}
Expand All @@ -537,10 +531,7 @@ function testPaginateMissingModel() {
* @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';
Expand Down Expand Up @@ -568,4 +559,137 @@ function testPaginateMaxLimit() {
$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);
}
}

0 comments on commit 6b3db0a

Please sign in to comment.