Merge "Improve normalization and sanitization of memcached keys"

diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
index 95f5c8d..12ba83e 100644 (file)
--- a/includes/objectcache/MemcachedBagOStuff.php
+++ b/includes/objectcache/MemcachedBagOStuff.php
@@ -104,6 +104,46 @@ class MemcachedBagOStuff extends BagOStuff {
                return $this->client;
        }
 
+       /**
+        * Construct a cache key.
+        *
+        * @since 1.27
+        * @param string $keyspace
+        * @param array $args
+        * @return string
+        */
+       public function makeKeyInternal( $keyspace, $args ) {
+               // Memcached keys have a maximum length of 255 characters. From that,
+               // subtract the number of characters we need for the keyspace and for
+               // 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 ) );
+
+                               // 33 = 32 characters for the MD5 + 1 for the '#' prefix.
+                               if ( $charsLeft > 33 && strlen( $arg ) > $charsLeft ) {
+                                       $arg = '#' . md5( $arg );
+                               }
+
+                               $charsLeft -= strlen( $arg );
+                               return $arg;
+                       },
+                       $args
+               );
+
+               if ( $charsLeft < 0 ) {
+                       $args = array( '##' . md5( implode( ':', $args ) ) );
+               }
+
+               return parent::makeKeyInternal( $keyspace, $args );
+       }
+
        /**
         * Encode a key for use on the wire inside the memcached protocol.
         *
@@ -115,7 +155,7 @@ class MemcachedBagOStuff extends BagOStuff {
         * @return string
         */
        public function encodeKey( $key ) {
-               return preg_replace_callback( '/[\x00-\x20\x25\x7f]+/',
+               return preg_replace_callback( '/[^\x21-\x7e]+/',
                        array( $this, 'encodeKeyCallback' ), $key );
        }
 

diff --git a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
new file mode 100644 (file)
index 0000000..64478a2
--- /dev/null
+++ b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
@@ -0,0 +1,55 @@
+<?php
+/**
+ * @group BagOStuff
+ */
+class MemcachedBagOStuffTest extends MediaWikiTestCase {
+       /** @var MemcachedBagOStuff */
+       private $cache;
+
+       protected function setUp() {
+               parent::setUp();
+               $this->cache = new MemcachedBagOStuff( array( 'keyspace' => 'test' ) );
+       }
+
+       /**
+        * @covers MemcachedBagOStuff::makeKeyInternal
+        */
+       public function testKeyNormalization() {
+               $this->assertEquals(
+                       $this->cache->makeKey( 'vanilla' ),
+                       'test:vanilla'
+               );
+
+               $this->assertEquals(
+                       $this->cache->makeKey( 'punctuation_marks_are_ok', '!@$%^&*()' ),
+                       'test: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'
+               );
+
+               $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'
+               );
+
+               $this->assertEquals(
+                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠 𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' ),
+                       'test:this:key:contains:#60190c8f5a63ba5438b124f5c10b91d0'
+               );
+
+               $this->assertEquals(
+                       $this->cache->makeKey( '𝕖𝕧𝕖𝕟', '𝕚𝕗', '𝕨𝕖', '𝕄𝔻𝟝', '𝕖𝕒𝕔𝕙',
+                               '𝕒𝕣𝕘𝕦𝕞𝕖𝕟𝕥', '𝕥𝕙𝕚𝕤', '𝕜𝕖𝕪', '𝕨𝕠𝕦𝕝𝕕', '𝕤𝕥𝕚𝕝𝕝', '𝕓𝕖', '𝕥𝕠𝕠', '𝕝𝕠𝕟𝕘' ),
+                       'test:##5820ad1d105aa4dc698585c39df73e19'
+               );
+
+               $this->assertEquals(
+                       $this->cache->makeKey( '##5820ad1d105aa4dc698585c39df73e19' ),
+                       'test:%23%235820ad1d105aa4dc698585c39df73e19'
+               );
+       }
+}