Merge "Minor fix to primeFileCache() comment"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 24 Oct 2015 07:43:17 +0000 (07:43 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 24 Oct 2015 07:43:17 +0000 (07:43 +0000)
includes/DefaultSettings.php
includes/installer/Installer.php
includes/installer/WebInstaller.php
includes/jobqueue/jobs/HTMLCacheUpdateJob.php
includes/libs/objectcache/BagOStuff.php
includes/objectcache/MemcachedBagOStuff.php
includes/objectcache/MemcachedPeclBagOStuff.php
tests/phpunit/includes/objectcache/BagOStuffTest.php
tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php

index 189ce42..04f3f31 100644 (file)
@@ -5202,80 +5202,85 @@ $wgApplyIpBlocksToXff = false;
  * @par Example:
  * To set a generic maximum of 4 hits in 60 seconds:
  * @code
- * $wgRateLimits = array( 4, 60 );
+ *     $wgRateLimits = array( 4, 60 );
  * @endcode
  *
- * You could also limit per action and then type of users. See the inline
- * code for a template to use.
- *
- * This option set is experimental and likely to change.
+ * @par Example:
+ * You could also limit per action and then type of users.
+ * @code
+ *     $wgRateLimits = array(
+ *         'edit' => array(
+ *             'anon' => array( x, y ), // any and all anonymous edits (aggregate)
+ *             'user' => array( x, y ), // each logged-in user
+ *             'newbie' => array( x, y ), // each new autoconfirmed accounts; overrides 'user'
+ *             'ip' => array( x, y ), // each anon and recent account
+ *             'subnet' => array( x, y ), // ... within a /24 subnet in IPv4 or /64 in IPv6
+ *         )
+ *     )
+ * @endcode
  *
- * @warning Requires memcached.
+ * @warning Requires that $wgMainCacheType is set to something persistent
  */
 $wgRateLimits = array(
+       // Page edits
        'edit' => array(
-               'anon' => null, // for any and all anonymous edits (aggregate)
-               'user' => null, // for each logged-in user
-               'newbie' => null, // for each recent (autoconfirmed) account; overrides 'user'
-               'ip' => null, // for each anon and recent account
-               'subnet' => null, // ... within a /24 subnet in IPv4 or /64 in IPv6
+               'ip' => array( 8, 60 ),
+               'newbie' => array( 8, 60 ),
+       ),
+       // Page moves
+       'move' => array(
+               'newbie' => array( 2, 120 ),
+               'user' => array( 8, 60 ),
        ),
+       // File uploads
        'upload' => array(
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+               'ip' => array( 8, 60 ),
+               'newbie' => array( 8, 60 ),
        ),
-       'move' => array(
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Page rollbacks
+       'rollback' => array(
+               'user' => array( 10, 60 ),
+               'newbie' => array( 5, 120 )
        ),
-       'mailpassword' => array( // triggering password resets emails
-               'anon' => null,
+       // Triggering password resets emails
+       'mailpassword' => array(
+               'ip' => array( 5, 3600 ),
        ),
-       'emailuser' => array( // emailing other users using MediaWiki
-               'user' => null,
+       // Emailing other users using MediaWiki
+       'emailuser' => array(
+               'ip' => array( 5, 86400 ),
+               'newbie' => array( 5, 86400 ),
+               'user' => array( 20, 86400 ),
        ),
-       'linkpurge' => array( // purges of link tables
-               'anon' => null,
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Purging pages
+       'purge' => array(
+               'ip' => array( 30, 60 ),
+               'user' => array( 30, 60 ),
        ),
-       'renderfile' => array( // files rendered via thumb.php or thumb_handler.php
-               'anon' => null,
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Purges of link tables
+       'linkpurge' => array(
+               'ip' => array( 30, 60 ),
+               'user' => array( 30, 60 ),
        ),
-       'renderfile-nonstandard' => array( // same as above but for non-standard thumbnails
-               'anon' => null,
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Files rendered via thumb.php or thumb_handler.php
+       'renderfile' => array(
+               'ip' => array( 700, 30 ),
+               'user' => array( 700, 30 ),
        ),
-       'stashedit' => array( // stashing edits into cache before save
-               'anon' => null,
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Same as above but for non-standard thumbnails
+       'renderfile-nonstandard' => array(
+               'ip' => array( 70, 30 ),
+               'user' => array( 70, 30 ),
        ),
-       'changetag' => array( // adding or removing change tags
-               'user' => null,
-               'newbie' => null,
+       // Stashing edits into cache before save
+       'stashedit' => array(
+               'ip' => array( 30, 60 ),
+               'newbie' => array( 30, 60 ),
        ),
-       'purge' => array( // purging pages
-               'anon' => null,
-               'user' => null,
-               'newbie' => null,
-               'ip' => null,
-               'subnet' => null,
+       // Adding or removing change tags
+       'changetag' => array(
+               'ip' => array( 8, 60 ),
+               'newbie' => array( 8, 60 ),
        ),
 );
 
index 064bd6d..91589e1 100644 (file)
@@ -691,17 +691,6 @@ abstract class Installer {
                return Status::newGood();
        }
 
-       /**
-        * Exports all wg* variables stored by the installer into global scope.
-        */
-       public function exportVars() {
-               foreach ( $this->settings as $name => $value ) {
-                       if ( substr( $name, 0, 2 ) == 'wg' ) {
-                               $GLOBALS[$name] = $value;
-                       }
-               }
-       }
-
        /**
         * Environment check for DB types.
         * @return bool
index 67a4def..9edc25a 100644 (file)
@@ -159,7 +159,6 @@ class WebInstaller extends Installer {
                        $this->settings = $session['settings'] + $this->settings;
                }
 
-               $this->exportVars();
                $this->setupLanguage();
 
                if ( ( $this->getVar( '_InstallDone' ) || $this->getVar( '_UpgradeDone' ) )
index ef9fec0..6b1a1e3 100644 (file)
@@ -103,6 +103,7 @@ class HTMLCacheUpdateJob extends Job {
                // Check $wgUpdateRowsPerQuery for sanity; batch jobs are sized by that already.
                foreach ( array_chunk( $pageIds, $wgUpdateRowsPerQuery ) as $batch ) {
                        $dbw->commit( __METHOD__, 'flush' );
+                       wfWaitForSlaves();
 
                        $dbw->update( 'page',
                                array( 'page_touched' => $dbw->timestamp( $touchTimestamp ) ),
index c3c357f..ecc5e37 100644 (file)
@@ -627,7 +627,11 @@ abstract class BagOStuff implements LoggerAwareInterface {
         * @return string
         */
        public function makeKeyInternal( $keyspace, $args ) {
-               $key = $keyspace . ':' . implode( ':', $args );
+               $key = $keyspace;
+               foreach ( $args as $arg ) {
+                       $arg = str_replace( ':', '%3A', $arg );
+                       $key = $key . ':' . $arg;
+               }
                return strtr( $key, ' ', '_' );
        }
 
index 12ba83e..e9df6a3 100644 (file)
@@ -65,25 +65,25 @@ class MemcachedBagOStuff extends BagOStuff {
        }
 
        protected function getWithToken( $key, &$casToken, $flags = 0 ) {
-               return $this->client->get( $this->encodeKey( $key ), $casToken );
+               return $this->client->get( $this->validateKeyEncoding( $key ), $casToken );
        }
 
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
-               return $this->client->set( $this->encodeKey( $key ), $value,
+               return $this->client->set( $this->validateKeyEncoding( $key ), $value,
                        $this->fixExpiry( $exptime ) );
        }
 
        protected function cas( $casToken, $key, $value, $exptime = 0 ) {
-               return $this->client->cas( $casToken, $this->encodeKey( $key ),
+               return $this->client->cas( $casToken, $this->validateKeyEncoding( $key ),
                        $value, $this->fixExpiry( $exptime ) );
        }
 
        public function delete( $key ) {
-               return $this->client->delete( $this->encodeKey( $key ) );
+               return $this->client->delete( $this->validateKeyEncoding( $key ) );
        }
 
        public function add( $key, $value, $exptime = 0 ) {
-               return $this->client->add( $this->encodeKey( $key ), $value,
+               return $this->client->add( $this->validateKeyEncoding( $key ), $value,
                        $this->fixExpiry( $exptime ) );
        }
 
@@ -118,13 +118,18 @@ class MemcachedBagOStuff extends BagOStuff {
                // the separator character needed for each argument.
                $charsLeft = 255 - strlen( $keyspace ) - count( $args );
 
-               $that = $this;
                $args = array_map(
-                       function ( $arg ) use ( $that, &$charsLeft ) {
-                               // Because MemcachedBagOStuff::encodeKey() will be called again
-                               // with this input once the key is actually used, we have to
-                               // encode pound signs here rather than in encodeKey().
-                               $arg = $that->encodeKey( str_replace( '#', '%23', $arg ) );
+                       function ( $arg ) use ( &$charsLeft ) {
+                               $arg = strtr( $arg, ' ', '_' );
+
+                               // Make sure %, #, and non-ASCII chars are escaped
+                               $arg = preg_replace_callback(
+                                       '/[^\x21-\x22\x24\x26-\x39\x3b-\x7e]+/',
+                                       function ( $m ) {
+                                               return rawurlencode( $m[0] );
+                                       },
+                                       $arg
+                               );
 
                                // 33 = 32 characters for the MD5 + 1 for the '#' prefix.
                                if ( $charsLeft > 33 && strlen( $arg ) > $charsLeft ) {
@@ -138,33 +143,25 @@ class MemcachedBagOStuff extends BagOStuff {
                );
 
                if ( $charsLeft < 0 ) {
-                       $args = array( '##' . md5( implode( ':', $args ) ) );
+                       return $keyspace . ':##' . md5( implode( ':', $args ) );
                }
 
-               return parent::makeKeyInternal( $keyspace, $args );
+               return $keyspace . ':' . implode( ':', $args );
        }
 
        /**
-        * Encode a key for use on the wire inside the memcached protocol.
+        * Ensure that a key is safe to use (contains no control characters and no
+        * characters above the ASCII range.)
         *
-        * We encode spaces and line breaks to avoid protocol errors. We encode
-        * the other control characters for compatibility with libmemcached
-        * verify_key. We leave other punctuation alone, to maximise backwards
-        * compatibility.
         * @param string $key
         * @return string
+        * @throws Exception
         */
-       public function encodeKey( $key ) {
-               return preg_replace_callback( '/[^\x21-\x7e]+/',
-                       array( $this, 'encodeKeyCallback' ), $key );
-       }
-
-       /**
-        * @param array $m
-        * @return string
-        */
-       protected function encodeKeyCallback( $m ) {
-               return rawurlencode( $m[0] );
+       public function validateKeyEncoding( $key ) {
+               if ( preg_match( '/[^\x21-\x7e]+/', $key ) ) {
+                       throw new Exception( "Key contains invalid characters: $key" );
+               }
+               return $key;
        }
 
        /**
@@ -183,21 +180,6 @@ class MemcachedBagOStuff extends BagOStuff {
                return (int)$expiry;
        }
 
-       /**
-        * Decode a key encoded with encodeKey(). This is provided as a convenience
-        * function for debugging.
-        *
-        * @param string $key
-        *
-        * @return string
-        */
-       public function decodeKey( $key ) {
-               // matches %00-%20, %25, %7F (=decoded alternatives for those encoded in encodeKey)
-               return preg_replace_callback( '/%([0-1][0-9]|20|25|7F)/i', function ( $match ) {
-                       return urldecode( $match[0] );
-               }, $key );
-       }
-
        /**
         * Send a debug message to the log
         * @param string $text
index 365236d..e6df900 100644 (file)
@@ -117,7 +117,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
 
        protected function getWithToken( $key, &$casToken, $flags = 0 ) {
                $this->debugLog( "get($key)" );
-               $result = $this->client->get( $this->encodeKey( $key ), null, $casToken );
+               $result = $this->client->get( $this->validateKeyEncoding( $key ), null, $casToken );
                $result = $this->checkResult( $key, $result );
                return $result;
        }
@@ -202,14 +202,10 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
 
        public function getMulti( array $keys, $flags = 0 ) {
                $this->debugLog( 'getMulti(' . implode( ', ', $keys ) . ')' );
-               $callback = array( $this, 'encodeKey' );
-               $encodedResult = $this->client->getMulti( array_map( $callback, $keys ) );
-               $encodedResult = $encodedResult ?: array(); // must be an array
-               $result = array();
-               foreach ( $encodedResult as $key => $value ) {
-                       $key = $this->decodeKey( $key );
-                       $result[$key] = $value;
+               foreach ( $keys as $key ) {
+                       $this->validateKeyEncoding( $key );
                }
+               $result = $this->client->getMulti( $keys ) ?: array();
                return $this->checkResult( false, $result );
        }
 
@@ -219,14 +215,10 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
         * @return bool
         */
        public function setMulti( array $data, $exptime = 0 ) {
-               foreach ( $data as $key => $value ) {
-                       $encKey = $this->encodeKey( $key );
-                       if ( $encKey !== $key ) {
-                               $data[$encKey] = $value;
-                               unset( $data[$key] );
-                       }
-               }
                $this->debugLog( 'setMulti(' . implode( ', ', array_keys( $data ) ) . ')' );
+               foreach ( array_keys( $data ) as $key ) {
+                       $this->validateKeyEncoding( $key );
+               }
                $result = $this->client->setMulti( $data, $this->fixExpiry( $exptime ) );
                return $this->checkResult( false, $result );
        }
index 466b9f5..94b74cb 100644 (file)
@@ -50,6 +50,11 @@ class BagOStuffTest extends MediaWikiTestCase {
                        $globalKey,
                        'Local key and global key with same parameters should not be equal'
                );
+
+               $this->assertNotEquals(
+                       $cache->makeKeyInternal( 'prefix', array( 'a', 'bc:', 'de' ) ),
+                       $cache->makeKeyInternal( 'prefix', array( 'a', 'bc', ':de' ) )
+               );
        }
 
        /**
index 64478a2..b0c9e4f 100644 (file)
@@ -16,40 +16,55 @@ class MemcachedBagOStuffTest extends MediaWikiTestCase {
         */
        public function testKeyNormalization() {
                $this->assertEquals(
-                       $this->cache->makeKey( 'vanilla' ),
-                       'test:vanilla'
+                       'test:vanilla',
+                       $this->cache->makeKey( 'vanilla' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'punctuation_marks_are_ok', '!@$%^&*()' ),
-                       'test:punctuation_marks_are_ok:!@$%^&*()'
+                       'test:punctuation_marks_are_ok:!@$^&*()',
+                       $this->cache->makeKey( 'punctuation_marks_are_ok', '!@$^&*()' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'but spaces', 'hashes#', "and\nnewlines", 'are_not' ),
-                       'test:but%20spaces:hashes%23:and%0Anewlines:are_not'
+                       'test:but_spaces:hashes%23:and%0Anewlines:are_not',
+                       $this->cache->makeKey( 'but spaces', 'hashes#', "and\nnewlines", 'are_not' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖', 'characters' ),
                        'test:this:key:contains:%F0%9D%95%9E%F0%9D%95%A6%F0%9D%95%9D%F0%9D%95%A5%F0%9' .
-                               'D%95%9A%F0%9D%95%93%F0%9D%95%AA%F0%9D%95%A5%F0%9D%95%96:characters'
+                               'D%95%9A%F0%9D%95%93%F0%9D%95%AA%F0%9D%95%A5%F0%9D%95%96:characters',
+                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖', 'characters' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠 𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' ),
-                       'test:this:key:contains:#60190c8f5a63ba5438b124f5c10b91d0'
+                       'test:this:key:contains:#c118f92685a635cb843039de50014c9c',
+                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠 𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' )
                );
 
                $this->assertEquals(
+                       'test:##5820ad1d105aa4dc698585c39df73e19',
                        $this->cache->makeKey( '𝕖𝕧𝕖𝕟', '𝕚𝕗', '𝕨𝕖', '𝕄𝔻𝟝', '𝕖𝕒𝕔𝕙',
-                               '𝕒𝕣𝕘𝕦𝕞𝕖𝕟𝕥', '𝕥𝕙𝕚𝕤', '𝕜𝕖𝕪', '𝕨𝕠𝕦𝕝𝕕', '𝕤𝕥𝕚𝕝𝕝', '𝕓𝕖', '𝕥𝕠𝕠', '𝕝𝕠𝕟𝕘' ),
-                       'test:##5820ad1d105aa4dc698585c39df73e19'
+                               '𝕒𝕣𝕘𝕦𝕞𝕖𝕟𝕥', '𝕥𝕙𝕚𝕤', '𝕜𝕖𝕪', '𝕨𝕠𝕦𝕝𝕕', '𝕤𝕥𝕚𝕝𝕝', '𝕓𝕖', '𝕥𝕠𝕠', '𝕝𝕠𝕟𝕘' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( '##5820ad1d105aa4dc698585c39df73e19' ),
-                       'test:%23%235820ad1d105aa4dc698585c39df73e19'
+                       'test:%23%235820ad1d105aa4dc698585c39df73e19',
+                       $this->cache->makeKey( '##5820ad1d105aa4dc698585c39df73e19' )
+               );
+
+               $this->assertEquals(
+                       'test:percent_is_escaped:!@$%25^&*()',
+                       $this->cache->makeKey( 'percent_is_escaped', '!@$%^&*()' )
+               );
+
+               $this->assertEquals(
+                       'test:colon_is_escaped:!@$%3A^&*()',
+                       $this->cache->makeKey( 'colon_is_escaped', '!@$:^&*()' )
+               );
+
+               $this->assertEquals(
+                       'test:long_key_part_hashed:#0244f7b1811d982dd932dd7de01465ac',
+                       $this->cache->makeKey( 'long_key_part_hashed', str_repeat( 'y', 500 ) )
                );
        }
 }