Page MenuHomePhabricator

"Fatal exception: MediaWiki\Revision\InvalidArgumentException" when trying to move user JS page on en.wp
Open, NormalPublic

Description

Attempting to move in own user space JS from

to

caused uncaught

Fatal exception of type "InvalidArgumentException"

several times. The origial plan has been not to move talk page, but trying other options without watching or with moving talk page brought the same error.

In T225309#5243275, @Aklapper wrote:
Thanks for reporting this! Please always post the exception_ID

Okay, here we are:

  • XPwI7wpAMFgAADILjcgAAAAG
  • XPwJYwpAIDAAAFTPS18AAACF
  • XPwJfApAICIAAIrzugkAAABP
  • XPwJogpAICsAAH5VdoIAAABS

Actually, I am not used to report fatal errors over years.

Details

Related Gerrit Patches:

Event Timeline

Reedy added a subscriber: Reedy.
2019-06-08 19:13:52 [XPwI7wpAMFgAADILjcgAAAAG] mw1253 enwiki 1.34.0-wmf.8 exception ERROR: [XPwI7wpAMFgAADILjcgAAAAG] /w/index.php?title=Special:MovePage&action=submit   InvalidArgumentException from line 100 of /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php: The given Title does not belong to page ID 41443888 but actually belongs to 60994499 {"exception_id":"XPwI7wpAMFgAADILjcgAAAAG","exception_url":"/w/index.php?title=Special:MovePage&action=submit","caught_by":"mwe_handler"} 
[Exception InvalidArgumentException] (/srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php:100) The given Title does not belong to page ID 41443888 but actually belongs to 60994499
  #0 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1826): MediaWiki\Revision\RevisionStoreRecord->__construct(Title, User, CommentStoreComment, stdClass, MediaWiki\Revision\RevisionSlots, boolean)
  #1 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(2167): MediaWiki\Revision\RevisionStore->newRevisionFromRow(stdClass, integer, Title)
  #2 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1535): MediaWiki\Revision\RevisionStore->loadRevisionFromConds(Wikimedia\Rdbms\DBConnRef, array, integer, Title)
  #3 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(1032): MediaWiki\Revision\RevisionStore->getRevisionByTitle(Title)
  #4 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(264): MediaWiki\Permissions\PermissionManager->checkUserConfigPermissions(string, User, array, string, boolean, Title)
  #5 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(145): MediaWiki\Permissions\PermissionManager->getPermissionErrorsInternal(string, User, Title, string)
  #6 /srv/mediawiki/php-1.34.0-wmf.8/includes/Title.php(2252): MediaWiki\Permissions\PermissionManager->getPermissionErrors(string, User, Title, string, array)
  #7 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(655): Title->getUserPermissionsErrors(string, User)
  #8 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(130): MovePageForm->doSubmit()
  #9 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPage.php(570): MovePageForm->execute(NULL)
  #10 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPageFactory.php(575): SpecialPage->run(NULL)
  #11 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
  #12 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(865): MediaWiki->performRequest()
  #13 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(515): MediaWiki->main()
  #14 /srv/mediawiki/php-1.34.0-wmf.8/index.php(42): MediaWiki->run()
  #15 /srv/mediawiki/w/index.php(3): require(string)
  #16 {main}
