From: Kevin Israel <pleasestand@live.com>
Date: Wed, 18 Jun 2014 00:18:21 +0000 (-0400)
Subject: Database: Remove use of strencode() in replaceVars()
X-Git-Tag: 1.31.0-rc.0~13105^2
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=c8c102359682f1c59ea304d3865a984fa95dfafc;p=lhc%2Fweb%2Fwiklou.git

Database: Remove use of strencode() in replaceVars()

Almost all known uses of the /*$var*/ syntax are for either the
table prefix or table options, neither of which need string
encoding. Use of that syntax inside strings has been discouraged
for a while anyway.

This is to allow deprecating strencode().

Also changed the method to replace in one pass (to not find
variables inside values of other variables).

Change-Id: Ia697ce5f14082e737b3e1c4e174bd3f6223acc5c
---

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 5d423a1c0d..2b347b983f 100644
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -254,6 +254,18 @@ changes to languages because of Bugzilla reports.
   ContentHandlerDefaultModelFor hook since MediaWiki 1.21.
 * Deprecated the TitleIsWikitextPage hook. Superseded by the
   ContentHandlerDefaultModelFor hook since MediaWiki 1.21.
+* Changed parsing of variables in schema (.sql) files:
+** The substituted values are no longer parsed. (Formerly, several passes
+   were made for each variable, so depending on the order in which variables
+   were defined, variables might have been found inside encoded values. This
+   is no longer the case.)
+** Variables are no longer string encoded when the /*$var*/ syntax is used.
+   If string encoding is necessary, use the '{$var}' syntax instead.
+** Variable names must only consist of one or more of the characters
+   "A-Za-z0-9_".
+** In source text of the form '{$A}'{$B}' or `{$A}`{$B}`, where variable A
+   does not exist yet variable B does, the latter may not be replaced.
+   However, this difference is unlikely to arise in practice.
 
 == Compatibility ==
 
diff --git a/includes/db/Database.php b/includes/db/Database.php
index fc13eebbec..11246172bd 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3907,47 +3907,49 @@ abstract class DatabaseBase implements IDatabase {
 	 *
 	 * - '{$var}' should be used for text and is passed through the database's
 	 *   addQuotes method.
-	 * - `{$var}` should be used for identifiers (eg: table and database names),
-	 *   it is passed through the database's addIdentifierQuotes method which
+	 * - `{$var}` should be used for identifiers (e.g. table and database names).
+	 *   It is passed through the database's addIdentifierQuotes method which
 	 *   can be overridden if the database uses something other than backticks.
-	 * - / *$var* / is just encoded, besides traditional table prefix and
-	 *   table options its use should be avoided.
+	 * - / *_* / or / *$wgDBprefix* / passes the name that follows through the
+	 *   database's tableName method.
+	 * - / *i* / passes the name that follows through the database's indexName method.
+	 * - In all other cases, / *$var* / is left unencoded. Except for table options,
+	 *   its use should be avoided. In 1.24 and older, string encoding was applied.
 	 *
 	 * @param string $ins SQL statement to replace variables in
 	 * @return string The new SQL statement with variables replaced
 	 */
-	protected function replaceSchemaVars( $ins ) {
-		$vars = $this->getSchemaVars();
-		foreach ( $vars as $var => $value ) {
-			// replace '{$var}'
-			$ins = str_replace( '\'{$' . $var . '}\'', $this->addQuotes( $value ), $ins );
-			// replace `{$var}`
-			$ins = str_replace( '`{$' . $var . '}`', $this->addIdentifierQuotes( $value ), $ins );
-			// replace /*$var*/
-			$ins = str_replace( '/*$' . $var . '*/', $this->strencode( $value ), $ins );
-		}
-
-		return $ins;
-	}
-
-	/**
-	 * Replace variables in sourced SQL
-	 *
-	 * @param string $ins
-	 * @return string
-	 */
 	protected function replaceVars( $ins ) {
-		$ins = $this->replaceSchemaVars( $ins );
-
-		// Table prefixes
-		$ins = preg_replace_callback( '!/\*(?:\$wgDBprefix|_)\*/([a-zA-Z_0-9]*)!',
-			array( $this, 'tableNameCallback' ), $ins );
-
-		// Index names
-		$ins = preg_replace_callback( '!/\*i\*/([a-zA-Z_0-9]*)!',
-			array( $this, 'indexNameCallback' ), $ins );
-
-		return $ins;
+		$that = $this;
+		$vars = $this->getSchemaVars();
+		return preg_replace_callback(
+			'!
+				/\* (\$wgDBprefix|[_i]) \*/ (\w*) | # 1-2. tableName, indexName
+				\'\{\$ (\w+) }\'                  | # 3. addQuotes
+				`\{\$ (\w+) }`                    | # 4. addIdentifierQuotes
+				/\*\$ (\w+) \*/                     # 5. leave unencoded
+			!x',
+			function ( $m ) use ( $that, $vars ) {
+				// Note: Because of <https://bugs.php.net/bug.php?id=51881>,
+				// check for both nonexistent keys *and* the empty string.
+				if ( isset( $m[1] ) && $m[1] !== '' ) {
+					if ( $m[1] === 'i' ) {
+						return $that->indexName( $m[2] );
+					} else {
+						return $that->tableName( $m[2] );
+					}
+				} elseif ( isset( $m[3] ) && $m[3] !== '' && array_key_exists( $m[3], $vars ) ) {
+					return $that->addQuotes( $vars[$m[3]] );
+				} elseif ( isset( $m[4] ) && $m[4] !== '' && array_key_exists( $m[4], $vars ) ) {
+					return $that->addIdentifierQuotes( $vars[$m[4]] );
+				} elseif ( isset( $m[5] ) && $m[5] !== '' && array_key_exists( $m[5], $vars ) ) {
+					return $vars[$m[5]];
+				} else {
+					return $m[0];
+				}
+			},
+			$ins
+		);
 	}
 
 	/**
@@ -3976,26 +3978,6 @@ abstract class DatabaseBase implements IDatabase {
 		return array();
 	}
 
-	/**
-	 * Table name callback
-	 *
-	 * @param array $matches
-	 * @return string
-	 */
-	protected function tableNameCallback( $matches ) {
-		return $this->tableName( $matches[1] );
-	}
-
-	/**
-	 * Index name callback
-	 *
-	 * @param array $matches
-	 * @return string
-	 */
-	protected function indexNameCallback( $matches ) {
-		return $this->indexName( $matches[1] );
-	}
-
 	/**
 	 * Check to see if a named lock is available. This is non-blocking.
 	 *