Page MenuHomePhabricator

Colon delimited key list passed to makeGlobalKey() seems to result in a silent failure
Open, Needs TriagePublic

Description

Forking from T271757: Fix issues with StopForumSpam's doUpdate function

Doing what is effectively $wanCache->makeGlobalKey( 'sfs:denylist:set' ) seems to result in things silently failing in terms of get/set with relation to the cache

$wanCache->makeGlobalKey( 'sfs-denylist-set' ) works fine

I don't know if this is some key mangling going on, or corruption... But it shouldn't just be failing silently IMHO

Event Timeline

$wgSFSDenyListKey = "sfs:denylist:set";

Looking at T271757, the value of $wgSFSDenyListKey was originally a fully-formatted cache key (like the output of calling wfMemcKey, BagOStuff::makeKey, or WANObjectCache::makeKey; where the pieces are already escaped as-needed and joined by a colon). This, as opposed to holding a key group or key component value that is passed to such a function. From that perspective, this is basically unsupported input. Or rather, it represents what would in the past have been a reasonable value to pass to a "get" or "set" function directly, as being an already formatted key string. (I say "reasonable", because at the time we probably didn't have a central concept for "global" keys yet, and the closest analog, wfForeignMemcKey, was specifically for global cache keys that have a "home" wiki of sorts to emulate. For cache keys that are wiki agnostic we didn't really have anything other than ad-hoc "cross fringers this won't conflict with a wiki id" strings such as the ones we see here).

For the past 10 years (since at least 2015 with MW 1.27), cache keys should all be formatted by makeKey or makeGlobalKey, where first part is a required key group (under 48 chars, and, if you want useful telemetry in Logstash and Grafana, it should not contain colons or non-ASCI characters), and any other components will be escaped/hashed as-needed in a deterministic way.

https://doc.wikimedia.org/mediawiki-core/master/php/classWikimedia_1_1ObjectCache_1_1BagOStuff.html#a500cabcc6f1d92010458eba12fbbe561

Having said that, the only loss when this is violated, should be the loss of some "ideal" stats and logging features. E.g. the keygroup may be altered or rewritten to escape any special chars, and thus may appear in Grafana under a slightly different alternate name, or (at worst) an unrecognisable md5 hash that, apart from being hard to recognise in debugging, would be a functional stand-in for your input.

Trying locally, with a basic HashBagOStuff, to test most of the wiring and encoding logic, the problem does not appear:

# $ php maintenance/eval.php 
$hash = new HashBagOStuff();
return $hash->makeGlobalKey('foo');
//> global:foo
return $hash->makeGlobalKey('foo:bar');
//> global:foo%3Abar

$k = $hash->makeGlobalKey('krinkle:T271784'); return $k;
//> global:krinkle%3AT271784
$hash->set($k, 'x', 60);
return $hash->get($k);
//> x

I also tried it on the Beta Cluster with Memcached (and its specific encoding logic in MemcachedBagOStuff) and WANObjectCache over top. I can't reproduce it ther either. I made sure to restart the process and to enable -d 1 so confirm that it is retreiving it and not coming from a process cache.

# krinkle@deployment-mediawiki13:/srv/mediawiki$ sudo -u www-data php multiversion/MWScript.php eval.php enwiki -d 1
$w = MW::srv()->getMainWANObjectCache();
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff::initializeClient: initializing new client instance.
# [debug] [memcached] MainWANObjectCache using store Wikimedia\ObjectCache\MemcachedPeclBagOStuff

$k = $w->makeGlobalKey('krinkle:T271784'); return $k;
//> global:krinkle%3AT271784
$w->set($k, 'y', 600);
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff debug: get(WANCache:global:krinkle%3AT271784|#|v)
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff debug: result: NOT FOUND
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff debug: add(WANCache:global:krinkle%3AT271784|#|v)
return $w->get($k);
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff debug: getMulti(WANCache:global:krinkle%3AT271784|#|v)
//> y
> ^D
# [debug] [rdbms] LBFactory shutdown completed

# krinkle@deployment-mediawiki13:/srv/mediawiki$ sudo -u www-data php multiversion/MWScript.php eval.php enwiki -d 1
$w = MW::srv()->getMainWANObjectCache();
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff::initializeClient: initializing new client instance.
# [debug] [memcached] MainWANObjectCache using store Wikimedia\ObjectCache\MemcachedPeclBagOStuff
$k = $w->makeGlobalKey('krinkle:T271784'); return $k;
//> global:krinkle%3AT271784
return $w->get($k);
# [debug] [memcached] Wikimedia\ObjectCache\MemcachedPeclBagOStuff debug: getMulti(WANCache:global:krinkle%3AT271784|#|v)
//> y