Page MenuHomePhabricator

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

Description 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.


#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:

Event Timeline

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.

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

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

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

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

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.

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""

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

Duplicate of I7fe8bbed3fb24240e47ba22de57da9dc8a9bea22

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

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

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

matmarex assigned this task to Prtksxna.
matmarex removed a project: Patch-For-Review.