Page MenuHomePhabricator

Argument 2 passed to File::userCan() must be an instance of User, null given, called in /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/LocalRepo.php on line 275
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

MediaWiki version: 1.36.0-wmf.9

message
Argument 2 passed to File::userCan() must be an instance of User, null given, called in /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/LocalRepo.php on line 275

Impact

About a dozen of these happened on group0 immediately after deployment there during the train. Not enough to be a train blocker yet, but reporting because it seems like a clear programming error.

Notes

Details

Request ID
8d03c252-b9e9-49c2-ace5-9dc7f76009df
Request URL
https://www.mediawiki.org/w/rest.php/www.mediawiki.org/v3/page/pagebundle/India_Hackathon_2011%2FLogo_concepts/441513
Stack Trace
exception.trace
#0 /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/LocalRepo.php(275): File->userCan(integer, NULL)
#1 /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/LocalRepo.php(295): LocalRepo->{closure}(LocalFile, array)
#2 /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/LocalRepo.php(318): LocalRepo->{closure}(Wikimedia\Rdbms\ResultWrapper, array, array)
#3 /srv/mediawiki/php-1.36.0-wmf.9/includes/filerepo/RepoGroup.php(210): LocalRepo->findFiles(array, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/extension/src/Config/DataAccess.php(160): RepoGroup->findFiles(array)
#5 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/PP/Processors/AddMediaInfo.php(425): MWParsoid\Config\DataAccess->getFileInfo(MWParsoid\Config\PageConfig, array)
#6 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/PP/Processors/AddMediaInfo.php(647): Wikimedia\Parsoid\Wt2Html\PP\Processors\AddMediaInfo::requestInfo(Wikimedia\Parsoid\Config\Env, string, array)
#7 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/DOMPostProcessor.php(157): Wikimedia\Parsoid\Wt2Html\PP\Processors\AddMediaInfo->run(Wikimedia\Parsoid\Config\Env, DOMElement, array, boolean)
#8 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/DOMPostProcessor.php(835): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->Wikimedia\Parsoid\Wt2Html\{closure}(DOMElement, array, boolean)
#9 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/DOMPostProcessor.php(884): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->doPostProcess(DOMDocument)
#10 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/DOMPostProcessor.php(901): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->process(DOMDocument)
#11 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/ParserPipeline.php(152): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->processChunkily(string, array)
#12 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/ParserPipeline.php(202): Wikimedia\Parsoid\Wt2Html\ParserPipeline->parseChunkily(string, array)
#13 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Wt2Html/ParserPipelineFactory.php(299): Wikimedia\Parsoid\Wt2Html\ParserPipeline->parseToplevelDoc(string, array)
#14 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Core/WikitextContentModelHandler.php(78): Wikimedia\Parsoid\Wt2Html\ParserPipelineFactory->parse(string)
#15 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Parsoid.php(161): Wikimedia\Parsoid\Core\WikitextContentModelHandler->toDOM(Wikimedia\Parsoid\Config\Env)
#16 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/src/Parsoid.php(193): Wikimedia\Parsoid\Parsoid->parseWikitext(MWParsoid\Config\PageConfig, array)
#17 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/extension/src/Rest/Handler/ParsoidHandler.php(589): Wikimedia\Parsoid\Parsoid->wikitext2html(MWParsoid\Config\PageConfig, array, NULL)
#18 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/extension/src/Rest/Handler/PageHandler.php(88): MWParsoid\Rest\Handler\ParsoidHandler->wt2html(MWParsoid\Config\PageConfig, array)
#19 /srv/mediawiki/php-1.36.0-wmf.9/vendor/wikimedia/parsoid/extension/src/Rest/Handler/ParsoidHandler.php(1046): MWParsoid\Rest\Handler\PageHandler->realExecute()
#20 /srv/mediawiki/php-1.36.0-wmf.9/includes/Rest/Router.php(365): MWParsoid\Rest\Handler\ParsoidHandler->execute()
#21 /srv/mediawiki/php-1.36.0-wmf.9/includes/Rest/Router.php(320): MediaWiki\Rest\Router->executeHandler(MWParsoid\Rest\Handler\PageHandler)
#22 /srv/mediawiki/php-1.36.0-wmf.9/includes/Rest/EntryPoint.php(144): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#23 /srv/mediawiki/php-1.36.0-wmf.9/includes/Rest/EntryPoint.php(110): MediaWiki\Rest\EntryPoint->execute()
#24 /srv/mediawiki/php-1.36.0-wmf.9/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#25 /srv/mediawiki/w/rest.php(3): require(string)
#26 {main}

