Page MenuHomePhabricator

LocalRenameUserJob: escape '$' in replacement title
Closed, ResolvedPublic

Description

Please take a look at the rename progress, which has failed and appears to be stuck.

Opened here per instructions. See also: T137973.

Event Timeline

Nihlus created this task.Feb 24 2018, 11:15 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
MarcoAurelio triaged this task as Normal priority.Feb 24 2018, 1:01 PM
bd808 added a subscriber: bd808.Feb 24 2018, 4:40 PM

https://logstash.wikimedia.org/goto/5a45a4cc40916467a0019cec5c6b1edc

[WouyEQpAAD8AAFI8KFwAAAAY] [no req]   BadMethodCallException from line 176 of /srv/mediawiki/php-1.31.0-wmf.21/extensions/CentralAuth/includes/LocalRenameJob/LocalRenameUserJob.php: Call to a member function getNamespace() on a non-object (null)
#0 /srv/mediawiki/php-1.31.0-wmf.21/extensions/CentralAuth/includes/LocalRenameJob/LocalRenameUserJob.php(105): LocalRenameUserJob->movePages(User)
#1 /srv/mediawiki/php-1.31.0-wmf.21/extensions/CentralAuth/includes/LocalRenameJob/LocalRenameJob.php(63): LocalRenameUserJob->doRun(string)
#2 /srv/mediawiki/php-1.31.0-wmf.21/includes/jobqueue/JobRunner.php(294): LocalRenameJob->run()
#3 /srv/mediawiki/php-1.31.0-wmf.21/includes/jobqueue/JobRunner.php(193): JobRunner->executeJob(LocalRenameUserJob, Wikimedia\Rdbms\LBFactoryMulti, BufferingStatsdDataFactory, integer)
#4 /srv/mediawiki/rpc/RunJobs.php(47): JobRunner->run(array)
#5 {main}
bd808 added a comment.Feb 24 2018, 5:57 PM

The failure here is caused by the $ in the username not being handled correctly in a preg_replace call in LocalRenameUserJob.

$ mwscript eval.php --wiki=enwiki
> $to = 'Drytime%$1600';
> $toTitle = Title::makeTitleSafe( NS_USER, $to );
> echo $toTitle->getDBkey();
Drytime%$1600
> echo preg_replace( '!^[^/]+!', $toTitle->getDBkey(), 'SimonFoundationContinence' );
Drytime%00
> $newPage = Title::makeTitleSafe( NS_USER, 'Drytime%00' );

> var_dump($newPage);
NULL
> MediaWiki\MediaWikiServices::getInstance()->getTitleParser()->parseTitle( 'Drytime%00', NS_USER );
Caught exception MalformedTitleException: The requested page title contains invalid characters: "%00".
#0 /srv/mediawiki/php-1.31.0-wmf.22/includes/title/MediaWikiTitleCodec.php(156): MediaWikiTitleCodec->splitTitleString('Drytime%00', 2)
#1 /srv/mediawiki/php-1.31.0-wmf.22/maintenance/eval.php(78) : eval()'d code(1): MediaWikiTitleCodec->parseTitle('Drytime%00', 2)
#2 /srv/mediawiki/php-1.31.0-wmf.22/maintenance/eval.php(78): eval()
#3 /srv/mediawiki/multiversion/MWScript.php(100): require_once('/srv/mediawiki/...')
#4 {main}

The problem is that $16 is treated as a backreference in preg_replace.

replacement may contain references of the form \\n or $n, with the latter form being the preferred one. Every such reference will be replaced by the text captured by the n'th parenthesized pattern. n can be from 0 to 99, and \\0 or $0 refers to the text matched by the whole pattern. Opening parentheses are counted from left to right (starting from 1) to obtain the number of the capturing subpattern. To use backslash in replacement, it must be doubled ("\\\\" PHP string). — https://secure.php.net/preg_replace

Escaping the $ when used in preg_replace should fix the problem:

php > echo preg_replace( '!^[^/]+!', 'Drytime%\$1600', 'SimonFoundationContinence' );
Drytime%$1600

