From cfde3c1958d72a57a72617c5fed4462161c550f7 Mon Sep 17 00:00:00 2001
From: Bryan Davis <bd808@wikimedia.org>
Date: Tue, 8 Oct 2013 12:14:52 -0600
Subject: [PATCH] Escape backticks when quoting MySQL identifiers

Previously mysql_real_escape_string() was improperly used which escapes
for inclusion in a plain string rather than an identifier.

Also adds basic test support for the DatabaseMysqlBase class.

Co-Authored by: Antoine Musso <hashar@free.fr>
Bug: 55427
Change-Id: Ic045e195c89d8d5d0f0edbda0cd1df781de7025c
---
 includes/db/DatabaseMysqlBase.php             |   4 +-
 .../includes/db/DatabaseMysqlBaseTest.php     | 128 ++++++++++++++++++
 2 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 tests/phpunit/includes/db/DatabaseMysqlBaseTest.php

diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php
index 5614ed209b..d33d7c71be 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -469,7 +469,9 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
 	 * @return string
 	 */
 	public function addIdentifierQuotes( $s ) {
-		return "`" . $this->strencode( $s ) . "`";
+		// Characters in the range \u0001-\uFFFF are valid in a quoted identifier
+		// Remove NUL bytes and escape backticks by doubling
+		return '`' . str_replace( array( "\0", '`' ), array( '', '``' ), $s )  . '`';
 	}
 
 	/**
diff --git a/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
new file mode 100644
index 0000000000..75c48374d2
--- /dev/null
+++ b/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
@@ -0,0 +1,128 @@
+<?php
+/**
+ * Holds tests for DatabaseMysqlBase MediaWiki class.
+ *
+ * @section LICENSE
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @author Antoine Musso
+ * @author Bryan Davis
+ * @copyright © 2013 Antoine Musso
+ * @copyright © 2013 Bryan Davis
+ * @copyright © 2013 Wikimedia Foundation Inc.
+ */
+
+/**
+ * Fake class around abstract class so we can call concrete methods.
+ */
+class FakeDatabaseMysqlBase extends DatabaseMysqlBase {
+	// From DatabaseBase
+	protected function closeConnection() {}
+	protected function doQuery( $sql ) {}
+
+	// From DatabaseMysql
+	protected function mysqlConnect( $realServer ) {}
+	protected function mysqlFreeResult( $res ) {}
+	protected function mysqlFetchObject( $res ) {}
+	protected function mysqlFetchArray( $res ) {}
+	protected function mysqlNumRows( $res ) {}
+	protected function mysqlNumFields( $res ) {}
+	protected function mysqlFieldName( $res, $n ) {}
+	protected function mysqlDataSeek( $res, $row ) {}
+	protected function mysqlError( $conn = null ) {}
+	protected function mysqlFetchField( $res, $n ) {}
+	protected function mysqlPing() {}
+
+	// From interface DatabaseType
+	function insertId() {}
+	function lastErrno() {}
+	function affectedRows() {}
+	function getServerVersion() {}
+}
+
+class DatabaseMysqlBaseTest extends MediaWikiTestCase {
+
+	/**
+	 * @dataProvider provideDiapers
+	 */
+	function testAddIdentifierQuotes( $expected, $in ) {
+		$db = new FakeDatabaseMysqlBase();
+		$quoted = $db->addIdentifierQuotes( $in );
+		$this->assertEquals($expected, $quoted);
+	}
+
+
+	/**
+	 * Feeds testAddIdentifierQuotes
+	 *
+	 * Named per bug 20281 convention.
+	 */
+	function provideDiapers() {
+		return array(
+			// Format: expected, input
+			array( '``', '' ),
+
+			// Yeah I really hate loosely typed PHP idiocies nowadays
+			array( '``', null ),
+
+			// Dear codereviewer, guess what addIdentifierQuotes()
+			// will return with thoses:
+			array( '``', false ),
+			array( '`1`', true ),
+			array( '`Array`', array() ),
+			//array( '`Object`', new stdClass() ),
+			// ^ Error: Object of class stdClass could not be converted to string
+
+			// We never know what could happen
+			array( '`0`', 0 ),
+			array( '`1`', 1 ),
+
+			// Whatchout! Should probably use something more meaningful
+			array( "`'`", "'" ),  # single quote
+			array( '`"`', '"' ),  # double quote
+			array( '````', '`' ), # backtick
+			array( '`’`', '’' ),  # apostrophe (look at your encyclopedia)
+
+			// sneaky NUL bytes are lurking everywhere
+			array( '``', "\0" ),
+			array( '`xyzzy`', "\0x\0y\0z\0z\0y\0" ),
+
+			// unicode chars
+			array(
+				self::createUnicodeString( '`\u0001a\uFFFFb`' ),
+				self::createUnicodeString( '\u0001a\uFFFFb' )
+			),
+			array(
+				self::createUnicodeString( '`\u0001\uFFFF`' ),
+				self::createUnicodeString( '\u0001\u0000\uFFFF\u0000' )
+			),
+			array( '`☃`', '☃' ),
+			array( '`メインページ`', 'メインページ' ),
+			array( '`Басты_бет`', 'Басты_бет' ),
+
+			// Real world:
+			array( '`Alix`', 'Alix' ),  # while( ! $recovered ) { sleep(); }
+			array( '`Backtick: ```', 'Backtick: `' ),
+			array( '`This is a test`', 'This is a test' ),
+		);
+	}
+
+	private static function createUnicodeString($str) {
+		return json_decode( '"' . $str . '"' );
+	}
+
+}
-- 
2.20.1