Page MenuHomePhabricator

Double ?uselang= passed to a file results in "Cache key contains characters that are not allowed: `P180_1101514477_fr?uselang=fr_label`"
Closed, ResolvedPublic

Description

Message
class@anonymous
exception
Cache key contains characters that are not allowed: `P180_1101514477_fr?uselang=fr_label`

Cache key contains characters that are not allowed: `P31_1111607050_⧼lang⧽_label`
from /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php:271
exception.trace
#0 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php(266): Wikibase\Lib\SimpleCacheWithBagOStuff->invalidArgument(string)
#1 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php(233): Wikibase\Lib\SimpleCacheWithBagOStuff->assertKeyIsValid(string)
#2 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/Store/UncachedTermsPrefetcher.php(76): Wikibase\Lib\SimpleCacheWithBagOStuff->has(string)
#3 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/Store/UncachedTermsPrefetcher.php(41): Wikibase\Lib\Store\UncachedTermsPrefetcher->getUncachedLanguagesForEntityTerms(Wikibase\Lib\SimpleCacheWithBagOStuff, Wikibase\DataModel\Entity\PropertyId, array, array)
#4 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/Store/CachingPrefetchingTermLookup.php(42): Wikibase\Lib\Store\UncachedTermsPrefetcher->prefetchUncached(Wikibase\Lib\SimpleCacheWithBagOStuff, array, array, array)
#5 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/data-access/src/ByTypeDispatchingPrefetchingTermLookup.php(52): Wikibase\Lib\Store\CachingPrefetchingTermLookup->prefetchTerms(array, array, array)
#6 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/data-access/src/ByTypeDispatchingPrefetchingTermLookup.php(52): Wikibase\DataAccess\ByTypeDispatchingPrefetchingTermLookup->prefetchTerms(array, array, array)
#7 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/data-access/src/ByTypeDispatchingPrefetchingTermLookup.php(88): Wikibase\DataAccess\ByTypeDispatchingPrefetchingTermLookup->prefetchTerms(array, array, array)
#8 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/Store/EntityTermLookupBase.php(52): Wikibase\DataAccess\ByTypeDispatchingPrefetchingTermLookup->getTermsOfType(Wikibase\DataModel\Entity\PropertyId, string, array)
#9 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/lib/includes/Store/LanguageFallbackLabelDescriptionLookup.php(51): Wikibase\Lib\Store\EntityTermLookupBase->getLabels(Wikibase\DataModel\Entity\PropertyId, array)
#10 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/client/includes/Usage/UsageTrackingLanguageFallbackLabelDescriptionLookup.php(72): Wikibase\Lib\Store\LanguageFallbackLabelDescriptionLookup->getLabel(Wikibase\DataModel\Entity\PropertyId)
#11 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/client/includes/DataAccess/Scribunto/WikibaseLanguageDependentLuaBindings.php(60): Wikibase\Client\Usage\UsageTrackingLanguageFallbackLabelDescriptionLookup->getLabel(Wikibase\DataModel\Entity\PropertyId)
#12 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Wikibase/client/includes/DataAccess/Scribunto/Scribunto_LuaWikibaseLibrary.php(595): Wikibase\Client\DataAccess\Scribunto\WikibaseLanguageDependentLuaBindings->getLabel(string)
#13 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxCallback.php(26): Wikibase\Client\DataAccess\Scribunto\Scribunto_LuaWikibaseLibrary->getLabel(string)
#14 [internal function]: Scribunto_LuaSandboxCallback->__call(string, array)
#15 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxInterpreter.php(113): LuaSandboxFunction->call(LuaSandboxFunction)
#16 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Scribunto/includes/engines/LuaCommon/LuaEngine.php(291): Scribunto_LuaSandboxInterpreter->callFunction(LuaSandboxFunction, LuaSandboxFunction)
#17 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Scribunto/includes/engines/LuaCommon/LuaModule.php(69): Scribunto_LuaEngine->executeFunctionChunk(LuaSandboxFunction, PPTemplateFrame_Hash)
#18 /srv/mediawiki/php-1.35.0-wmf.19/extensions/Scribunto/includes/common/Hooks.php(128): Scribunto_LuaModule->invoke(string, PPTemplateFrame_Hash)
#19 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3599): ScribuntoHooks::invokeHook(Parser, PPTemplateFrame_Hash, array)
#20 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3303): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#21 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#22 /srv/mediawiki/php-1.35.0-wmf.19/extensions/ParserFunctions/includes/ParserFunctions.php(123): PPFrame_Hash->expand(PPNode_Hash_Tree)
#23 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3599): MediaWiki\Extensions\ParserFunctions\ParserFunctions::if(Parser, PPTemplateFrame_Hash, array)
#24 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3303): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#25 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#26 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3481): PPFrame_Hash->expand(PPNode_Hash_Tree)
#27 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#28 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/PPTemplateFrame_Hash.php(89): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#29 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3478): PPTemplateFrame_Hash->cachedExpand(string, PPNode_Hash_Tree)
#30 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPFrame_Hash)
#31 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(3144): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#32 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(1482): Parser->replaceVariables(string)
#33 /srv/mediawiki/php-1.35.0-wmf.19/includes/parser/Parser.php(598): Parser->internalParse(string)
#34 /srv/mediawiki/php-1.35.0-wmf.19/includes/content/WikitextContent.php(368): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#35 /srv/mediawiki/php-1.35.0-wmf.19/includes/content/AbstractContent.php(565): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#36 /srv/mediawiki/php-1.35.0-wmf.19/includes/Revision/RenderedRevision.php(267): AbstractContent->getParserOutput(Title, integer, ParserOptions, boolean)
#37 /srv/mediawiki/php-1.35.0-wmf.19/includes/Revision/RenderedRevision.php(236): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#38 /srv/mediawiki/php-1.35.0-wmf.19/includes/Revision/RevisionRenderer.php(215): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string)
#39 /srv/mediawiki/php-1.35.0-wmf.19/includes/Revision/RevisionRenderer.php(152): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#40 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#41 /srv/mediawiki/php-1.35.0-wmf.19/includes/Revision/RenderedRevision.php(198): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#42 /srv/mediawiki/php-1.35.0-wmf.19/includes/poolcounter/PoolWorkArticleView.php(196): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#43 /srv/mediawiki/php-1.35.0-wmf.19/includes/poolcounter/PoolCounterWork.php(125): PoolWorkArticleView->doWork()
#44 /srv/mediawiki/php-1.35.0-wmf.19/includes/page/Article.php(803): PoolCounterWork->execute()
#45 /srv/mediawiki/php-1.35.0-wmf.19/includes/page/CategoryPage.php(66): Article->view()
#46 /srv/mediawiki/php-1.35.0-wmf.19/includes/actions/ViewAction.php(63): CategoryPage->view()
#47 /srv/mediawiki/php-1.35.0-wmf.19/includes/MediaWiki.php(518): ViewAction->show()
#48 /srv/mediawiki/php-1.35.0-wmf.19/includes/MediaWiki.php(304): MediaWiki->performAction(CategoryTreeCategoryPage, Title)
#49 /srv/mediawiki/php-1.35.0-wmf.19/includes/MediaWiki.php(971): MediaWiki->performRequest()
#50 /srv/mediawiki/php-1.35.0-wmf.19/includes/MediaWiki.php(534): MediaWiki->main()
#51 /srv/mediawiki/php-1.35.0-wmf.19/index.php(47): MediaWiki->run()
#52 /srv/mediawiki/w/index.php(3): require(string)
#53 {main}
Impact

