Page MenuHomePhabricator
Create Task
Maniphest T199763

Special:Log UnexpectedValueException after changes in HTMLTitleTextField and MapCacheLRU
Closed, ResolvedPublic
Actions

Description

https://en.wikipedia.beta.wmflabs.org/wiki/Special:Log is broken:

[W01XLwpEEj4AAA7ee@EAAAAF] /wiki/Special:Log UnexpectedValueException from line 144 of /srv/mediawiki/php-master/includes/libs/MapCacheLRU.php: MapCacheLRU::has called with invalid key. Must be string or integer.

Backtrace:

#0 /srv/mediawiki/php-master/includes/libs/MapCacheLRU.php(163): MapCacheLRU->has(NULL)
#1 /srv/mediawiki/php-master/includes/Title.php(318): MapCacheLRU->get(NULL)
#2 /srv/mediawiki/php-master/includes/htmlform/fields/HTMLTitleTextField.php(51): Title::newFromTextThrow(NULL)
#3 /srv/mediawiki/php-master/includes/htmlform/HTMLFormField.php(862): HTMLTitleTextField->validate(NULL, array)
#4 /srv/mediawiki/php-master/includes/htmlform/HTMLFormField.php(476): HTMLFormField->getErrorsAndErrorClass(NULL)
#5 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(1668): HTMLFormField->getTableRow(NULL)
#6 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(1250): HTMLForm->displaySection(array, string)
#7 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(1040): HTMLForm->getBody()
#8 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(1020): HTMLForm->getHTML(boolean)
#9 /srv/mediawiki/php-master/includes/logging/LogEventsList.php(168): HTMLForm->displayForm(boolean)
#10 /srv/mediawiki/php-master/includes/specials/SpecialLog.php(255): LogEventsList->showOptions(array, string, string, boolean, NULL, NULL, NULL, array, string, string)
#11 /srv/mediawiki/php-master/includes/specials/SpecialLog.php(147): SpecialLog->show(FormOptions, array)
#12 /srv/mediawiki/php-master/includes/specialpage/SpecialPage.php(566): SpecialLog->execute(NULL)
#13 /srv/mediawiki/php-master/includes/specialpage/SpecialPageFactory.php(569): SpecialPage->run(NULL)
#14 /srv/mediawiki/php-master/includes/MediaWiki.php(288): SpecialPageFactory::executePath(Title, RequestContext)
#15 /srv/mediawiki/php-master/includes/MediaWiki.php(867): MediaWiki->performRequest()
#16 /srv/mediawiki/php-master/includes/MediaWiki.php(524): MediaWiki->main()
#17 /srv/mediawiki/php-master/index.php(42): MediaWiki->run()
#18 /srv/mediawiki/w/index.php(3): include(string)
#19 {main}

Recent changes in HTMLTitleTextField and MapCacheLRU.

Other pages affected:

Details

Related Changes in Gerrit:
SubjectAuthorRepoBranchLines +/-
Convert Title::getTitleCache() to using MapCacheLRU (again)Aaron Schulzmediawiki/coremaster+0 -0
HTMLTitleTextField, HTMLUserTextField: Check for `null` before using the valuePrtksxnamediawiki/coremaster+11 -1
Revert "Revert "Convert Title::getTitleCache() to using MapCacheLRU""Bartosz Dziewońskimediawiki/coremaster+0 -0
Revert "Convert Title::getTitleCache() to using MapCacheLRU"Krinklemediawiki/coremaster+3 -3
Customize query in gerrit

Related Objects

Event Timeline

• Prtksxna created this task.Jul 17 2018, 2:59 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 17 2018, 2:59 AM
• Prtksxna updated the task description. (Show Details)Jul 17 2018, 3:16 AM
• Prtksxna added a comment.EditedJul 17 2018, 4:25 AM

git bisect points to rMW7a25cd388c8a: Convert Title::getTitleCache() to using MapCacheLRU.

• Prtksxna updated the task description. (Show Details)Jul 17 2018, 4:37 AM
Krinkle added a comment.EditedJul 17 2018, 4:45 AM

The commit in question (correctly) enforces a restriction that was previously implied. The restriction being that: cache keys cannot be null. Many code paths were already enforcing this in their own code, but it seems we found one method used on Special:Log that wasn't accounting for this.

