From 77e0a4abba72bbe533b627c045585f5497b854d5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 14 Mar 2015 13:27:47 -0700 Subject: [PATCH] Guarantee that Maniphest paging clauses strictly progress Ref T7548. Some of these clauses are not guaranteed to select only rows following the cursor. --- .../maniphest/query/ManiphestTaskQuery.php | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index c8883c4145..c8995a9e4e 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -1054,11 +1054,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { $app_columns = $this->buildApplicationSearchPagination($conn_r, $cursor); if ($app_columns) { $columns = array_merge($columns, $app_columns); - $columns[] = array( - 'name' => 'task.id', - 'value' => (int)$cursor->getID(), - 'type' => 'int', - ); } else { switch ($this->orderBy) { case self::ORDER_PRIORITY: @@ -1082,11 +1077,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { ); break; case self::ORDER_CREATED: - $columns[] = array( - 'name' => 'task.id', - 'value' => (int)$cursor->getID(), - 'type' => 'int', - ); + // This just uses the ID column, below. break; case self::ORDER_MODIFIED: $columns[] = array( @@ -1101,17 +1092,18 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { 'value' => $cursor->getTitle(), 'type' => 'string', ); - $columns[] = array( - 'name' => 'task.id', - 'value' => $cursor->getID(), - 'type' => 'int', - ); break; default: throw new Exception("Unknown order query '{$this->orderBy}'!"); } } + $columns[] = array( + 'name' => 'task.id', + 'value' => $cursor->getID(), + 'type' => 'int', + ); + return $this->buildPagingClauseFromMultipleColumns( $conn_r, $columns,