About 50 per minute. 400 today.

Started 2012-02-12 with wmf.19 rolling out.

Notes

This error is somehow corrupting out error channels (Monolog/Logstash, something?) because the message field is entirely overwritten by the meaningless string class@anonymous which made this rather hard to find. Once fixed, that should also be investigated.

Details

Request ID
XkV86QpAICsAACWu934AAAAB
Request URL
https://commons.wikimedia.org/wiki/Category:Kingdom_of_Romania?uselang=%E2%A7%BCLang%E2%A7%BD
Related Gerrit Patches:
mediawiki/extensions/Wikibase : masterDo not try to load label in unknown languages in Lua
mediawiki/extensions/Wikibase : masterPrevent invalid term languages from cached PrefetchingTermLookup
mediawiki/extensions/Wikibase : wmf/1.35.0-wmf.19Prevent invalid term languages from cached PrefetchingTermLookup
mediawiki/extensions/Wikibase : masterDNM Possible band aid for T245062
mediawiki/extensions/Wikibase : masterDNM Test reproducing the issue reported in T245062

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptWed, Feb 12, 9:37 PM
Jdforrester-WMF triaged this task as High priority.Wed, Feb 12, 9:39 PM

Not sure if this stripping is meant to happen at the Wikibase or WikibaseMediaInfo, so tagging both.

