Browse Source

Handle bad limit/maxLimit values.

Originally I had thought it would be best to handle these cases with
exceptions. However, in reviewing the existing test cases, we coalesce
bad user input into limit=1. Because the maxLimit *could* be whitelisted
as an end user parameter, I think massaging the data is more reasonable.

Refs #8926
Mark Story 9 years ago
parent
commit
a2adbf74c7

+ 1 - 1
src/Controller/Component/PaginatorComponent.php

@@ -386,7 +386,7 @@ class PaginatorComponent extends Component
         if (empty($options['limit']) || $options['limit'] < 1) {
             $options['limit'] = 1;
         }
-        $options['limit'] = min($options['limit'], $options['maxLimit']);
+        $options['limit'] = max(min($options['limit'], $options['maxLimit']), 1);
         return $options;
     }
 }

+ 46 - 15
tests/TestCase/Controller/Component/PaginatorComponentTest.php

@@ -753,27 +753,58 @@ class PaginatorComponentTest extends TestCase
         $this->assertEquals([], $result['order']);
     }
 
+    public function checkLimitProvider()
+    {
+        return [
+            'out of bounds' => [
+                ['limit' => 1000000, 'maxLimit' => 100],
+                100,
+            ],
+            'limit is nan' => [
+                ['limit' => 'sheep!', 'maxLimit' => 100],
+                1,
+            ],
+            'negative limit' => [
+                ['limit' => '-1', 'maxLimit' => 100],
+                1,
+            ],
+            'unset limit' => [
+                ['limit' => null, 'maxLimit' => 100],
+                1,
+            ],
+            'limit = 0' => [
+                ['limit' => 0, 'maxLimit' => 100],
+                1,
+            ],
+            'limit = 0' => [
+                ['limit' => 0, 'maxLimit' => 0],
+                1,
+            ],
+            'limit = null' => [
+                ['limit' => null, 'maxLimit' => 0],
+                1,
+            ],
+            'bad input, results in 1' => [
+                ['limit' => null, 'maxLimit' => null],
+                1,
+            ],
+            'bad input, results in 1' => [
+                ['limit' => false, 'maxLimit' => false],
+                1,
+            ],
+        ];
+    }
+
     /**
      * test that maxLimit is respected
      *
+     * @dataProvider checkLimitProvider
      * @return void
      */
-    public function testCheckLimit()
+    public function testCheckLimit($input, $expected)
     {
-        $result = $this->Paginator->checkLimit(['limit' => 1000000, 'maxLimit' => 100]);
-        $this->assertEquals(100, $result['limit']);
-
-        $result = $this->Paginator->checkLimit(['limit' => 'sheep!', 'maxLimit' => 100]);
-        $this->assertEquals(1, $result['limit']);
-
-        $result = $this->Paginator->checkLimit(['limit' => '-1', 'maxLimit' => 100]);
-        $this->assertEquals(1, $result['limit']);
-
-        $result = $this->Paginator->checkLimit(['limit' => null, 'maxLimit' => 100]);
-        $this->assertEquals(1, $result['limit']);
-
-        $result = $this->Paginator->checkLimit(['limit' => 0, 'maxLimit' => 100]);
-        $this->assertEquals(1, $result['limit']);
+        $result = $this->Paginator->checkLimit($input);
+        $this->assertSame($expected, $result['limit']);
     }
 
     /**