Page MenuHomePhabricator

passing invalid timestamp in url for a file returns fatal
Closed, ResolvedPublicPRODUCTION ERROR

Description

Reproducible urls:

https://de.wikipedia.org/w/index.php?title=Datei:Juan_de_Flandes_001.jpg&filetimestamp=00000000000000
https://de.wikipedia.org/w/index.php?title=Datei:Label_for_dangerous_goods_-_class_4.2.svg&filetimestamp=2007081805593
https://de.wikipedia.org/w/index.php?title=File:Not-known.svg&filetimestamp=2007081805593

Example trace from Logstash:

#0 /srv/mediawiki/php-1.31.0-wmf.21/includes/libs/rdbms/database/Database.php(3184): Wikimedia\Timestamp\ConvertibleTimestamp->getTimestamp(integer)
#1 /srv/mediawiki/php-1.31.0-wmf.21/includes/filerepo/file/OldLocalFile.php(238): Wikimedia\Rdbms\Database->timestamp(string)
#2 /srv/mediawiki/php-1.31.0-wmf.21/includes/filerepo/file/LocalFile.php(311): OldLocalFile->loadFromDB(integer)
#3 /srv/mediawiki/php-1.31.0-wmf.21/includes/filerepo/file/LocalFile.php(628): LocalFile->loadFromCache()
#4 /srv/mediawiki/php-1.31.0-wmf.21/includes/filerepo/FileRepo.php(450): LocalFile->load(integer)
#5 /srv/mediawiki/php-1.31.0-wmf.21/includes/filerepo/RepoGroup.php(154): FileRepo->findFile(Title, array)
#6 /srv/mediawiki/php-1.31.0-wmf.21/includes/GlobalFunctions.php(2928): RepoGroup->findFile(Title, array)
#7 /srv/mediawiki/php-1.31.0-wmf.21/extensions/FlaggedRevs/frontend/FlaggablePageView.php(927): wfFindFile(Title, array)
#8 /srv/mediawiki/php-1.31.0-wmf.21/extensions/FlaggedRevs/frontend/FlaggedRevsUI.hooks.php(162): FlaggablePageView->imagePageFindFile(boolean, boolean)
#9 /srv/mediawiki/php-1.31.0-wmf.21/includes/Hooks.php(177): FlaggedRevsUIHooks::onImagePageFindFile(ImagePage, boolean, boolean)
#10 /srv/mediawiki/php-1.31.0-wmf.21/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#11 /srv/mediawiki/php-1.31.0-wmf.21/includes/page/ImagePage.php(75): Hooks::run(string, array)
#12 /srv/mediawiki/php-1.31.0-wmf.21/includes/page/ImagePage.php(114): ImagePage->loadFile()
#13 /srv/mediawiki/php-1.31.0-wmf.21/includes/actions/ViewAction.php(68): ImagePage->view()
#14 /srv/mediawiki/php-1.31.0-wmf.21/includes/MediaWiki.php(500): ViewAction->show()
#15 /srv/mediawiki/php-1.31.0-wmf.21/includes/MediaWiki.php(294): MediaWiki->performAction(ImagePage, Title)
#16 /srv/mediawiki/php-1.31.0-wmf.21/includes/MediaWiki.php(861): MediaWiki->performRequest()
#17 /srv/mediawiki/php-1.31.0-wmf.21/includes/MediaWiki.php(524): MediaWiki->main()
#18 /srv/mediawiki/php-1.31.0-wmf.21/index.php(42): MediaWiki->run()
#19 /srv/mediawiki/w/index.php(3): include(string)
#20 {main}

I see two problems here:

  1. The url parameter should not be unconditionally trusted as if it was a valid timestamp.
  2. It seems in this particular case, there is no file by this name. Which means we should probably fail for "unknown file" before we even consider the timestamp in the first place.

I am able to consistently reproduce this on German Wikipedia, but not on Commons. Which suggests that bug number 2 (looking up filetimestamp before knowing that the file is valid) might be specific to FlaggedRevs or some other dewiki extension.

Event Timeline

Change 453538 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/FlaggedRevs@master] Call MWTimestamp::convert on query provided timestamp

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

Umherirrender triaged this task as Medium priority.
Umherirrender added a subscriber: Umherirrender.
  1. It seems in this particular case, there is no file by this name. Which means we should probably fail for "unknown file" before we even consider the timestamp in the first place.

That is not possible, because the name and timestamp is used to find the file in one database query. Validating the name first seems waste of resources, because in 99 % the name is valid, because building the url manually is rare.

Change 453538 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] Call MWTimestamp::convert on query provided timestamp

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

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:11 PM