Krinkle removed a project: MediaWiki-Cache.
Krinkle added subscribers: WMDE-leszek, Addshore, Krinkle.

Looks similar to T244562. \cc @Addshore @WMDE-leszek

Beta Cluster
/wiki/Special:RecentChanges   class@anonymous /srv/mediawiki/php-master/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php
0x7f2867cdc02: Cache key contains characters that are not allowed: `d:P231054_1051159_en_label`
Krinkle set Request URL to https://commons.wikimedia.org/wiki/Category:Kingdom_of_Romania?uselang=%E2%A7%BCLang%E2%A7%BD.Thu, Feb 13, 4:50 PM
Krinkle changed Request ID from AXA7UBxCh3Uj6x1z44vf to XkV86QpAICsAACWu934AAAAB.
Krinkle updated the task description. (Show Details)
Krinkle edited Stack Trace. (Show Details)
Krinkle updated the task description. (Show Details)

There is another variation of this error:

url: /w/api.php?origin=*&action=wbsearchentities&format=json&limit=50&continue=0&language=en&uselang=en&search=…%3AP17&type=property

InvalidArgumentException: Invalid code "internal_api_error_class@anonymous
 from line 121 of /srv/mediawiki/php-1.35.0-wmf.19/includes/api/ApiMessageTrait.php

#0 /srv/mediawiki/php-1.35.0-wmf.19/includes/api/ApiMessage.php(87): ApiMessage->setApiCode(string, array)
#1 /srv/mediawiki/php-1.35.0-wmf.19/includes/api/ApiMessage.php(62): ApiMessage->__construct(array, string, array)
#2 /srv/mediawiki/php-1.35.0-wmf.19/includes/api/ApiMain.php(1036): ApiMessage::create(array, string, array)
#3 /srv/mediawiki/php-1.35.0-wmf.19/includes/api/ApiMain.php(1072): ApiMain->errorMessagesFromException(class@anonymous

Note that the stack trace is cut off after @anonymous due to a similar corruption it seems?

Krinkle raised the priority of this task from High to Unbreak Now!.Thu, Feb 13, 4:53 PM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptThu, Feb 13, 4:53 PM

Looks like the provided "language code" is not validated/sanitized?

Ladsgroup added subscribers: Jakob_WMDE, hoo, Ladsgroup.

I think this is caused by the caching work. Please take a look, I'm traveling atm

I'm not sure this should block the train; it'd be better if they emitted 400 errors rather than 500s, but it's definitely undefined behaviour territory.

Change 572045 had a related patch set uploaded (by WMDE-leszek; owner: WMDE-leszek):
[mediawiki/extensions/Wikibase@master] DNM Test reproducing the issue reported in T245062

https://gerrit.wikimedia.org/r/572045

I am note sure if the issue has been introduced in some recent work as @Ladsgroup said.
Looking at the stacktrace, and what happens in WikibaseLanguageDependentLuaBindings I see that the the instance of UsageTrackingLanguageFallbackLabelDescriptionLookup can easily get the garbage "language code" injected in WikibaseClient::getDataAccessLanguageFallbackChain

I've put https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/572045 to visualize this.

SimpleCacheWithBagOStuff rightfully makes sure that invalid characters are not written to the cache as a key, but the user input (lang code) should be validated and handled properly on some way higher level, which seems to not be happening.

Change 572054 had a related patch set uploaded (by WMDE-leszek; owner: WMDE-leszek):
[mediawiki/extensions/Wikibase@master] DNM Possible band aid for T245062

https://gerrit.wikimedia.org/r/572054

@WMDE-leszek If I understand correctly, the issue is that an unsafe value is passed to BagOStuff->get(). Specifically, a value that has not gone through makeKey() on the same BagOStuff. A BagOStuff implementation may have any number of restrictions on its cache key format (such as forbidden characters, or maximum length). All of which makeKey() is responsible for normalising away.

I don't think it's valid to assume that all supported language codes and variants are within a length and charset that all possible bagostuffs support. Although it seems likely that all such input be less than 20 chars and contain only ASCII, I don't think that's enforced. Especially for "uselang hack" cases such as Special:Upload?uselang=de-somethingüberspecial, which are currently supported by MediaWiki as long as they pass Language::isValidCode in RequetContext::getLanguage, which is quite tolerant.

Tarrow claimed this task.Fri, Feb 14, 9:43 AM
Tarrow added a subscriber: Tarrow.

We (the wikidata campsite) will pick this up and have a look this morning.

Change 572228 had a related patch set uploaded (by Tarrow; owner: Tarrow):
[mediawiki/extensions/Wikibase@master] Filter to only prefect valid terms

https://gerrit.wikimedia.org/r/572228

Tarrow added a subscriber: Rosalie_WMDE.EditedFri, Feb 14, 2:17 PM

So we (@Jakob_WMDE and @Rosalie_WMDE and me) looked at this in some detail. We came to the following conclusions (chip in if I misrepresented anything)

The ideal solution is for us to fix \Wikibase\Lib\SimpleCacheWithBagOStuff to reduce the number of times it fails by including some attempt to use makeKey. We were not keen to touch this on a Friday to fix the UBN though because:

  • it's currently untested due to T237164
  • it will change all our Wikibase caches (and potentially mean things like using new keys)

We evaluated the patches by Leszek and concluded that:

  • it only filtered out possible non-term languages from the fallback chain
    • this meant that bad languages could still be passed on to the EntityAccessor

We considered moving this test for a valid term language to the getLanguage method but the problem was still that:

  • this would mean that valid interface languages (but not valid terms languages) would be filtered out (e.g. users with interface language 'fil' would not fallback to 'tag' but straight to 'en'. I.e. we don't want to mix up user interface and term languages at this level.

We concluded that code merged that resulted in these errors starting now is probably due to the usage of \Wikibase\Lib\Store\TermCacheKeyBuilder::buildCacheKey. The two "new" places that use this are \Wikibase\Lib\Store\CachingPrefetchingTermLookup::getCacheKey and \Wikibase\Lib\Store\UncachedTermsPrefetcher::getCacheKey whereas \Wikibase\Lib\Store\CachingFallbackLabelDescriptionLookup::getTerm has used this for some time.

We are happy to see that these two are only concerned with terms so this seems an appropriate place to add that filtering.

To quickly follow is:

  • back-portable patch to get the train going ASAP
  • separate patch for testing CachingPrefetchingTermLookup and UncachedTermsPrefetcher

We will also make tickets/reevaluate priority for:

  • SimpleCacheWithBagOStuff being tested
  • Making SimpleCacheWithBagOStuff less likely to throw
  • investigating in greater detail how those bad languages got into wikibase scribunto (we struggled to see how in our rough and ready testing)

On the lua side of things: You can reproduce this problem by setting the language to almost anything other than a valid one like "uselang=???". The good news is that I couldn't break out of the string so it's not an injection vulnerability AFAIK but this can be fixed rather easily in the lua side. I will try to make a patch.

Change 572259 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Do not try to load label in unknown languages in Lua

https://gerrit.wikimedia.org/r/572259

Change 572264 had a related patch set uploaded (by Jakob; owner: Tarrow):
[mediawiki/extensions/Wikibase@wmf/1.35.0-wmf.19] Prevent invalid term languages from cached PrefetchingTermLookup

https://gerrit.wikimedia.org/r/572264

Change 572264 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@wmf/1.35.0-wmf.19] Prevent invalid term languages from cached PrefetchingTermLookup

https://gerrit.wikimedia.org/r/572264

Mentioned in SAL (#wikimedia-operations) [2020-02-14T17:33:54Z] <jforrester@deploy1001> Locking from deployment [ALL REPOSITORIES]: Testing T245062 fix on mwdebug1001 (planned duration: 60m 00s)

Mentioned in SAL (#wikimedia-operations) [2020-02-14T17:37:00Z] <jforrester@deploy1001> Unlocked for deployment [ALL REPOSITORIES]: Testing T245062 fix on mwdebug1001 (duration: 03m 05s)

OK, this works OK, but SRE vetoed deploying to production this late on a holiday Friday, so I'll sync it out early on Tuesday SF time.

Change 572228 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Prevent invalid term languages from cached PrefetchingTermLookup

https://gerrit.wikimedia.org/r/572228

Mentioned in SAL (#wikimedia-operations) [2020-02-14T19:09:59Z] <jforrester@deploy1001> Synchronized php-1.35.0-wmf.19/extensions/Wikibase: T245062 Prevent invalid term languages from cached PrefetchingTermLookup (duration: 01m 09s)

I deployed this just now, and the error has gone. Thanks, all. Filing a follow-up for the class@anonymous logging bug.

Jdforrester-WMF closed this task as Resolved.Fri, Feb 14, 7:29 PM