jenkins-bot has submitted this change and it was merged.

Change subject: Database: Remove use of strencode() in replaceVars()
......................................................................


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
---
M RELEASE-NOTES-1.25
M includes/db/Database.php
2 files changed, 49 insertions(+), 55 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 5d423a1..2b347b9 100644
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -254,6 +254,18 @@
   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 fc13eeb..1124617 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3907,47 +3907,49 @@
         *
         * - '{$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
+               );
        }
 
        /**
@@ -3974,26 +3976,6 @@
         */
        protected function getDefaultSchemaVars() {
                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] );
        }
 
        /**

-- 
To view, visit https://gerrit.wikimedia.org/r/146363
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia697ce5f14082e737b3e1c4e174bd3f6223acc5c
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: PleaseStand <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Parent5446 <[email protected]>
Gerrit-Reviewer: PleaseStand <[email protected]>
Gerrit-Reviewer: Umherirrender <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to