jenkins-bot has submitted this change and it was merged.
Change subject: Escape colons in BagOStuff key segments
......................................................................
Escape colons in BagOStuff key segments
For the sake of safety and correctness, the following BagOStuff::makeKey()
invocations should return distinct keys:
$cache->makeKey( 'ab:', 'cd' );
$cache->makeKey( 'ab', ':cd' );
That is not currently the case, because while we use ':' as a key path
separator, we don't escape ':' in the input supplied to makeKey(). So, make
BagOStuff::makeKeyInternal() URL-encode colons.
To prevent this from messing up the max. key length calculations, reproduce
this logic in MemcachedBagOStuff::makeKeyInternal(), in lieu of having the
method call its parent.
Change-Id: I83ea7e7336a1c9e64aa42284c2517089a736efe5
---
M includes/libs/objectcache/BagOStuff.php
M includes/objectcache/MemcachedBagOStuff.php
M tests/phpunit/includes/objectcache/BagOStuffTest.php
M tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
4 files changed, 17 insertions(+), 7 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/libs/objectcache/BagOStuff.php
b/includes/libs/objectcache/BagOStuff.php
index c3c357f..ecc5e37 100644
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -627,7 +627,11 @@
* @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, ' ', '_' );
}
diff --git a/includes/objectcache/MemcachedBagOStuff.php
b/includes/objectcache/MemcachedBagOStuff.php
index 0489531..8bf6e3a 100644
--- a/includes/objectcache/MemcachedBagOStuff.php
+++ b/includes/objectcache/MemcachedBagOStuff.php
@@ -118,9 +118,10 @@
// the separator character needed for each argument.
$charsLeft = 255 - strlen( $keyspace ) - count( $args );
- $that = $this;
$args = array_map(
- function ( $arg ) use ( $that, &$charsLeft ) {
+ function ( $arg ) use ( &$charsLeft ) {
+ $arg = strtr( $arg, ' ', '_' );
+
// Make sure %, #, and non-ASCII chars are
escaped
$arg = preg_replace_callback(
'/[^\x21-\x22\x24\x26-\x7e]+/',
@@ -142,10 +143,10 @@
);
if ( $charsLeft < 0 ) {
- $args = array( '##' . md5( implode( ':', $args ) ) );
+ return $keyspace . ':##' . md5( implode( ':', $args ) );
}
- return parent::makeKeyInternal( $keyspace, $args );
+ return $keyspace . ':' . implode( ':', $args );
}
/**
diff --git a/tests/phpunit/includes/objectcache/BagOStuffTest.php
b/tests/phpunit/includes/objectcache/BagOStuffTest.php
index 466b9f5..94b74cb 100644
--- a/tests/phpunit/includes/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/objectcache/BagOStuffTest.php
@@ -50,6 +50,11 @@
$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' ) )
+ );
}
/**
diff --git a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
index 938f3b9..a946c3c 100644
--- a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
+++ b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php
@@ -32,7 +32,7 @@
$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->assertEquals(
@@ -43,7 +43,7 @@
$this->assertEquals(
$this->cache->makeKey( 'this', 'key', 'contains', '𝕥𝕠𝕠
𝕞𝕒𝕟𝕪 𝕞𝕦𝕝𝕥𝕚𝕓𝕪𝕥𝕖 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤' ),
-
'test:this:key:contains:#60190c8f5a63ba5438b124f5c10b91d0'
+
'test:this:key:contains:#c118f92685a635cb843039de50014c9c'
);
$this->assertEquals(
--
To view, visit https://gerrit.wikimedia.org/r/248539
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I83ea7e7336a1c9e64aa42284c2517089a736efe5
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