2019-06-08 19:15:48 [XPwJYwpAIDAAAFTPS18AAACF] mw1327 enwiki 1.34.0-wmf.8 exception ERROR: [XPwJYwpAIDAAAFTPS18AAACF] /w/index.php?title=Special:MovePage&action=submit   InvalidArgumentException from line 100 of /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php: The given Title does not belong to page ID 41443888 but actually belongs to 60994510 {"exception_id":"XPwJYwpAIDAAAFTPS18AAACF","exception_url":"/w/index.php?title=Special:MovePage&action=submit","caught_by":"mwe_handler"} 
[Exception InvalidArgumentException] (/srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php:100) The given Title does not belong to page ID 41443888 but actually belongs to 60994510
  #0 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1826): MediaWiki\Revision\RevisionStoreRecord->__construct(Title, User, CommentStoreComment, stdClass, MediaWiki\Revision\RevisionSlots, boolean)
  #1 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(2167): MediaWiki\Revision\RevisionStore->newRevisionFromRow(stdClass, integer, Title)
  #2 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1535): MediaWiki\Revision\RevisionStore->loadRevisionFromConds(Wikimedia\Rdbms\DBConnRef, array, integer, Title)
  #3 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(1032): MediaWiki\Revision\RevisionStore->getRevisionByTitle(Title)
  #4 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(264): MediaWiki\Permissions\PermissionManager->checkUserConfigPermissions(string, User, array, string, boolean, Title)
  #5 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(145): MediaWiki\Permissions\PermissionManager->getPermissionErrorsInternal(string, User, Title, string)
  #6 /srv/mediawiki/php-1.34.0-wmf.8/includes/Title.php(2252): MediaWiki\Permissions\PermissionManager->getPermissionErrors(string, User, Title, string, array)
  #7 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(655): Title->getUserPermissionsErrors(string, User)
  #8 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(130): MovePageForm->doSubmit()
  #9 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPage.php(570): MovePageForm->execute(NULL)
  #10 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPageFactory.php(575): SpecialPage->run(NULL)
  #11 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
  #12 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(865): MediaWiki->performRequest()
  #13 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(515): MediaWiki->main()
  #14 /srv/mediawiki/php-1.34.0-wmf.8/index.php(42): MediaWiki->run()
  #15 /srv/mediawiki/w/index.php(3): require(string)
  #16 {main}
2019-06-08 19:16:12 [XPwJfApAICIAAIrzugkAAABP] mw1332 enwiki 1.34.0-wmf.8 exception ERROR: [XPwJfApAICIAAIrzugkAAABP] /w/index.php?title=Special:MovePage&action=submit   InvalidArgumentException from line 100 of /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php: The given Title does not belong to page ID 41443888 but actually belongs to 60994514 {"exception_id":"XPwJfApAICIAAIrzugkAAABP","exception_url":"/w/index.php?title=Special:MovePage&action=submit","caught_by":"mwe_handler"} 
[Exception InvalidArgumentException] (/srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php:100) The given Title does not belong to page ID 41443888 but actually belongs to 60994514
  #0 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1826): MediaWiki\Revision\RevisionStoreRecord->__construct(Title, User, CommentStoreComment, stdClass, MediaWiki\Revision\RevisionSlots, boolean)
  #1 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(2167): MediaWiki\Revision\RevisionStore->newRevisionFromRow(stdClass, integer, Title)
  #2 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1535): MediaWiki\Revision\RevisionStore->loadRevisionFromConds(Wikimedia\Rdbms\DBConnRef, array, integer, Title)
  #3 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(1032): MediaWiki\Revision\RevisionStore->getRevisionByTitle(Title)
  #4 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(264): MediaWiki\Permissions\PermissionManager->checkUserConfigPermissions(string, User, array, string, boolean, Title)
  #5 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(145): MediaWiki\Permissions\PermissionManager->getPermissionErrorsInternal(string, User, Title, string)
  #6 /srv/mediawiki/php-1.34.0-wmf.8/includes/Title.php(2252): MediaWiki\Permissions\PermissionManager->getPermissionErrors(string, User, Title, string, array)
  #7 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(655): Title->getUserPermissionsErrors(string, User)
  #8 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(130): MovePageForm->doSubmit()
  #9 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPage.php(570): MovePageForm->execute(NULL)
  #10 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPageFactory.php(575): SpecialPage->run(NULL)
  #11 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
  #12 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(865): MediaWiki->performRequest()
  #13 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(515): MediaWiki->main()
  #14 /srv/mediawiki/php-1.34.0-wmf.8/index.php(42): MediaWiki->run()
  #15 /srv/mediawiki/w/index.php(3): require(string)
  #16 {main}