As such, it causes the exception to be thrown. We can temporarily revert that, but ultimately the commit can't be improved or done differently.

The HTMLTitleTextField class is passing null as a title to validate() which goes unfiltered and makes its way to Title::newFromTextThrow. That method happens to work by accident, because it is interpreting null as "Null" and treating that as if it was an article page name (e.g. [[Null]]), and says it is "valid".

That code is broken and needs to be fixed.

To confirm this I have restored my MediaWiki install to before f88a5a18155a (git checkout -b test 33912421), and confirmed Special:Log still works, and then cherry-picked 7a25cd388c8a (git cherry-pick 7a25cd388c8a), and confirmed Special:Log still works.

gerritbot subscribed.Jul 17 2018, 4:56 AM

Change 446216 had a related patch set uploaded (by Prtksxna; owner: Prtksxna):
[mediawiki/core@master] HTMLTitleTextField: Also check is_null when checking if value is empty

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

gerritbot added a project: Patch-For-Review.Jul 17 2018, 4:56 AM
• Prtksxna mentioned this in T117737: Convert Special:Log to OOUI.Jul 17 2018, 5:03 AM
• Prtksxna updated the task description. (Show Details)Jul 17 2018, 5:09 AM
Krinkle added a parent task: T191059: 1.32.0-wmf.13 deployment blockers.Jul 17 2018, 5:40 AM
gerritbot added a comment.Jul 17 2018, 7:02 AM

Change 446228 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Revert "Convert Title::getTitleCache() to using MapCacheLRU"

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

gerritbot added a comment.Jul 17 2018, 7:15 AM

Change 446228 merged by jenkins-bot:
[mediawiki/core@master] Revert "Convert Title::getTitleCache() to using MapCacheLRU"

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-07-17 (1.32.0-wmf.13)).Jul 17 2018, 8:00 AM
zeljkofilipin subscribed.Jul 17 2018, 9:46 AM

Special:Log works for me, 446228 is merged, can this be resolved?

• Prtksxna added a comment.Jul 17 2018, 9:50 AM

To fully fix the issue, we need to fix HTMLTitleTextField (https://gerrit.wikimedia.org/r/#/c/446216/) and then revert the revert of rMW7a25cd388c8a: Convert Title::getTitleCache() to using MapCacheLRU.

zeljkofilipin added a comment.Jul 17 2018, 10:21 AM

Is it still blocking T191059: 1.32.0-wmf.13 deployment blockers?

matmarex added a comment.Jul 17 2018, 2:54 PM

I think it is not blocking the deployment any more, we need to do some more work to clean up the tech debt, but master is in a stable state right now.

gerritbot added a comment.Jul 17 2018, 11:51 PM

Change 446510 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Revert "Revert "Convert Title::getTitleCache() to using MapCacheLRU""

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

Krinkle removed a parent task: T191059: 1.32.0-wmf.13 deployment blockers.Jul 17 2018, 11:51 PM
gerritbot added a comment.Jul 18 2018, 5:54 PM

Change 446510 abandoned by Bartosz Dziewoński:
Revert "Revert "Convert Title::getTitleCache() to using MapCacheLRU""

Reason:
Duplicate of I7fe8bb

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

gerritbot added a comment.Jul 18 2018, 5:54 PM

Change 446240 had a related patch set uploaded (by Krinkle; owner: Aaron Schulz):
[mediawiki/core@master] [DNM] Convert Title::getTitleCache() to using MapCacheLRU (again)

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

gerritbot added a comment.Jul 18 2018, 6:10 PM

Change 446216 merged by jenkins-bot:
[mediawiki/core@master] HTMLTitleTextField, HTMLUserTextField: Check for null before using the value

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

gerritbot added a comment.Jul 19 2018, 6:20 PM

Change 446240 merged by jenkins-bot:
[mediawiki/core@master] Convert Title::getTitleCache() to using MapCacheLRU (again)

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

matmarex closed this task as Resolved.Jul 19 2018, 6:39 PM
matmarex assigned this task to • Prtksxna.
matmarex removed a project: Patch-For-Review.
ReleaseTaggerBot edited projects, added: MW-1.32-notes (WMF-deploy-2018-07-24 (1.32.0-wmf.14)); removed: MW-1.32-notes (WMF-deploy-2018-07-17 (1.32.0-wmf.13)).Jul 19 2018, 8:01 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits