Remove fallback to $wgUser in LocalRepo::findFiles(), FileRepo::findFiles(), and FileRepo::findFileFromKey().
These were added in 5e703cdf669f85e2924dee76c5a8831633259ce0 as a hacky backport patch to fix T263014 and properly deprecate a missing hard-deprecation in File::canUser. But it should be fixed properly in 1.36.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T11968 Reduce use of global and public variables | |||
| Open | None | T159283 Deprecate non-configuration globals | |||
| Open | DannyS712 | T159299 Deprecate and remove $wgUser | |||
| Open | None | T267861 [GOAL] Emit deprecation warnings reading from $wgUser | |||
| Open | DannyS712 | T243708 Mediawiki core needs uses of global $wgUser removed | |||
| Resolved | DannyS712 | T245331 Remove core fallbacks to global $wgUser [1.36] | |||
| Resolved | DannyS712 | T263033 Remove fallback to `$wgUser` in LocalRepo::findFiles(), FileRepo::findFiles(), and FileRepo::findFileFromKey() |
Note: callers refers to deployed callers only. Didn't check everywhere
Fallbacks exist in:
FileRepo::findFile if $options['private'] is not empty and not a user object
FileRepo::findFileFromKey if $options['private'] is not empty and not a user object
LocalRepo::findFiles if $items is not an array with $items['user'] being a user object
Tackling findFileFromKey first, since that seems easiest.
Callers:
FlaggedRevs - $options['private'] is not set, fallback not used
Core:
RepoGroup::findFileFromKey - $options is whatever the method was called with; method has the same name, so callers of it are included in this list already
Parser::fetchFileNoRegister - $options is whatever the method was called with.
Callers of Parser::fetchFileNoRegister:
Protected method, only callers are in the parser class:
Parser::fetchFileAndTitle - $options is whatever the method was called with
Parser::renderImageGallery - $options is an empty array that can be modified by extensions using the BeforeParserFetchFileAndTitle hook. The only deployed extension to use the hook, FlaggedRevs, does not set the private key. We should document that if it is set it must be a user object, but there are no other changes needed.
Callers of Parser::fetchFileAndTitle
WikidataPageBanner: no $options are set, fallback not used
Core:
TraditionalImageGallery::toHTML - $options same as Parser::renderImageGallery - empty and then modified by BeforeParserFetchFileAndTitle hook.
Parser::handleInternalLinks2 - same as above, BeforeParserFetchFileAndTitle
Parser::makeImage - same as above, BeforeParserFetchFileAndTitle
Solution:
no deployed code is using this fallback. To remove it:
Document that if the hook sets the private option, it must be set to a user object
Through an exception in findFileFromKey if its set and not a user
Change 628563 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Remove fallback to $wgUser in FileRepo::findFileFromKey
Almost all callers pass an array of titles, rather than an array of arrays with private and title set. Outside of core, deployed extensions that would need to be updated include MachineVision, TimedMediaHandler, GlobalUsage, and parsoid. Rather than trying to rush through updating all of them, I suggest using RequestContext::getMain for now, and at some point in the future change the method signature so that you don't need to set a user for each file that should be retrieved.
Change 628564 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Remove fallback to $wgUser in LocalRepo::findFiles
Fallback if options['private'] is not empty and not a user object
Callers in core:
BadFileLookup::isBadFile - no options
EditPage::showIntro - no options
wfFindFile - whatever options are passed to it (see below)
Linker::makeThumbLink2 - no options
Linker::makeBrokenImageLinkObj - no options
Linker::makeMediaLinkObj - private is not set
MediaFileHandler::getFile - private is properly set to a user
Title::isAlwaysKnown - no options
InfoAction::pageInfo - no options
ApiImageRotate::execute - no options
ApiMove::execute (x2) - no options
FileRepo::findFiles - based on caller - as noted above, this usually does not have a user set. Suggesting falling back to RequestContext::getMain for now
RepoGroup::findFile (x2) - based on caller, callers already included in this list (same method name)
TraditionalImageGallery::toHTML - no options
ImagePage::loadFile - no options
WikiFilePage::loadFile - no options
CoreParserFunctions::filepath - no options
Parser::fetchFileNoRegister - based on caller, see above
RevisionSearchResultTrait::initFromTitle - no options
SearchNearMatcher::getNearMatchInternal - no options
FullSearchResultWidget::generateFileHtml - no options
SpecialFileDuplicateSearch::execute - no options
SpecialMovepage::doSubmit (x2) - no options
SpecialRedirect::dispatchFile - no options
SpecialWantedfiles::existenceCheck - no options
ImageListPager::formatValue - private is not set
UploadBase::checkOverwrite - private is not set
DeleteBatch::execute - private is not set
DumpUploads::outputItem - no options
ApiUploadTestCase::deleteFileByTitle - private is not set
UploadFromUrlTest::deleteFile - private is not set
thumb.php - no options
Callers in extensions:
(rather than listing everything, because there are a lot of callers, not listing those with hardcoded options that don't set private or don't include any options)
FlaggedRevs - all good
MachineVision - all good
PageImages - all good
TimedMediaHandler - all good
MobileFrontend - mostly good
ApiMobileView::findFile - whatever options are passed to it, since its the same method name callers already included in this list
Wikibase - all good
CirrusSearch - all good
GlobalUsage - all good
ProofreadPage - all good
vendor - not sure if its referring to the same method, but either way no second parameter is passed
3D - all good
AbuseFilter - RCVariableGenerator::addUploadVars - sets private to true, NEEDS FIXING
CommonsMetadata - all good
FileImporter - all good
GeoData - all good
ImageMap - all good
MediaModeration - all good
ParserFunctions - all good
Score - all good
Scribunto - all good
Translate - all good
UploadWizard - all good
VipsScaler - all good
WikibaseMediaInfo - all good
WikidataPageBanner - all good
WikimediaIncubator - all good
parsoid / deploy - not sure if its the same method, but either way no second parameter set
Nostalgia - all good
Timeless - all good
Deployed callers of wfFindFile:
Graph - all good, no options
Translate - not in MW 1.34+, anyway all good, no options
Summary:
AbuseFilter - RCVariableGenerator::addUploadVars - sets private to true, NEEDS FIXING
FileRepo::findFiles - based on caller - as noted above, this usually does not have a user set. Suggesting falling back to RequestContext::getMain for now
Change 628565 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/AbuseFilter@master] Pass a user to RCVariableGenerator::getVars
Change 628606 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Remove fallback to $wgUser in FileRepo::findFile
@Daimona can you please take a look at https://gerrit.wikimedia.org/r/628565 ? Injecting a user into RCVariableGenerator::getVars
Change 628565 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Inject a user into RCVariableGenerator
Change 628606 merged by jenkins-bot:
[mediawiki/core@master] Remove fallback to $wgUser in FileRepo::findFile
Change 628564 merged by jenkins-bot:
[mediawiki/core@master] Remove fallback to $wgUser in LocalRepo::findFiles
Change 628563 merged by jenkins-bot:
[mediawiki/core@master] Remove fallback to $wgUser in FileRepo::findFileFromKey