Event Timeline

Restricted Application added a project: Commons. · View Herald TranscriptSep 16 2020, 10:26 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

There's more of these, at 66 now.

LarsWirzenius triaged this task as Unbreak Now! priority.Sep 16 2020, 10:39 AM

Over 700 now, raising urgency.

Change 627786 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

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

Change 627787 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@wmf/1.36.0-wmf.9] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

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

Change 627788 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@master] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

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

Change 627788 abandoned by Urbanecm:
[mediawiki/core@master] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

Reason:
dupe of https://gerrit.wikimedia.org/r/627786

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

LarsWirzenius lowered the priority of this task from Unbreak Now! to Needs Triage.Sep 16 2020, 10:51 AM
LarsWirzenius added a subscriber: Jdforrester-WMF.

lowering urgency and removing as train blocker after @Jdforrester-WMF backported revert fix for train. This still needs fixing in parsoid properly, though.

Urbanecm triaged this task as High priority.Sep 16 2020, 11:00 AM

Change 627787 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.9] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

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

Mentioned in SAL (#wikimedia-operations) [2020-09-16T11:12:39Z] <jforrester@deploy1001> Synchronized php-1.36.0-wmf.9/includes/filerepo/file: T263014 Revert "Remove support for (Archived|OldLocal)File::userCan without a user" (duration: 01m 04s)

cscott added a subscriber: cscott.EditedSep 16 2020, 2:27 PM

lowering urgency and removing as train blocker after @Jdforrester-WMF backported revert fix for train. This still needs fixing in parsoid properly, though.

I don't think this is actually a Parsoid bug. Core contains this code in LocalRepo.php:

		$fileMatchesSearch = function ( File $file, array $search ) {
			// Note: file name comparison done elsewhere (to handle redirects)
			$user = ( !empty( $search['private'] ) && $search['private'] instanceof User )
				? $search['private']
				: null;

			return (
				$file->exists() &&
				(
					( empty( $search['time'] ) && !$file->isOld() ) ||
					( !empty( $search['time'] ) && $search['time'] === $file->getTimestamp() )
				) &&
				( !empty( $search['private'] ) || !$file->isDeleted( File::DELETED_FILE ) ) &&
				$file->userCan( File::DELETED_FILE, $user )
			);
		};

The path which sets $user to null is in core. So I think that needs to be fixed as part of the "Remove support for (Archived|OldLocal)File::userCan without a user" patch. (And why didn't that "remove support" patch go through the hard-deprecation pathway first, which would have turned up this issue w/o being an UBN?) EDIT: looks like there was a hard-deprecation, so why wasn't this deprecation message in the logs seen previously?

looks like there was a hard-deprecation, so why wasn't this deprecation message in the logs seen previously?

Yeah, I was confused by that too. Maybe it's a newly-used route through the code in Parsoid/something in core?

A simple fix would be to use $user = $wgUser in that section in LocalRepo, which would match the old behavior. But it seems like we should figure out where those wfDeprecated messages disappeared to in case there are other lurking issues here which weren't previously caught.

The Parsoid DataAccess code in question was last changed in 0b9bc6b8ec (July) although even that didn't actually alter the ::fileFiles call in question. So I don't think there was anything recently changed in Parsoid-land. It's possible that the LocalRepo.php(275): File->userCan(integer, NULL) => LocalRepo.php(295): LocalRepo->{closure}(LocalFile, array) => LocalRepo.php(318): LocalRepo->{closure}(Wikimedia\Rdbms\ResultWrapper, array, array) => RepoGroup.php(210): LocalRepo->findFiles(array, integer) path changed more recently though.

ssastry added a subscriber: ssastry.

Untagging Parsoid.

Ah, found it: the issue is that File::userCan(...$user=null) wasn't previously hard-deprecated, even though the null case in ArchivedFile::userCan and OldLocalFile::userCan were. So we weren't seeing the hard-deprecation message (and probably there are a bunch of other users which weren't previously tickling the hard deprecation).

Change 627863 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Hard deprecate File::userCan() with $user=null

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

Change 627786 merged by jenkins-bot:
[mediawiki/core@master] Revert "Remove support for (Archived|OldLocal)File::userCan without a user"

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

Change 627863 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecate File::userCan() with $user=null

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

Change 627801 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@REL1_35] Hard deprecate File::userCan() with $user=null

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

Change 627801 merged by jenkins-bot:
[mediawiki/core@REL1_35] Hard deprecate File::userCan() with $user=null

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

DannyS712 closed this task as Resolved.Sep 20 2020, 8:47 PM
DannyS712 assigned this task to cscott.