From 0877963f9543ae29c6a06ab2d8b085b81b2ca467 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 23 Oct 2015 21:03:11 -0700
Subject: [PATCH] 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
---
 includes/objectcache/MemcachedBagOStuff.php   |  2 +-
 .../objectcache/MemcachedBagOStuffTest.php    | 46 +++++++++++--------
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
index 8bf6e3a1ff..e9df6a37d4 100644
--- 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 a946c3cdf9..b0c9e4f389 100644
--- 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 ) )
 		);
 	}
 }
-- 
2.20.1