Use a WikiPage object when filtering edits on a non-existing Title

Remove $title->exists() from the check, so we have the following
changes:
 - The AbuseLog will add a diff link for page creations
 - Searching the AbuseLog for impact:saved will include page creations
 - We don't have to recreate the WikiPage again in RunVariableGenerator

Also remove an old reference to "bug 31656": that comment was added in
rEABFefecf8b2441ae2f31f924ff33103f5affe5d1d62, which changed
Article::getContent() to Article::getRevision()->getRawText(). Nowadays
we don't even use Article anymore, and that conditional isn't even for
retrieving the page content, so the comment is wrong.

Add logging for when the Title object cannot exist, as this should never
happen in the context of the EditFilterMergedContent hook, and always
create a WikiPage. Some signatures were changed to require a WikiPage
object now, and every caller updated to provide it.

Bug: T263104
Bug: T62179
Depends-On: Ic238eaa529ef6bfba06b4dd03924a8e0111d8259
Change-Id: Ibf3bf4f68328ba4a5616ab8f26a8b44d27a25cd7
This commit is contained in:
Daimona Eaytoy 2020-09-17 13:54:48 +02:00 committed by Jforrester
parent e7813fbafb
commit 6305746de3
4 changed files with 19 additions and 25 deletions

View file

@ -166,20 +166,21 @@ class AbuseFilterHooks {
$text = AbuseFilter::contentToString( $content );
$title = $context->getTitle();
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
if ( $title === null ) {
// T144265: This *should* never happen.
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
$logger->warning( __METHOD__ . ' received a null title.' );
return Status::newGood();
}
if ( $title->canExist() && $title->exists() ) {
// Make sure we load the latest text saved in database (bug 31656)
$page = $context->getWikiPage();
} else {
$page = null;
if ( !$title->canExist() ) {
// This also should be handled in EditPage or whoever is calling the hook.
$logger->warning( __METHOD__ . ' received a Title that cannot exist.' );
// Note that if the title cannot exist, there's no much point in filtering the edit anyway
return Status::newGood();
}
$page = $context->getWikiPage();
$vars = new AbuseFilterVariableHolder();
$builder = new RunVariableGenerator( $vars, $user, $title );
$vars = $builder->getEditVars( $content, $text, $summary, $slot, $page );

View file

@ -210,7 +210,7 @@ class RCVariableGenerator extends VariableGenerator {
$this->vars->setVar( 'old_wikitext', '' );
}
$this->addEditVars( $title );
$this->addEditVars( $title, \WikiPage::factory( $title ) );
return $this;
}

View file

@ -110,7 +110,7 @@ class RunVariableGenerator extends VariableGenerator {
}
/**
* @param WikiPage|null $page
* @param WikiPage $page
* @param string $summary
* @param Content $newcontent
* @param string $text
@ -120,7 +120,7 @@ class RunVariableGenerator extends VariableGenerator {
* @throws MWException
*/
private function newVariableHolderForEdit(
?WikiPage $page,
WikiPage $page,
string $summary,
Content $newcontent,
string $text,
@ -153,7 +153,7 @@ class RunVariableGenerator extends VariableGenerator {
* @param string $text
* @param string $summary
* @param string $slot
* @param WikiPage|null $page
* @param WikiPage $page
* @return AbuseFilterVariableHolder|null
*/
public function getEditVars(
@ -161,17 +161,17 @@ class RunVariableGenerator extends VariableGenerator {
string $text,
string $summary,
$slot,
WikiPage $page = null
WikiPage $page
) : ?AbuseFilterVariableHolder {
$oldContent = null;
if ( $page !== null ) {
if ( $this->title->exists() ) {
$filterText = $this->getEditTextForFiltering( $page, $content, $slot );
if ( $filterText === null ) {
return null;
}
list( $oldContent, $oldAfText, $text ) = $filterText;
} else {
// Optimization
$oldContent = null;
$oldAfText = '';
}
@ -262,8 +262,8 @@ class RunVariableGenerator extends VariableGenerator {
// We only have the upload comment and page text when using the UploadVerifyUpload hook
if ( $summary !== null && $text !== null ) {
// This block is adapted from self::getTextForFiltering()
$page = WikiPage::factory( $this->title );
if ( $this->title->exists() ) {
$page = WikiPage::factory( $this->title );
$revRec = $page->getRevisionRecord();
if ( !$revRec ) {
return null;
@ -279,7 +279,6 @@ class RunVariableGenerator extends VariableGenerator {
// Page text is ignored for uploads when the page already exists
$text = $oldtext;
} else {
$page = null;
$oldtext = '';
}

View file

@ -161,16 +161,10 @@ class VariableGenerator {
/**
* @param Title $title
* @param WikiPage|null $page
* @param WikiPage $page
* @return $this For chaining
*/
public function addEditVars( Title $title, WikiPage $page = null ) : self {
// NOTE: $page may end up remaining null, e.g. if $title points to a special page.
if ( !$page && $title->canExist() ) {
// TODO: The caller should do this!
$page = WikiPage::factory( $title );
}
public function addEditVars( Title $title, WikiPage $page ) : self {
$this->vars->setLazyLoadVar( 'edit_diff', 'diff-array',
[ 'oldtext-var' => 'old_wikitext', 'newtext-var' => 'new_wikitext' ] );
$this->vars->setLazyLoadVar( 'edit_diff_pst', 'diff-array',