Fixes to MemcachedBagOStuff::makeKeyInternal()

* Follow-up to 0c9fb12265e2
* Make sure colons actually get escaped
* Added more unit tests
* Also fixed the test actual/expected order

Change-Id: Ie04ea6059ee1eb6d1da8f30fefdec52fa49d38fb

diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
index 8bf6e3a..e9df6a3 100644 (file)
--- a/includes/objectcache/MemcachedBagOStuff.php
+++ b/includes/objectcache/MemcachedBagOStuff.php
@@ -124,7 +124,7 @@ class MemcachedBagOStuff extends BagOStuff {
 
                                // Make sure %, #, and non-ASCII chars are escaped
                                $arg = preg_replace_callback(
-                                       '/[^\x21-\x22\x24\x26-\x7e]+/',
+                                       '/[^\x21-\x22\x24\x26-\x39\x3b-\x7e]+/',
                                        function ( $m ) {
                                                return rawurlencode( $m[0] );
                                        },

diff --git a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
index a946c3c..b0c9e4f 100644 (file)
--- a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
+++ b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
@@ -16,45 +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( 'percent_is_escaped', '!@$%^&*()' ),
-                       'test:percent_is_escaped:!@$%25^&*()'
+                       'test:but_spaces:hashes%23:and%0Anewlines:are_not',
+                       $this->cache->makeKey( 'but spaces', 'hashes#', "and\nnewlines", 'are_not' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'but spaces', 'hashes#', "and\nnewlines", 'are_not' ),
-                       'test:but_spaces:hashes%23:and%0Anewlines:are_not'
+                       '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->cache->makeKey( 'this', 'key', 'contains', '𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖', 'characters' )
                );
 
                $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'
+                       'test:this:key:contains:#c118f92685a635cb843039de50014c9c',
+                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠 𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠 𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' ),
-                       'test:this:key:contains:#c118f92685a635cb843039de50014c9c'
+                       'test:##5820ad1d105aa4dc698585c39df73e19',
+                       $this->cache->makeKey( '𝕖𝕧𝕖𝕟', '𝕚𝕗', '𝕨𝕖', '𝕄𝔻𝟝', '𝕖𝕒𝕔𝕙',
+                               '𝕒𝕣𝕘𝕦𝕞𝕖𝕟𝕥', '𝕥𝕙𝕚𝕤', '𝕜𝕖𝕪', '𝕨𝕠𝕦𝕝𝕕', '𝕤𝕥𝕚𝕝𝕝', '𝕓𝕖', '𝕥𝕠𝕠', '𝕝𝕠𝕟𝕘' )
                );
 
                $this->assertEquals(
-                       $this->cache->makeKey( '𝕖𝕧𝕖𝕟', '𝕚𝕗', '𝕨𝕖', '𝕄𝔻𝟝', '𝕖𝕒𝕔𝕙',
-                               '𝕒𝕣𝕘𝕦𝕞𝕖𝕟𝕥', '𝕥𝕙𝕚𝕤', '𝕜𝕖𝕪', '𝕨𝕠𝕦𝕝𝕕', '𝕤𝕥𝕚𝕝𝕝', '𝕓𝕖', '𝕥𝕠𝕠', '𝕝𝕠𝕟𝕘' ),
-                       'test:##5820ad1d105aa4dc698585c39df73e19'
+                       '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(
-                       $this->cache->makeKey( '##5820ad1d105aa4dc698585c39df73e19' ),
-                       'test:%23%235820ad1d105aa4dc698585c39df73e19'
+                       'test:long_key_part_hashed:#0244f7b1811d982dd932dd7de01465ac',
+                       $this->cache->makeKey( 'long_key_part_hashed', str_repeat( 'y', 500 ) )
                );
        }
 }