diff --git a/cake/libs/controller/components/paginator.php b/cake/libs/controller/components/paginator.php index 847ea3ead..5d05a1a9e 100644 --- a/cake/libs/controller/components/paginator.php +++ b/cake/libs/controller/components/paginator.php @@ -77,7 +77,7 @@ class PaginatorComponent extends Component { * Handles automatic pagination of model records. * * @param mixed $object Model to paginate (e.g: model instance, or 'Model', or 'Model.InnerModel') - * @param mixed $scope Conditions to use while paginating + * @param mixed $scope Additional find conditions to use while paginating * @param array $whitelist List of allowed options for paging * @return array Model query results */ @@ -87,7 +87,6 @@ class PaginatorComponent extends Component { $scope = $object; $object = null; } - $assoc = null; $object = $this->_getObject($object); @@ -95,8 +94,9 @@ class PaginatorComponent extends Component { throw new MissingModelException($object); } - $options = $this->mergeOptions($object->alias, $scope, $whitelist); + $options = $this->mergeOptions($object->alias, $whitelist); $options = $this->validateSort($object, $options); + $options = $this->checkLimit($options); $conditions = $fields = $order = $limit = $page = $recursive = null; @@ -111,13 +111,6 @@ class PaginatorComponent extends Component { unset($options[0]); } - $options = array_merge(array('page' => 1, 'limit' => 20, 'maxLimit' => 100), $options); - $options['limit'] = (int) $options['limit']; - if (empty($options['limit']) || $options['limit'] < 1) { - $options['limit'] = 1; - } - $options['limit'] = min((int)$options['limit'], $options['maxLimit']); - extract($options); if (is_array($scope) && !empty($scope)) { @@ -248,20 +241,20 @@ class PaginatorComponent extends Component { * * @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()) { + public function mergeOptions($alias, $whitelist = array()) { if (isset($this->settings[$alias])) { $defaults = $this->settings[$alias]; } else { $defaults = $this->settings; } - if (empty($defaults['paramType'])) { - $defaults['paramType'] = 'named'; - } + $defaults = array_merge( + array('page' => 1, 'limit' => 20, 'maxLimit' => 100, 'paramType' => 'named'), + $defaults + ); switch ($defaults['paramType']) { case 'named': $request = $this->Controller->request->params['named']; @@ -277,7 +270,7 @@ class PaginatorComponent extends Component { $whitelist = array_flip(array_merge($this->whitelist, $whitelist)); $request = array_intersect_key($request, $whitelist); - return array_merge($defaults, $request, $options); + return array_merge($defaults, $request); } /** @@ -322,4 +315,19 @@ class PaginatorComponent extends Component { return $options; } + +/** + * Check the limit parameter and ensure its within the maxLimit bounds. + * + * @param array $options An array of options with a limit key to be checked. + * @return array An array of options for pagination + */ + public function checkLimit($options) { + $options['limit'] = (int) $options['limit']; + if (empty($options['limit']) || $options['limit'] < 1) { + $options['limit'] = 1; + } + $options['limit'] = min((int)$options['limit'], $options['maxLimit']); + return $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 9fb22993e..ecb9f812f 100644 --- a/cake/tests/cases/libs/controller/components/paginator.test.php +++ b/cake/tests/cases/libs/controller/components/paginator.test.php @@ -582,14 +582,11 @@ class PaginatorTest extends CakeTestCase { 'paramType' => 'named', ) ); - $result = $this->Paginator->mergeOptions('Silly', array()); + $result = $this->Paginator->mergeOptions('Silly'); $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); + $result = $this->Paginator->mergeOptions('Post'); + $expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'maxLimit' => 50); $this->assertEquals($expected, $result); } @@ -609,12 +606,9 @@ class PaginatorTest extends CakeTestCase { 'maxLimit' => 100, 'paramType' => 'named', ); - $result = $this->Paginator->mergeOptions('Post', array()); + $result = $this->Paginator->mergeOptions('Post'); $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'); } /** @@ -637,12 +631,9 @@ class PaginatorTest extends CakeTestCase { 'maxLimit' => 100, 'paramType' => 'querystring', ); - $result = $this->Paginator->mergeOptions('Post', array()); + $result = $this->Paginator->mergeOptions('Post'); $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'); } /** @@ -665,7 +656,7 @@ class PaginatorTest extends CakeTestCase { 'maxLimit' => 100, 'paramType' => 'named', ); - $result = $this->Paginator->mergeOptions('Post', array()); + $result = $this->Paginator->mergeOptions('Post'); $expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named'); $this->assertEquals($expected, $result); } @@ -690,7 +681,7 @@ class PaginatorTest extends CakeTestCase { 'maxLimit' => 100, 'paramType' => 'named', ); - $result = $this->Paginator->mergeOptions('Post', array(), array('fields')); + $result = $this->Paginator->mergeOptions('Post', array('fields')); $expected = array( 'page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named', 'fields' => array('bad.stuff') ); @@ -737,4 +728,26 @@ class PaginatorTest extends CakeTestCase { $this->assertEquals('desc', $result['order']['something']); } + +/** + * test that maxLimit is respected + * + * @return void + */ + function testCheckLimit() { + $result = $this->Paginator->checkLimit(array('limit' => 1000000, 'maxLimit' => 100)); + $this->assertEquals(100, $result['limit']); + + $result = $this->Paginator->checkLimit(array('limit' => 'sheep!', 'maxLimit' => 100)); + $this->assertEquals(1, $result['limit']); + + $result = $this->Paginator->checkLimit(array('limit' => '-1', 'maxLimit' => 100)); + $this->assertEquals(1, $result['limit']); + + $result = $this->Paginator->checkLimit(array('limit' => null, 'maxLimit' => 100)); + $this->assertEquals(1, $result['limit']); + + $result = $this->Paginator->checkLimit(array('limit' => 0, 'maxLimit' => 100)); + $this->assertEquals(1, $result['limit']); + } } \ No newline at end of file