Clamp limit values to be unsigned integers.

This solves large page numbers potentially turning into scientific notation when being formatted into queries. It also further safeguards against SQL manipulation. Refs #GH-1263
cakephp · May 3, 2013 · 2096d3f · 2096d3f
1 parent bd3be28
commit 2096d3f
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 25 deletions.
diff --git a/lib/Cake/Controller/Component/PaginatorComponent.php b/lib/Cake/Controller/Component/PaginatorComponent.php
@@ -212,6 +212,9 @@ public function paginate($object = null, $scope = array(), $whitelist = array())
 		$pageCount = intval(ceil($count / $limit));
 		$requestedPage = $page;
 		$page = max(min($page, $pageCount), 1);
+		if ($requestedPage > $page) {
+			throw new NotFoundException();
+		}
 
 		$paging = array(
 			'page' => $page,
@@ -234,10 +237,6 @@ public function paginate($object = null, $scope = array(), $whitelist = array())
 			array($object->alias => $paging)
 		);
 
-		if ($requestedPage > $page) {
-			throw new NotFoundException();
-		}
-
 		if (
 			!in_array('Paginator', $this->Controller->helpers) &&
 			!array_key_exists('Paginator', $this->Controller->helpers)

diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php
@@ -2672,16 +2672,13 @@ protected function _quoteMatchedField($match) {
  */
 	public function limit($limit, $offset = null) {
 		if ($limit) {
-			$rt = '';
-			if (!strpos(strtolower($limit), 'limit')) {
-				$rt = ' LIMIT';
-			}
+			$rt = ' LIMIT';
 
 			if ($offset) {
-				$rt .= ' ' . $offset . ',';
+				$rt .= sprintf(' %u,', $offset);
 			}
 
-			$rt .= ' ' . $limit;
+			$rt .= sprintf(' %u', $limit);
 			return $rt;
 		}
 		return null;

diff --git a/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php b/lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php
@@ -896,44 +896,35 @@ public function testOutOfRangePageNumberGetsClamped() {
  * @expectedException NotFoundException
  * @return void
  */
-	public function testOutOfVeryBigRangePageNumberGetsClamped() {
+	public function testOutOfVeryBigPageNumberGetsClamped() {
 		$Controller = new PaginatorTestController($this->request);
 		$Controller->uses = array('PaginatorControllerPost');
 		$Controller->params['named'] = array(
-			'page' => 3000000000000000000000000,
+			'page' => '3000000000000000000000000',
 		);
 		$Controller->constructClasses();
 		$Controller->PaginatorControllerPost->recursive = 0;
 		$Controller->Paginator->paginate('PaginatorControllerPost');
-	}	
+	}
 
 /**
  * testOutOfRangePageNumberAndPageCountZero
  *
+ * @expectedException NotFoundException
  * @return void
  */
 	public function testOutOfRangePageNumberAndPageCountZero() {
 		$Controller = new PaginatorTestController($this->request);
 		$Controller->uses = array('PaginatorControllerPost');
 		$Controller->params['named'] = array(
-			'page' => 3000,
+			'page' => '3000',
 		);
 		$Controller->constructClasses();
 		$Controller->PaginatorControllerPost->recursive = 0;
 		$Controller->paginate = array(
 			'conditions' => array('PaginatorControllerPost.id >' => 100)
 		);
-		try {
-			$Controller->Paginator->paginate('PaginatorControllerPost');
-		} catch (NotFoundException $e) {
-			$this->assertEquals(
-				1,
-				$Controller->request->params['paging']['PaginatorControllerPost']['page'],
-				'Page number should not be 0'
-			);
-			return;
-		}
-		$this->fail();
+		$Controller->Paginator->paginate('PaginatorControllerPost');
 	}
 
 /**

diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php
@@ -1229,4 +1229,28 @@ public function testConditionKeysToStringVirtualField() {
 		$this->assertEquals($expected, $result[0]);
 	}
 
+/**
+ * Test the limit function.
+ *
+ * @return void
+ */
+	public function testLimit() {
+		$db = new DboTestSource;
+
+		$result = $db->limit('0');
+		$this->assertNull($result);
+
+		$result = $db->limit('10');
+		$this->assertEquals(' LIMIT 10', $result);
+
+		$result = $db->limit('FARTS', 'BOOGERS');
+		$this->assertEquals(' LIMIT 0, 0', $result);
+
+		$result = $db->limit(20, 10);
+		$this->assertEquals(' LIMIT 10, 20', $result);
+
+		$result = $db->limit(10, 300000000000000000000000000000);
+		$this->assertEquals(' LIMIT 0, 10', $result);
+	}
+
 }