rdbms: in Database::selectSQLText, do not treat $conds = "0" as no condition

This fixes an issue that arises because empty( "0" ) is true in PHP.

The new behavior rejects any conditions that are not strings or arrays,
and lets $conds = "0" be passed to the databases as WHERE 0.

Some databases may reject this as invalid syntax, which is the expected
behavior here, instead of silently ignoring the 0, causing no condition to
be applied to the query.

Bug: T188314
Change-Id: I5bc4d7f41221a886c85e54d9da67c4c095a7d9ce

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index d5fc357..ddc8df5 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1435,14 +1435,27 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                list( $startOpts, $useIndex, $preLimitTail, $postLimitTail, $ignoreIndex ) =
                        $this->makeSelectOptions( $options );
 
-               if ( !empty( $conds ) ) {
-                       if ( is_array( $conds ) ) {
-                               $conds = $this->makeList( $conds, self::LIST_AND );
-                       }
+               if ( is_array( $conds ) ) {
+                       $conds = $this->makeList( $conds, self::LIST_AND );
+               }
+
+               if ( $conds === null || $conds === false ) {
+                       $this->queryLogger->warning(
+                               __METHOD__
+                               . ' called from '
+                               . $fname
+                               . ' with incorrect parameters: $conds must be a string or an array'
+                       );
+                       $conds = '';
+               }
+
+               if ( $conds === '' ) {
+                       $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex $preLimitTail";
+               } elseif ( is_string( $conds ) ) {
                        $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex " .
                                "WHERE $conds $preLimitTail";
                } else {
-                       $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex $preLimitTail";
+                       throw new DBUnexpectedError( $this, __METHOD__ . ' called with incorrect parameters' );
                }
 
                if ( isset( $options['LIMIT'] ) ) {

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 3d1fe1a..5c1943b 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -64,6 +64,44 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                        "FROM table " .
                                        "WHERE alias = 'text'"
                        ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => 'alias = \'text\'',
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table " .
+                               "WHERE alias = 'text'"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => [],
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => '',
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => '0', // T188314
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table " .
+                               "WHERE 0"
+                       ],
                        [
                                [
                                        // 'tables' with space prepended indicates pre-escaped table name