2019-06-08 19:16:51 [XPwJogpAICsAAH5VdoIAAABS] mw1322 enwiki 1.34.0-wmf.8 exception ERROR: [XPwJogpAICsAAH5VdoIAAABS] /w/index.php?title=Special:MovePage&action=submit   InvalidArgumentException from line 100 of /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php: The given Title does not belong to page ID 41443888 but actually belongs to 60994518 {"exception_id":"XPwJogpAICsAAH5VdoIAAABS","exception_url":"/w/index.php?title=Special:MovePage&action=submit","caught_by":"mwe_handler"} 
[Exception InvalidArgumentException] (/srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStoreRecord.php:100) The given Title does not belong to page ID 41443888 but actually belongs to 60994518
  #0 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1826): MediaWiki\Revision\RevisionStoreRecord->__construct(Title, User, CommentStoreComment, stdClass, MediaWiki\Revision\RevisionSlots, boolean)
  #1 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(2167): MediaWiki\Revision\RevisionStore->newRevisionFromRow(stdClass, integer, Title)
  #2 /srv/mediawiki/php-1.34.0-wmf.8/includes/Revision/RevisionStore.php(1535): MediaWiki\Revision\RevisionStore->loadRevisionFromConds(Wikimedia\Rdbms\DBConnRef, array, integer, Title)
  #3 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(1032): MediaWiki\Revision\RevisionStore->getRevisionByTitle(Title)
  #4 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(264): MediaWiki\Permissions\PermissionManager->checkUserConfigPermissions(string, User, array, string, boolean, Title)
  #5 /srv/mediawiki/php-1.34.0-wmf.8/includes/Permissions/PermissionManager.php(145): MediaWiki\Permissions\PermissionManager->getPermissionErrorsInternal(string, User, Title, string)
  #6 /srv/mediawiki/php-1.34.0-wmf.8/includes/Title.php(2252): MediaWiki\Permissions\PermissionManager->getPermissionErrors(string, User, Title, string, array)
  #7 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(655): Title->getUserPermissionsErrors(string, User)
  #8 /srv/mediawiki/php-1.34.0-wmf.8/includes/specials/SpecialMovepage.php(130): MovePageForm->doSubmit()
  #9 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPage.php(570): MovePageForm->execute(NULL)
  #10 /srv/mediawiki/php-1.34.0-wmf.8/includes/specialpage/SpecialPageFactory.php(575): SpecialPage->run(NULL)
  #11 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
  #12 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(865): MediaWiki->performRequest()
  #13 /srv/mediawiki/php-1.34.0-wmf.8/includes/MediaWiki.php(515): MediaWiki->main()
  #14 /srv/mediawiki/php-1.34.0-wmf.8/index.php(42): MediaWiki->run()
  #15 /srv/mediawiki/w/index.php(3): require(string)
  #16 {main}
Tgr added a subscriber: Tgr.Jun 9 2019, 3:36 PM

DB lag or some uninvalidated cache, presumably. Note that the permission check is triggered from a security patch that's more or less the equivalent of gerrit 502667 (would be nice to get that merged or dropped) so that will be needed to reproduce it. (I tried but worked fine for me locally. Which is not surprising if it is indeed DB lag.)

Persisting:

[XP6iKApAICIAAJV-xuIAAACS] 2019-06-10 18:32:10

At least no temporary DB lag.

Move worked in March.

daniel added a subscriber: daniel.EditedJun 12 2019, 9:48 AM

At least no temporary DB lag.

So this is caused by SpecialMovepage checking the move-subpages right just after moving the page to a different title, which, with the patch mentioned by @Tgr applied, will try to load the latest revision of each subpage to see whether it's a redirect. This seems to fail because it's loading a new Title object from a stale replica, ending up with the wrong page ID.

One quick fix would be to simply check the right to move the subpages before moving the page itself. I see no need to do this after the move.

But the whole situation confuses me a lot:

  • Looking at the code in the patch on gerrit, it seems like the permission check is done on the "base" page, not for each subpage. But I must be confused about this, since the check should be skipped if Title::isUserJsConfigPage returns false.
  • Looking at RevisionStore::getRevisionbyTitle, it seems like the same Title object is used for querying the revision and constructing the RevisionRecord. So there should be no inconsistency there, even if the Title has the wrong page ID.
  • Since we should be checking whether the subpage is a redirect, not the "base" page, there is no reason for the Title object to be stale - the subpage wasn't moved yet.

My suspicion is that the security patch that does the check somehow uses the wrong Title object.

Also, there is code duplication between SpecialMovepage (aka PageMoveForm) and the MovePage helper. Both have code to move subpages. Perhaps this duplication could be removed when fixing this problem.

Tgr added a comment.Jun 12 2019, 11:02 AM

move-subpages is just a right like any other; I don't see any special-casing that would actually try to check anything about the subpages. User:PerfektesChaos/js/WikiSyntaxTextMod/rO.js does not have subpages, anyway. So if there is a replag issue, that can only affect the Title object of the base page.

Given that the bug started in March, and the security patch has been around for a long time, maybe the refactoring from Title to LinkTarget caused a new Title object to be created where previously the same object would have been passed along, preserving any state changes. But then, LinkCache should preserve the state change anyway and MovePage does seem to invalidate it correctly.

Tgr added a comment.Jun 12 2019, 11:25 AM

One quick fix would be to simply check the right to move the subpages before moving the page itself. I see no need to do this after the move.

It seems more correct, even. If that permission somehow depends on the content of the page (say, users are only allowed to move subpages of pages they created), that check should be performed on the page that is to be moved, not on the redirect that gets created in its place.

Given that the bug started in March, and the security patch has been around for a long time, maybe the refactoring from Title to LinkTarget caused a new Title object to be created where previously the same object would have been passed along, preserving any state changes.

Given the fact that Title::newFromLinkTarget will just return the parameter unchanged if it's already a Title, that should still work. And given that RevisionStore uses the same Title for querying and object construction should ensure consistency, even if it's stale. It's rather odd.

One quick fix would be to simply check the right to move the subpages before moving the page itself. I see no need to do this after the move.

It seems more correct, even. If that permission somehow depends on the content of the page (say, users are only allowed to move subpages of pages they created), that check should be performed on the page that is to be moved, not on the redirect that gets created in its place.

Right. Moreover, we should probably only check that right if moving subpages was requested in the first place.

I'll make a patch, but I feel uneasy about the fact that this will hide the stale Title issue instead of finding it.

Change 516614 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Do move options checks before the move

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

Tgr added a comment.Jun 12 2019, 8:02 PM

I wonder what exactly triggers the error... I can't reproduce it on beta enwiki, which does have replicas. Also, there seem to be plenty of JS subpage moves on enwiki (although they all change the username, not the filename, so probably rename-related; and maybe that skips some permission checks?)

Change 516614 merged by jenkins-bot:
[mediawiki/core@master] Do move options checks before the move

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

For the record: success on 2019-07-03.

daniel added a comment.Jul 5 2019, 5:12 PM

So, the concrete issue was fixed by moving the check, but we still don't know the root cause. Is that correct?

Tgr added a comment.Jul 5 2019, 5:31 PM

Yeah. The error was reproducible so I suppose someone could revert on mwdebug, add logging if needed, and test there.

This time a regular article page, no JS nor user page.

[XVBv5wpAADsAAGy2lboAAACV] 2019-08-11 19:43:36: Fataler Ausnahmefehler des Typs „InvalidArgumentException“

Attempt to move back Albert Feldstein‎ back over Al Feldstein.

Same attempt before, same message already on 2 May 2019.

May have similar cause, or might require new thread.

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