Page MenuHomePhabricator

Title::getContentModel() et al. return incorrect results for objects loaded from incomplete rows
Closed, ResolvedPublic

Description

In the context of a proposed fix for T86612, Anomie mentioned the following:

This analysis is not correct. LinkCache::addGoodLinkObjFromRow is never called from ApiPageSet. The real bug is that Title::getContentModel() assumes that if $this->mContentModel is falsey then $this->getArticleID() is going to add the title to the link cache. But that doesn't happen (without GAID_FOR_UPDATE, which we don't need here) when $this->mArticleID is already initialized.
In other words, the real fix is that Title::getContentModel() needs to call $linkCache->addLinkObj( $this ) itself before calling $linkCache->getGoodLinkFieldObj( $this, 'model' ), since it can't rely on $this->getArticleID() having done it.

This means code like the following (referring to pages on my test wiki) may return the wrong result:

> var_dump(Title::newFromRow((object)['page_id'=>638,'page_namespace'=>'0','page_title'=>'XD'])->getContentModel()); // should be "text"
string(8) "wikitext"

when the content model is actually something else like "text". Title::isRedirect() and Title::getLength() appear to have the same issue:

> var_dump(Title::newFromRow((object)['page_id'=>7,'page_namespace'=>'0','page_title'=>'Abc'])->isRedirect()); // should be true
bool(false)

> var_dump(Title::newFromRow((object)['page_id'=>7,'page_namespace'=>'0','page_title'=>'Abc'])->getLength()); // should be 37
int(0)

This issue also exists when Title::resetArticleID() is used to clear the LinkCache entry for the title and an article ID is provided. It is related to T71789, for which the fix was thus incomplete.

Event Timeline

PleaseStand raised the priority of this task from to Needs Triage.
PleaseStand updated the task description. (Show Details)
PleaseStand added a project: MediaWiki-General.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 15 2015, 10:02 PM
PleaseStand updated the task description. (Show Details)Jan 15 2015, 11:04 PM
PleaseStand set Security to None.
gerritbot added a subscriber: gerritbot.

Change 185348 had a related patch set uploaded (by PleaseStand):
Title: Always add title to LinkCache (in 3 methods)

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

Patch-For-Review

Aklapper triaged this task as Medium priority.Jan 16 2015, 1:45 PM

Change 185348 merged by jenkins-bot:
Title: Always add title to LinkCache when necessary (in 3 methods)

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

Anomie closed this task as Resolved.Jan 23 2015, 10:13 PM
Anomie assigned this task to PleaseStand.

This should be deployed to WMF wikis with 1.25wmf16, see https://www.mediawiki.org/wiki/MediaWiki_1.25/Roadmap for the schedule.

Change 187156 had a related patch set uploaded (by EBernhardson):
Title: Always add title to LinkCache when necessary (in 3 methods)

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

Patch-For-Review

Change 187158 had a related patch set uploaded (by EBernhardson):
Title: Always add title to LinkCache when necessary (in 3 methods)

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

Patch-For-Review

Moving forward deployment of this patch to jan 28 afternoon (PST) swat deploy.

Change 187158 merged by jenkins-bot:
Title: Always add title to LinkCache when necessary (in 3 methods)

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

Change 187156 merged by jenkins-bot:
Title: Always add title to LinkCache when necessary (in 3 methods)

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