Page MenuHomePhabricator

null argument on DifferenceEngine->generateContentDiffBody() on Special:Undelete
Closed, ResolvedPublicPRODUCTION ERROR

Description

Steps to reproduce:

  1. Create a page, do three edits. It should have four lines in its history.
  2. With an oversight account, select the too oldest revs (the creation and the first edit) and delete them (mark "hide the content" "also for admins")
  3. With a simple admin account, go to Special:Undelete, the two more recent "diff" links are blue. When you click on the second diff, an error is shown.

The error appear when an admin tries to see a diff between two revs, when one of both of them was deleted.

Backtrace I reproduced on vagrant:

[d2fc727824a22ed6c87bda38] /w/index.php?title=Special:Undelete&target=Test&timestamp=20180827184204&diff=prev TypeError from line 1200 of /vagrant/mediawiki/includes/diff/DifferenceEngine.php: Argument 1 passed to DifferenceEngine::generateContentDiffBody() must implement interface Content, null given, called in /vagrant/mediawiki/includes/specials/SpecialUndelete.php on line 537

Backtrace:

#0 /vagrant/mediawiki/includes/specials/SpecialUndelete.php(537): DifferenceEngine->generateContentDiffBody(NULL, WikitextContent)
#1 /vagrant/mediawiki/includes/specials/SpecialUndelete.php(399): SpecialUndelete->showDiff(Revision, Revision)
#2 /vagrant/mediawiki/includes/specials/SpecialUndelete.php(182): SpecialUndelete->showRevision(string)
#3 /vagrant/mediawiki/includes/specialpage/SpecialPage.php(569): SpecialUndelete->execute(NULL)
#4 /vagrant/mediawiki/includes/specialpage/SpecialPageFactory.php(581): SpecialPage->run(NULL)
#5 /vagrant/mediawiki/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#6 /vagrant/mediawiki/includes/MediaWiki.php(868): MediaWiki->performRequest()
#7 /vagrant/mediawiki/includes/MediaWiki.php(525): MediaWiki->main()
#8 /vagrant/mediawiki/index.php(42): MediaWiki->run()
#9 /var/www/w/index.php(5): require(string)
#10 {main}

Vue generale.png (964×1 px, 202 KB)

error.png (866×1 px, 37 KB)

Reported by @Bastenbas

Event Timeline

Tgr added a subscriber: Tgr.

DifferenceEngine::generateContentDiffBody takes two Content objects but when a user without the appropriate permissions fetches the content, they get null. Special:Undelete apparently lack error handling for that case.

Not really related to MCR which did not change any relevant code. OTOH at some point this code will need to be rewritten for MCR anyway since it can't handle multiple slots - that's T201848: Make DifferenceEngine callers pass revisions, not contents. Given the fringe circumstances needed for this error, I think we can just fix it then.

I merged a patch by Gergö that touches the relevant code: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/461863

From manual testing, this seems to fix the issue. The fix should go live with the regular deployment "train" next week, please test again and let me know whether the problem is solved.

Krinkle assigned this task to Tgr.
Krinkle added a subscriber: Krinkle.

Not seen in WMF Logstash for at least 30 days.

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