Change 414117 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[mediawiki/extensions/CentralAuth@master] LocalRenameUserJob: escape '$' in replacement title

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

Framawiki renamed this task from Please unblock stuck global rename of SimonFoundationContinence to Drytime%$1600 to Escape '$' in replacement title.Feb 24 2018, 6:09 PM
Framawiki renamed this task from Escape '$' in replacement title to LocalRenameUserJob: escape '$' in replacement title.

So once that fix is merged (and maybe backported) can we run the fixStuckGlobalRename.php script safely so the rename is performed? Thanks.

Melos added a subscriber: Melos.Feb 25 2018, 3:56 PM

A bit of discussion led to noticing that we also need to escape \ (e.g. https://test.wikipedia.org/wiki/Special:CentralAuth/VeryU%5CniqueAccount)

Change 414117 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] LocalRenameUserJob: escape backreferences in replacement title

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

Can we cherry-pick this to the current wmf branch? If we wait for the train we won't be able to do this until all group0,1,2 wikis are using it; unless that's not a problem? Thanks.

Change 414972 had a related patch set uploaded (by MarcoAurelio; owner: Bryan Davis):
[mediawiki/extensions/CentralAuth@wmf/1.31.0-wmf.22] LocalRenameUserJob: escape backreferences in replacement title

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

Change 414972 abandoned by MarcoAurelio:
LocalRenameUserJob: escape backreferences in replacement title

Reason:
I abandon this request.

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

Change 414972 restored by MarcoAurelio:
LocalRenameUserJob: escape backreferences in replacement title

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

Change 414972 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@wmf/1.31.0-wmf.22] LocalRenameUserJob: escape backreferences in replacement title

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

Mentioned in SAL (#wikimedia-operations) [2018-02-28T00:03:00Z] <thcipriani@tin> Synchronized php-1.31.0-wmf.22/extensions/CentralAuth/includes/LocalRenameJob/LocalRenameUserJob.php: [[gerrit:414972|LocalRenameUserJob: escape backreferences in replacement title]] T188171 (duration: 01m 13s)

MarcoAurelio added a subscriber: thcipriani.EditedFeb 28 2018, 12:21 AM

The cherry-pick from above has been merged and @thcipriani deployed it and ran mwscript extensions/CentralAuth/maintenance/fixStuckGlobalRename.php --wiki=enwiki --logwiki=metawiki 'SimonFoundationContinence' 'Drytime%$1600' and the rename worked. However https://meta.wikimedia.org/wiki/Special:CentralAuth/Drytime%$1600 gives Error 400

400 Bad Request
nginx/1.13.6

I don't know what Logstash might have kept for the error as I don't have access to it. Either what it causes the error there or we revert the rename and blacklist those weird characters. Your call :)

The CentralAuth link works with &target= parameter though. See https://meta.wikimedia.org/w/index.php?title=Special%3ACentralAuth&target=Drytime%25%241600

If the $ and % characters are converted, the page https://meta.wikimedia.org/wiki/Special:CentralAuth/Drytime%25%241600 also works well.

As you can see, the accounts have been left unnatached after the rename. @Legoktm Could you please look into this?

Thanks.

bd808 added a comment.Feb 28 2018, 1:04 AM

As you can see, the accounts have been left unnatached after the rename. @Legoktm Could you please look into this?

These accounts are actually attached, but their lu_attached_method is not set so they look weird in the list. I fixed this by running:

wikiadmin@db1062(centralauth)>update localuser set lu_attached_method='login' where lu_name = 'Drytime%$1600' and lu_attached_method is null;
Query OK, 3 rows affected (0.01 sec)
Rows matched: 3  Changed: 3  Warnings: 0
MarcoAurelio closed this task as Resolved.Feb 28 2018, 11:12 AM
MarcoAurelio claimed this task.

I'll close this task as resolved as the issue with those characters is solved. I'll report the URL issues in another ticket.

Restricted Application added a project: User-MarcoAurelio. · View Herald TranscriptFeb 28 2018, 11:12 AM
MarcoAurelio moved this task from Backlog to Closed on the GlobalRename board.Sep 1 2018, 12:16 PM