From: daniel <daniel.kinzler@wikimedia.de>
Date: Tue, 27 Feb 2018 18:08:47 +0000 (+0100)
Subject: rdbms: in Database::selectSQLText, do not treat $conds = "0" as no condition
X-Git-Tag: 1.31.0-rc.0~468
X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=fa9f1aca87cd1481bee9ae50928e95241832e350;p=lhc%2Fweb%2Fwiklou.git

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 d5fc357f64..ddc8df5c14 100644
--- 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 3d1fe1a1a6..5c1943b128 100644
--- 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