Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LinkBatch: skip bad input

Browse source 
LinkBatch used to be lenient about receiving null or invalid titles.
This patch restores this lenient behavior.

Bug: T282180
Bug: T282070
Change-Id: I2c6378a3a0d508c77bcb290a6ed07f4d5f96d62c
This commit is contained in:
daniel 2021-05-07 12:37:46 +02:00
parent cd8d97368f
commit 316053ed88
2 changed files with 84 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
includes/cache/LinkBatch.php vendored
View file

@ -23,6 +23,7 @@
use MediaWiki\Cache\CacheKeyHelper;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
use MediaWiki\Page\PageIdentity;
use MediaWiki\Page\PageIdentityValue;
@ -130,6 +131,16 @@ class LinkBatch {
* @param LinkTarget|PageReference $link
*/
public function addObj( $link ) {
if ( !$link ) {
// Don't die if we got null, just skip. There is nothing to do anyway.
// For now, let's avoid things like T282180. We should be more strict in the future.
LoggerFactory::getInstance( 'LinkBatch' )->warning(
'Skipping null link, probably due to a bad title.',
[ 'trace' => wfBacktrace( true ) ]
);
return;
}
Assert::parameterType( [ LinkTarget::class, PageReference::class ], $link, '$link' );
$this->add( $link->getNamespace(), $link->getDBkey() );
}
@ -240,8 +251,9 @@ class LinkBatch {
$ids = [];
$remaining = $this->data;
foreach ( $res as $row ) {
$title = TitleValue::tryNew( (int)$row->page_namespace, $row->page_title );
if ( $title ) {
try {
$title = new TitleValue( (int)$row->page_namespace, $row->page_title );
$cache->addGoodLinkObjFromRow( $title, $row );
$pdbk = $this->titleFormatter->getPrefixedDBkey( $title );
$ids[$pdbk] = $row->page_id;
@ -255,9 +267,11 @@ class LinkBatch {
$key = CacheKeyHelper::getKeyForPage( $pageIdentity );
$this->pageIdentities[$key] = $pageIdentity;
} else {
wfLogWarning( __METHOD__ . ': encountered invalid title: ' .
$row->page_namespace . '-' . $row->page_title );
} catch ( InvalidArgumentException $ex ) {
LoggerFactory::getInstance( 'LinkBatch' )->warning(
'Encountered invalid title',
[ 'title_namespace' => $row->page_namespace, 'title_dbkey' => $row->page_title ]
);
}
unset( $remaining[$row->page_namespace][$row->page_title] );
@ -266,8 +280,9 @@ class LinkBatch {
// The remaining links in $data are bad links, register them as such
foreach ( $remaining as $ns => $dbkeys ) {
foreach ( $dbkeys as $dbkey => $unused ) {
$title = TitleValue::tryNew( (int)$ns, (string)$dbkey );
if ( $title ) {
try {
$title = new TitleValue( (int)$ns, (string)$dbkey );
$cache->addBadLinkObj( $title );
$pdbk = $this->titleFormatter->getPrefixedDBkey( $title );
$ids[$pdbk] = 0;
@ -275,8 +290,11 @@ class LinkBatch {
$pageIdentity = new PageIdentityValue( 0, (int)$ns, $dbkey, PageIdentity::LOCAL );
$key = CacheKeyHelper::getKeyForPage( $pageIdentity );
$this->pageIdentities[$key] = $pageIdentity;
} else {
wfLogWarning( __METHOD__ . ': encountered invalid title: ' . $ns . '-' . $dbkey );
} catch ( InvalidArgumentException $ex ) {
LoggerFactory::getInstance( 'LinkBatch' )->warning(
'Encountered invalid title',
[ 'title_namespace' => $ns, 'title_dbkey' => $dbkey ]
);
}
}
}

72
tests/phpunit/includes/cache/LinkBatchTest.php vendored
View file

@ -1,6 +1,7 @@
<?php
use MediaWiki\Cache\CacheKeyHelper;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Page\PageReference;
use MediaWiki\Page\PageReferenceValue;
use Wikimedia\AtEase\AtEase;
@ -82,21 +83,33 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
}
/**
* @covers LinkBatch::addObj()
* @covers LinkBatch::getSize()
* @param iterable<LinkTarget>|iterable<PageReference> $objects
*
* @return LinkBatch
* @throws Exception
*/
public function testAddObj() {
$batch = new LinkBatch(
[
new TitleValue( NS_MAIN, 'Foo' ),
new PageReferenceValue( NS_USER, 'Foo', PageReference::LOCAL ),
],
private function newLinkBatch( $objects = [] ) {
return new LinkBatch(
$objects,
$this->createMock( LinkCache::class ),
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->getServiceContainer()->getDBLoadBalancer()
);
}
/**
* @covers LinkBatch::addObj()
* @covers LinkBatch::getSize()
*/
public function testAddObj() {
$batch = $this->newLinkBatch(
[
new TitleValue( NS_MAIN, 'Foo' ),
new PageReferenceValue( NS_USER, 'Foo', PageReference::LOCAL ),
]
);
$batch->addObj( new PageReferenceValue( NS_TALK, 'Bar', PageReference::LOCAL ) );
$batch->addObj( new TitleValue( NS_MAIN, 'Foo' ) );
@ -110,15 +123,10 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
* @covers LinkBatch::getSize()
*/
public function testAdd() {
$batch = new LinkBatch(
$batch = $this->newLinkBatch(
[
new TitleValue( NS_MAIN, 'Foo' )
],
$this->createMock( LinkCache::class ),
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->getServiceContainer()->getDBLoadBalancer()
]
);
$batch->add( NS_TALK, 'Bar' );
@ -254,4 +262,38 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->assertTrue( $batch->doGenderQuery() );
}
public function provideBadObjects() {
yield 'null' => [ null ];
yield 'empty' => [ Title::makeTitle( NS_MAIN, '' ) ];
yield 'bad user' => [ Title::makeTitle( NS_USER, '#12345' ) ];
yield 'section' => [ new TitleValue( NS_MAIN, '', '#See_also' ) ];
yield 'special' => [ new TitleValue( NS_SPECIAL, 'RecentChanges' ) ];
}
/**
* @dataProvider provideBadObjects
*/
public function testAddBadObj( $obj ) {
$linkBatch = $this->newLinkBatch();
$linkBatch->addObj( $obj );
$linkBatch->execute();
$this->addToAssertionCount( 1 );
}
public function provideBadDBKeys() {
yield 'empty' => [ '' ];
yield 'section' => [ '#See_also' ];
yield 'pipe' => [ 'foo|bar' ];
}
/**
* @dataProvider provideBadDBKeys
*/
public function testAddBadDBKeys( $key ) {
$linkBatch = $this->newLinkBatch();
$linkBatch->add( NS_MAIN, $key );
$linkBatch->execute();
$this->addToAssertionCount( 1 );
}
}