Fix column sorting with limit subquery

The used regex found the subquery limit and added the order clause before that. Better to use Query::replaceClause here. Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
phpmyadmin · May 3, 2024 · 1d2c808 · 1d2c808
1 parent eb17438
commit 1d2c808
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 49 deletions.
diff --git a/libraries/classes/Display/Results.php b/libraries/classes/Display/Results.php
@@ -982,7 +982,6 @@ private function getMoveForwardButtonsForTableNavigation(
      * @param array              $sortDirection             sort direction
      * @param bool               $isLimitedDisplay          with limited operations
      *                                                        or not
-     * @param string             $unsortedSqlQuery          query without the sort part
      *
      * @return string html content
      */
@@ -992,8 +991,7 @@ private function getTableHeadersForColumns(
         array $sortExpression,
         array $sortExpressionNoDirection,
         array $sortDirection,
-        $isLimitedDisplay,
-        $unsortedSqlQuery
+        $isLimitedDisplay
     ) {
         // required to generate sort links that will remember whether the
         // "Show all" button has been clicked
@@ -1040,7 +1038,7 @@ private function getTableHeadersForColumns(
                     $fieldsMeta[$i],
                     $sortExpression,
                     $sortExpressionNoDirection,
-                    $unsortedSqlQuery,
+                    $analyzedSqlResults,
                     $sessionMaxRows,
                     $comments,
                     $sortDirection,
@@ -1092,7 +1090,6 @@ private function getTableHeadersForColumns(
      *
      * @param array              $displayParts              which elements to display
      * @param array              $analyzedSqlResults        analyzed sql results
-     * @param string             $unsortedSqlQuery          the unsorted sql query
      * @param array              $sortExpression            sort expression
      * @param array<int, string> $sortExpressionNoDirection sort expression without direction
      * @param array              $sortDirection             sort direction
@@ -1110,7 +1107,6 @@ private function getTableHeadersForColumns(
     private function getTableHeaders(
         array $displayParts,
         array $analyzedSqlResults,
-        $unsortedSqlQuery,
         array $sortExpression = [],
         array $sortExpressionNoDirection = [],
         array $sortDirection = [],
@@ -1161,8 +1157,7 @@ private function getTableHeaders(
             $sortExpression,
             $sortExpressionNoDirection,
             $sortDirection,
-            $isLimitedDisplay,
-            $unsortedSqlQuery
+            $isLimitedDisplay
         );
 
         // Display column at rightside - checkboxes or empty column
@@ -1513,7 +1508,7 @@ private function getCommentForRow(array $commentsMap, FieldMetadata $fieldsMeta)
      * @param FieldMetadata      $fieldsMeta                set of field properties
      * @param array              $sortExpression            sort expression
      * @param array<int, string> $sortExpressionNoDirection sort expression without direction
-     * @param string             $unsortedSqlQuery          the unsorted sql query
+     * @param array              $analyzedSqlResults        analyzed sql results
      * @param int                $sessionMaxRows            maximum rows resulted by sql
      * @param string             $comments                  comment for row
      * @param array              $sortDirection             sort direction
@@ -1536,7 +1531,7 @@ private function getOrderLinkAndSortedHeaderHtml(
         FieldMetadata $fieldsMeta,
         array $sortExpression,
         array $sortExpressionNoDirection,
-        $unsortedSqlQuery,
+        array $analyzedSqlResults,
         $sessionMaxRows,
         string $comments,
         array $sortDirection,
@@ -1565,19 +1560,16 @@ private function getOrderLinkAndSortedHeaderHtml(
             $fieldsMeta
         );
 
-        if (
-            preg_match(
-                '@(.*)([[:space:]](LIMIT (.*)|PROCEDURE (.*)|FOR UPDATE|LOCK IN SHARE MODE))@is',
-                $unsortedSqlQuery,
-                $regs3
-            )
-        ) {
-            $singleSortedSqlQuery = $regs3[1] . $singleSortOrder . $regs3[2];
-            $multiSortedSqlQuery = $regs3[1] . $multiSortOrder . $regs3[2];
-        } else {
-            $singleSortedSqlQuery = $unsortedSqlQuery . $singleSortOrder;
-            $multiSortedSqlQuery = $unsortedSqlQuery . $multiSortOrder;
-        }
+        $singleSortedSqlQuery = Query::replaceClause(
+            $analyzedSqlResults['statement'],
+            $analyzedSqlResults['parser']->list,
+            $singleSortOrder
+        );
+        $multiSortedSqlQuery = Query::replaceClause(
+            $analyzedSqlResults['statement'],
+            $analyzedSqlResults['parser']->list,
+            $multiSortOrder
+        );
 
         $singleUrlParams = [
             'db' => $this->properties['db'],
@@ -3711,7 +3703,6 @@ public function getTable(
             $this->properties['table'] = $fieldsMeta[0]->table;
         }
 
-        $unsortedSqlQuery = '';
         $sortByKeyData = [];
         // can the result be sorted?
         if ($displayParts['sort_lnk'] == '1' && isset($analyzedSqlResults['statement'])) {
@@ -3764,7 +3755,6 @@ public function getTable(
         $headers = $this->getTableHeaders(
             $displayParts,
             $analyzedSqlResults,
-            $unsortedSqlQuery,
             $sortExpression,
             $sortExpressionNoDirection,
             $sortDirection,

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -3025,6 +3025,11 @@ parameters:
 			count: 1
 			path: libraries/classes/Display/Results.php
 
+		-
+			message: "#^Method PhpMyAdmin\\\\Display\\\\Results\\:\\:getOrderLinkAndSortedHeaderHtml\\(\\) has parameter \\$analyzedSqlResults with no value type specified in iterable type array\\.$#"
+			count: 1
+			path: libraries/classes/Display/Results.php
+
 		-
 			message: "#^Method PhpMyAdmin\\\\Display\\\\Results\\:\\:getOrderLinkAndSortedHeaderHtml\\(\\) has parameter \\$sortDirection with no value type specified in iterable type array\\.$#"
 			count: 1
@@ -3306,7 +3311,7 @@ parameters:
 			path: libraries/classes/Display/Results.php
 
 		-
-			message: "#^Parameter \\#5 \\$sortExpressionNoDirection of method PhpMyAdmin\\\\Display\\\\Results\\:\\:getTableHeaders\\(\\) expects array\\<int, string\\>, array\\<int, string\\|null\\> given\\.$#"
+			message: "#^Parameter \\#4 \\$sortExpressionNoDirection of method PhpMyAdmin\\\\Display\\\\Results\\:\\:getTableHeaders\\(\\) expects array\\<int, string\\>, array\\<int, string\\|null\\> given\\.$#"
 			count: 1
 			path: libraries/classes/Display/Results.php
 

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -5997,18 +5997,22 @@
       <code>$GLOBALS['cfg']['Order']</code>
       <code>$delUrlParams</code>
     </InvalidArrayOffset>
-    <MixedArgument occurrences="57">
+    <MixedArgument occurrences="61">
       <code>$_SESSION['tmpval']['max_rows']</code>
       <code>$_SESSION['tmpval']['pos'] / $_SESSION['tmpval']['max_rows']</code>
       <code>$_SESSION['tmpval']['query']</code>
       <code>$_SESSION['tmpval']['query']</code>
       <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['parser']-&gt;list</code>
+      <code>$analyzedSqlResults['parser']-&gt;list</code>
+      <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['select_tables']</code>
       <code>$analyzedSqlResults['statement']</code>
       <code>$analyzedSqlResults['statement']</code>
       <code>$analyzedSqlResults['statement']</code>
+      <code>$analyzedSqlResults['statement']</code>
+      <code>$analyzedSqlResults['statement']</code>
       <code>$analyzedSqlResults['statement']-&gt;from</code>
       <code>$clause</code>
       <code>$clauseIsUnique</code>
@@ -6278,7 +6282,8 @@
       <code>$firstShownRec</code>
       <code>$sortExpressionNoDirection[$indexInExpression]</code>
     </MixedOperand>
-    <MixedPropertyFetch occurrences="16">
+    <MixedPropertyFetch occurrences="17">
+      <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['parser']-&gt;list</code>
       <code>$analyzedSqlResults['statement']-&gt;expr</code>

diff --git a/test/classes/Display/ResultsTest.php b/test/classes/Display/ResultsTest.php
@@ -1466,13 +1466,13 @@ public function testGetTable(): void
                 [
                     'column_name' => 'id',
                     'order_link' => '<a href="index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60id%60+ASC'
-                        . '&sql_signature=dcfe20b407b35309f6af81f745e77a10f723d39b082d2a8f9cb8e75b17c4d3ce'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60id%60+ASC+'
+                        . '&sql_signature=5b09494be0beb7899b460ba6b695504ca89d1ad1fbc8705f3b60f7da71f61b2f'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en" class="sortlink">id'
                         . '<input type="hidden" value="'
                         . 'index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60id%60+ASC'
-                        . '&sql_signature=dcfe20b407b35309f6af81f745e77a10f723d39b082d2a8f9cb8e75b17c4d3ce'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60id%60+ASC+'
+                        . '&sql_signature=5b09494be0beb7899b460ba6b695504ca89d1ad1fbc8705f3b60f7da71f61b2f'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en"></a>'
                         . '<input type="hidden" name="url-remove-order" value="index.php?route=/sql&db=test_db'
                         . '&table=test_table&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60'
@@ -1481,8 +1481,8 @@ public function testGetTable(): void
                         . '&discard_remembered_sort=1">' . "\n"
                         . '<input type="hidden" name="url-add-order" value="'
                         . 'index.php?route=/sql&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60id%60+ASC'
-                        . '&sql_signature=dcfe20b407b35309f6af81f745e77a10f723d39b082d2a8f9cb8e75b17c4d3ce'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60id%60+ASC+'
+                        . '&sql_signature=5b09494be0beb7899b460ba6b695504ca89d1ad1fbc8705f3b60f7da71f61b2f'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en">',
                     'comments' => '',
                     'is_browse_pointer_enabled' => true,
@@ -1493,13 +1493,13 @@ public function testGetTable(): void
                 [
                     'column_name' => 'name',
                     'order_link' => '<a href="index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60name%60+ASC'
-                        . '&sql_signature=0d06fa8d6795b1c69892cca27d6213c08401bd434145d16cb35c365ab3e03039'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60name%60+ASC+'
+                        . '&sql_signature=deb7ae82acc39ae4faa69b87f757edb5c3a6a714196d2f5fefe5cccc06985aba'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en" class="sortlink">name'
                         . '<input type="hidden" value="'
                         . 'index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60name%60+ASC'
-                        . '&sql_signature=0d06fa8d6795b1c69892cca27d6213c08401bd434145d16cb35c365ab3e03039'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60name%60+ASC+'
+                        . '&sql_signature=deb7ae82acc39ae4faa69b87f757edb5c3a6a714196d2f5fefe5cccc06985aba'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en"></a>'
                         . '<input type="hidden" name="url-remove-order" value="index.php?route=/sql&db=test_db'
                         . '&table=test_table&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60'
@@ -1508,8 +1508,8 @@ public function testGetTable(): void
                         . '&discard_remembered_sort=1">' . "\n"
                         . '<input type="hidden" name="url-add-order" value="'
                         . 'index.php?route=/sql&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0AORDER+BY+%60name%60+ASC'
-                        . '&sql_signature=0d06fa8d6795b1c69892cca27d6213c08401bd434145d16cb35c365ab3e03039'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0AORDER+BY+%60name%60+ASC+'
+                        . '&sql_signature=deb7ae82acc39ae4faa69b87f757edb5c3a6a714196d2f5fefe5cccc06985aba'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en">',
                     'comments' => '',
                     'is_browse_pointer_enabled' => true,
@@ -1520,15 +1520,15 @@ public function testGetTable(): void
                 [
                     'column_name' => 'datetimefield',
                     'order_link' => '<a href="index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0A'
-                        . 'ORDER+BY+%60datetimefield%60+DESC'
-                        . '&sql_signature=1c46f7e3c625f9e0846fb2de844ca1732319e5fb7fb93e96c89a4b6218579358'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0A'
+                        . 'ORDER+BY+%60datetimefield%60+DESC+'
+                        . '&sql_signature=d7f66b34e106a07349e748fa1f6c517fb33e0a717c285b623d10e7f0e24a3db4'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en" class="sortlink">datetimefield'
                         . '<input type="hidden" value="'
                         . 'index.php?route=/sql&server=0&lang=en&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0A'
-                        . 'ORDER+BY+%60datetimefield%60+DESC'
-                        . '&sql_signature=1c46f7e3c625f9e0846fb2de844ca1732319e5fb7fb93e96c89a4b6218579358'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0A'
+                        . 'ORDER+BY+%60datetimefield%60+DESC+'
+                        . '&sql_signature=d7f66b34e106a07349e748fa1f6c517fb33e0a717c285b623d10e7f0e24a3db4'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en"></a>'
                         . '<input type="hidden" name="url-remove-order" value="index.php?route=/sql&db=test_db'
                         . '&table=test_table&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60'
@@ -1537,9 +1537,9 @@ public function testGetTable(): void
                         . '&discard_remembered_sort=1">' . "\n"
                         . '<input type="hidden" name="url-add-order" value="'
                         . 'index.php?route=/sql&db=test_db&table=test_table'
-                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60++%0A'
-                        . 'ORDER+BY+%60datetimefield%60+DESC'
-                        . '&sql_signature=1c46f7e3c625f9e0846fb2de844ca1732319e5fb7fb93e96c89a4b6218579358'
+                        . '&sql_query=SELECT+%2A+FROM+%60test_db%60.%60test_table%60+%0A'
+                        . 'ORDER+BY+%60datetimefield%60+DESC+'
+                        . '&sql_signature=d7f66b34e106a07349e748fa1f6c517fb33e0a717c285b623d10e7f0e24a3db4'
                         . '&session_max_rows=25&is_browse_distinct=0&server=0&lang=en">',
                     'comments' => '',
                     'is_browse_pointer_enabled' => true,