diff --git a/cake/libs/controller/components/paginator.php b/cake/libs/controller/components/paginator.php index 7fbae137a..c857b573e 100644 --- a/cake/libs/controller/components/paginator.php +++ b/cake/libs/controller/components/paginator.php @@ -74,7 +74,7 @@ class PaginatorComponent extends Component { ); /** - * A list of request parameters users are allowed to set. Modifying + * A list of parameters users are allowed to set using request parameters. Modifying * this list will allow users to have more influence over pagination, * be careful with what you permit. * @@ -101,7 +101,8 @@ class PaginatorComponent extends Component { * * @param mixed $object Model to paginate (e.g: model instance, or 'Model', or 'Model.InnerModel') * @param mixed $scope Additional find conditions to use while paginating - * @param array $whitelist List of allowed options for paging + * @param array $whitelist List of allowed fields for ordering. This allows you to prevent ordering + * on non-indexed, or undesirable columns. * @return array Model query results */ public function paginate($object = null, $scope = array(), $whitelist = array()) { @@ -117,8 +118,8 @@ class PaginatorComponent extends Component { throw new MissingModelException($object); } - $options = $this->mergeOptions($object->alias, $whitelist); - $options = $this->validateSort($object, $options); + $options = $this->mergeOptions($object->alias); + $options = $this->validateSort($object, $options, $whitelist); $options = $this->checkLimit($options); $conditions = $fields = $order = $limit = $page = $recursive = null; @@ -272,11 +273,9 @@ 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 $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, $whitelist = array()) { + public function mergeOptions($alias) { $defaults = $this->getDefaults($alias); switch ($defaults['paramType']) { case 'named': @@ -285,14 +284,8 @@ class PaginatorComponent extends Component { 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); - + $request = array_intersect_key($request, array_flip($this->whitelist)); return array_merge($defaults, $request); } @@ -322,9 +315,10 @@ class PaginatorComponent extends Component { * * @param Model $object The model being paginated. * @param array $options The pagination options being used for this request. + * @param array $whitelist The list of columns that can be used for sorting. If empty all keys are allowed. * @return array An array of options with sort + direction removed and replaced with order if possible. */ - public function validateSort($object, $options) { + public function validateSort($object, $options, $whitelist = array()) { if (isset($options['sort'])) { $direction = null; if (isset($options['direction'])) { @@ -335,6 +329,13 @@ class PaginatorComponent extends Component { } $options['order'] = array($options['sort'] => $direction); } + + if (!empty($whitelist)) { + $field = key($options['order']); + if (!in_array($field, $whitelist)) { + $options['order'] = null; + } + } if (!empty($options['order']) && is_array($options['order'])) { $alias = $object->alias ; diff --git a/cake/tests/cases/libs/controller/components/paginator.test.php b/cake/tests/cases/libs/controller/components/paginator.test.php index e8c925fc4..cbaecb2b9 100644 --- a/cake/tests/cases/libs/controller/components/paginator.test.php +++ b/cake/tests/cases/libs/controller/components/paginator.test.php @@ -635,7 +635,8 @@ class PaginatorTest extends CakeTestCase { 'maxLimit' => 100, 'paramType' => 'named', ); - $result = $this->Paginator->mergeOptions('Post', array('fields')); + $this->Paginator->whitelist[] = 'fields'; + $result = $this->Paginator->mergeOptions('Post'); $expected = array( 'page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named', 'fields' => array('bad.stuff') ); @@ -658,6 +659,22 @@ class PaginatorTest extends CakeTestCase { $this->assertEquals('asc', $result['order']['model.something']); } +/** + * test that fields not in whitelist won't be part of order conditions. + * + * @return void + */ + function testValidateSortWhitelistFailure() { + $model = $this->getMock('Model'); + $model->alias = 'model'; + $model->expects($this->any())->method('hasField')->will($this->returnValue(true)); + + $options = array('sort' => 'body', 'direction' => 'asc'); + $result = $this->Paginator->validateSort($model, $options, array('title', 'id')); + + $this->assertNull($result['order']); + } + /** * test that virtual fields work. *