Page MenuHomePhabricator

Wikimedia\Assert\ParameterAssertionException when rendering a log snippet and log_user_text is empty
Closed, ResolvedPublic

Description

https://en.wiktionary.org/w/index.php?title=Wyvern&action=edit

`
[XM5xIQpAIEIAAHdQDLEAAABH] 2019-05-05 05:14:09: Wikimedia\Assert\ParameterAssertionException from line 63 of /srv/mediawiki/php-1.34.0-wmf.3/vendor/wikimedia/assert/src/Assert.php: Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty
#0 /srv/mediawiki/php-1.34.0-wmf.3/includes/title/TitleValue.php(107): Wikimedia\Assert\Assert::parameter(boolean, string, string)
#1 /srv/mediawiki/php-1.34.0-wmf.3/includes/Linker.php(1021): TitleValue->__construct(integer, string)
#2 /srv/mediawiki/php-1.34.0-wmf.3/includes/Linker.php(951): Linker::userTalkLink(integer, string)
#3 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogFormatter.php(768): Linker::userToolLinks(integer, string, boolean, integer, NULL, boolean)
#4 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogFormatter.php(674): LogFormatter->makeUserLink(User)
#5 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogFormatter.php(552): LogFormatter->getPerformerElement()
#6 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/MoveLogFormatter.php(50): LogFormatter->getMessageParameters()
#7 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogFormatter.php(465): MoveLogFormatter->getMessageParameters()
#8 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogFormatter.php(440): LogFormatter->getActionMessage()
#9 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogEventsList.php(397): LogFormatter->getActionText()
#10 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogPager.php(397): LogEventsList->logLine(stdClass)
#11 /srv/mediawiki/php-1.34.0-wmf.3/includes/pager/IndexPager.php(490): LogPager->formatRow(stdClass)
#12 /srv/mediawiki/php-1.34.0-wmf.3/includes/logging/LogEventsList.php(689): IndexPager->getBody()
#13 /srv/mediawiki/php-1.34.0-wmf.3/includes/EditPage.php(2662): LogEventsList::showLogExtract(OutputPage, array, Title, string, array)
#14 /srv/mediawiki/php-1.34.0-wmf.3/includes/EditPage.php(677): EditPage->showIntro()
#15 /srv/mediawiki/php-1.34.0-wmf.3/includes/actions/EditAction.php(60): EditPage->edit()
#16 /srv/mediawiki/php-1.34.0-wmf.3/includes/MediaWiki.php(499): EditAction->show()
#17 /srv/mediawiki/php-1.34.0-wmf.3/includes/MediaWiki.php(294): MediaWiki->performAction(Article, Title)
#18 /srv/mediawiki/php-1.34.0-wmf.3/includes/MediaWiki.php(865): MediaWiki->performRequest()
#19 /srv/mediawiki/php-1.34.0-wmf.3/includes/MediaWiki.php(515): MediaWiki->main()
#20 /srv/mediawiki/php-1.34.0-wmf.3/index.php(42): MediaWiki->run()
#21 /srv/mediawiki/w/index.php(3): include(string)
#22 {main}

Event Timeline

A2093064 created this task.May 5 2019, 5:14 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 5 2019, 5:14 AM
Tgr updated the task description. (Show Details)May 5 2019, 6:55 AM
Tgr added a subscriber: Tgr.May 5 2019, 8:47 AM

So the page is move+deleted, edit (also view/history) tries to show a move log snippet, and LogEventsList::showLogExtract explodes. Probably due to rMWb6e1e99bec8d: Use LinkTarget in Linker instead of Title?
https://en.wiktionary.org/wiki/Special:Undelete/Wyvern says

(diff) 01:34, 30 June 2005 . . Conversion script (talk | block) 20 bytes (Wyvern moved to wyvern: Converting page titles to lowercase)

Conversion script does not have a local account, so I'm guessing it's also present in the move log, User::getName() ends up being empty somehow(?) and we now have an assert for that.

So this is an actual bug -- until now, we were presumably creating a bogus link (to https://en.wiktionary.org/wiki/User_talk:). What should we actually do here? Is the database actually supposed to ever have empty usernames in it? That doesn't seem reasonable. Surely conversion scripts should also have usernames?

daniel added a subscriber: daniel.May 5 2019, 7:52 PM

As a stop gap, I propose to have Linker::userToolLinks bail out early if the user name is empty, returning an empty string or an (HTML) error message. The issue should also be logged.
The same could be done in LogFormatter::makeUserLink, which could provide more context to the log. To be safe, we should probably do both.

So the conversion script revisions have Conversion script as rev_user_text and 0 as rev_user. Which is interesting on its own right but I guess whatever injected that script name, didn't do it right for the move log.

wikiadmin@10.64.48.34(enwiktionary)> select log_id, log_user, log_user_text, actor.* from logging join actor on actor_id = log_actor where log_namespace = 0 and log_title = 'Wyvern' and log_type = 'move' \G
*************************** 1. row ***************************
       log_id: 68369
     log_user: 0
log_user_text: 
     actor_id: 4090805
   actor_user: NULL
   actor_name: 
1 row in set (0.02 sec)

As Daniel says, this shouldn't make a page inaccessible; there should be some fallback username (<invalid user> or <no name> or something like that).

Change 508287 had a related patch set uploaded (by simetrical; owner: simetrical):
[mediawiki/core@master] Log warning and show error on empty username

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

daniel triaged this task as High priority.May 6 2019, 3:38 PM

Bumping to high, since this interferes with critical functionality and causes a fatal error.

Aklapper renamed this task from Wikimedia\Assert\ParameterAssertionException on enwiktionary to Wikimedia\Assert\ParameterAssertionException on enwiktionary when creating page with uppercase letter and lowercase version exists.May 6 2019, 4:56 PM
BPirkle claimed this task.May 6 2019, 5:22 PM
BPirkle added a subscriber: BPirkle.

I'll pick up where @Simetrical left off.

Tgr renamed this task from Wikimedia\Assert\ParameterAssertionException on enwiktionary when creating page with uppercase letter and lowercase version exists to Wikimedia\Assert\ParameterAssertionException when rendering a log snippet and log_user_text is empty.May 6 2019, 5:23 PM
Tgr removed BPirkle as the assignee of this task.
Tgr assigned this task to BPirkle.

Change 508287 merged by jenkins-bot:
[mediawiki/core@master] Log warning and show error on empty username

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

This comment was removed by daniel.

Change 508559 had a related patch set uploaded (by Daniel Kinzler; owner: simetrical):
[mediawiki/core@wmf/1.34.0-wmf.3] Log warning and show error on empty username

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

Scheduled for European mid-day SWAT on Wednesday.

I don't know of any good way to test this on beta. This ticket should stay open until we can confirm that https://en.wiktionary.org/w/index.php?title=Wyvern&action=edit is working.

Change 508559 merged by jenkins-bot:
[mediawiki/core@wmf/1.34.0-wmf.3] Log warning and show error on empty username

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

Mentioned in SAL (#wikimedia-operations) [2019-05-08T12:06:43Z] <kartik@deploy1001> Synchronized php-1.34.0-wmf.3: SWAT: [[gerrit:508559|Log warning and show error on empty username (T222529)]] (duration: 07m 29s)

daniel closed this task as Resolved.May 8 2019, 12:09 PM

Confirmed: https://en.wiktionary.org/w/index.php?title=Wyvern&action=edit no longer causes an exception.

Localization messages have not made it into the cache yet, but I suppose that will happen soon.

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