Maniphest T301947

EditPage.php on line 2286
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Assigned To

Authored By

	hashar
	Feb 17 2022, 8:38 AM

Description

Error

mwversion: 1.38.0-wmf.21
reqId: 044114d4-4e85-4602-9a26-37b354fd430b
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   TypeError: Argument 5 passed to MediaWiki\EditPage\Constraint\AutoSummaryMissingSummaryConstraint::__construct() must implement interface Content, null given, called in /srv/mediawiki/php-1.38.0-wmf.21/includes/EditPage.php on

exception.trace

from /srv/mediawiki/php-1.38.0-wmf.21/includes/editpage/Constraint/AutoSummaryMissingSummaryConstraint.php(68)
#0 /srv/mediawiki/php-1.38.0-wmf.21/includes/EditPage.php(2286): MediaWiki\EditPage\Constraint\AutoSummaryMissingSummaryConstraint->__construct(string, string, boolean, WikitextContent, NULL)
#1 /srv/mediawiki/php-1.38.0-wmf.21/includes/EditPage.php(1696): EditPage->internalAttemptSave(NULL, boolean, boolean)
#2 /srv/mediawiki/php-1.38.0-wmf.21/includes/EditPage.php(670): EditPage->attemptSave(NULL)
#3 /srv/mediawiki/php-1.38.0-wmf.21/includes/actions/EditAction.php(71): EditPage->edit()
#4 /srv/mediawiki/php-1.38.0-wmf.21/includes/actions/SubmitAction.php(38): EditAction->show()
#5 /srv/mediawiki/php-1.38.0-wmf.21/includes/MediaWiki.php(543): SubmitAction->show()
#6 /srv/mediawiki/php-1.38.0-wmf.21/includes/MediaWiki.php(320): MediaWiki->performAction(Article, Title)
#7 /srv/mediawiki/php-1.38.0-wmf.21/includes/MediaWiki.php(903): MediaWiki->performRequest()
#8 /srv/mediawiki/php-1.38.0-wmf.21/includes/MediaWiki.php(563): MediaWiki->main()
#9 /srv/mediawiki/php-1.38.0-wmf.21/index.php(53): MediaWiki->run()
#10 /srv/mediawiki/php-1.38.0-wmf.21/index.php(46): wfIndexMain()
#11 /srv/mediawiki/w/index.php(3): require(string)
#12 {main}

Impact

A single occurrence found in the log. Maybe because a person submitted an edit without any summary?

Notes

Details

Request URL: https://ja.wikipedia.org/w/index.php?title=*&action=submit

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	EditPage: Improve handling for revision deleted while editing is ongoing	mediawiki/core	master	+16 -2

Customize query in gerrit

Related Objects

Mentioned In: T293313: TypeError: Argument 1 passed to EditPage::displayViewSourcePage() must implement interface Content, null given, called in /srv/mediawiki/php-1.38.0-wmf.3/includes/EditPage.php on line 646
Mentioned Here: T341396: Improve wikitext editor behavior when editing a deleted revision of a page
T341397: Improve visual editor behavior when editing a deleted revision of a page
rMWbd371e2e94e9: EditPage: misc cleanup

Event Timeline

hashar created this task.Feb 17 2022, 8:38 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 17 2022, 8:38 AM

Zabe subscribed.Feb 17 2022, 2:00 PM

Krinkle moved this task from Untriaged to Feb 2022 on the Wikimedia-production-error board.Feb 17 2022, 4:46 PM

DannyS712 subscribed.Feb 17 2022, 9:36 PM

Argument 5 is the original content of the page, not the edit summary.
I suspect perhaps rMWbd371e2e94e9: EditPage: misc cleanup, CC @Daimona since this was dealing with fairly brittle code. Something in the comparisons of == 'new' vs === 'new' maybe?

In T301947#7719902, @DannyS712 wrote:

Argument 5 is the original content of the page, not the edit summary.
I suspect perhaps rMWbd371e2e94e9: EditPage: misc cleanup, CC @Daimona since this was dealing with fairly brittle code. Something in the comparisons of == 'new' vs === 'new' maybe?

A sensible guess per se, but wrong in this case. On logstash I'm seeing instances of this error back in November; in fact, there've been 21 hits in the last 3 months. Also, EditPage::getOriginalContent is documented as @return Content|null, but its return value is being unconditionally passed to AutoSummaryMissingSummaryConstraint which requires a Content, so the code seems wrong.

Looking at the code, my first guess would be that a permission check fails, e.g. if the page content is (rev)deleted while you are editing and you're no longer allowed to view it by the time you try to save the page. I think this would fit with the rate of 7 errors/month.

...And this is now reported by phan at r761914, thanks to the patch adding a return typehint to getOriginalContent and r769514 enabling stricter rules for phan.

In T301947#7720896, @Daimona wrote:

Looking at the code, my first guess would be that a permission check fails, e.g. if the page content is (rev)deleted while you are editing and you're no longer allowed to view it by the time you try to save the page. I think this would fit with the rate of 7 errors/month.

Seems like I was right. I was able to reproduce this locally as follows:

Create a page, then edit it again
In an incognito window, as logged out, open the first version of the page in edit mode
Then, in a logged-in session, revdelete the first version of the page
In the anon session, try to publish an edit

The question is how to fix this. If I disable the summary check, the edit goes through. I'm not sure if this is wanted: if that revision was deleted in the meanwhile, you'd likely be restoring deleted comment, which

You shouldn't be able to do, and
Would require another revdeletion to get rid of the unwanted stuff

I don't think this has much potential for abuse, but I'd say it's more serious than a simple type error.

matmarex mentioned this in T293313: TypeError: Argument 1 passed to EditPage::displayViewSourcePage() must implement interface Content, null given, called in /srv/mediawiki/php-1.38.0-wmf.3/includes/EditPage.php on line 646.Jun 29 2023, 8:55 PM

Change 936082 had a related patch set uploaded (by Ammarpad; author: Ammarpad):

[mediawiki/core@master] EditPage: Improve handling for revision deleted while editing is ongoing

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

gerritbot added a project: Patch-For-Review.Jul 6 2023, 7:05 PM

Change 936082 merged by jenkins-bot: