From: daniel Date: Mon, 3 Jun 2019 22:56:22 +0000 (+0200) Subject: Ensure canHaveTalkPage returns false when getTalkPage would fail. X-Git-Tag: 1.34.0-rc.0~1184^2 X-Git-Url: http://git.cyclocoop.org/url?a=commitdiff_plain;h=dbce648a15ee7100383c3ec9781775f0f895c645;p=lhc%2Fweb%2Fwiklou.git Ensure canHaveTalkPage returns false when getTalkPage would fail. This causes Title::getTalkPage and NamespaceInfo::getTitle() to throw an MWException when called on a LinkTarget that is an interwiki link or a relative section link. These methods were already throwing MWException when called on a link to a Special page. Bug: T224814 Change-Id: I525c186a5b8b8fc22bca195da48afead3bfbd402 --- diff --git a/includes/Title.php b/includes/Title.php index b27baa8db1..6e75102c92 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1145,14 +1145,16 @@ class Title implements LinkTarget, IDBAccessObject { /** * Can this title have a corresponding talk page? * - * @see NamespaceInfo::hasTalkNamespace + * False for relative section links (with getText() === ''), + * interwiki links (with getInterwiki() !== ''), and pages in NS_SPECIAL. + * + * @see NamespaceInfo::canHaveTalkPage * @since 1.30 * * @return bool True if this title either is a talk page or can have a talk page associated. */ public function canHaveTalkPage() { - return MediaWikiServices::getInstance()->getNamespaceInfo()-> - hasTalkNamespace( $this->mNamespace ); + return MediaWikiServices::getInstance()->getNamespaceInfo()->canHaveTalkPage( $this ); } /** @@ -1167,11 +1169,15 @@ class Title implements LinkTarget, IDBAccessObject { /** * Can this title be added to a user's watchlist? * + * False for relative section links (with getText() === ''), + * interwiki links (with getInterwiki() !== ''), and pages in NS_SPECIAL. + * * @return bool */ public function isWatchable() { - return !$this->isExternal() && MediaWikiServices::getInstance()->getNamespaceInfo()-> - isWatchable( $this->mNamespace ); + $nsInfo = MediaWikiServices::getInstance()->getNamespaceInfo(); + return $this->getText() !== '' && !$this->isExternal() && + $nsInfo->isWatchable( $this->mNamespace ); } /** @@ -1532,8 +1538,11 @@ class Title implements LinkTarget, IDBAccessObject { /** * Get a Title object associated with the talk page of this article * - * @deprecated since 1.34, use NamespaceInfo::getTalkPage + * @deprecated since 1.34, use getTalkPageIfDefined() or NamespaceInfo::getTalkPage() + * with NamespaceInfo::canHaveTalkPage(). * @return Title The object for the talk page + * @throws MWException if $target doesn't have talk pages, e.g. because it's in NS_SPECIAL + * or because it's a relative link, or an interwiki link. */ public function getTalkPage() { return self::castFromLinkTarget( diff --git a/includes/title/NamespaceInfo.php b/includes/title/NamespaceInfo.php index cdb8f2554f..2ed8729ac6 100644 --- a/includes/title/NamespaceInfo.php +++ b/includes/title/NamespaceInfo.php @@ -142,6 +142,8 @@ class NamespaceInfo { * * @param int $index Namespace index * @return int + * @throws MWException if the given namespace doesn't have an associated talk namespace + * (e.g. NS_SPECIAL). */ public function getTalk( $index ) { $this->isMethodValidFor( $index, __METHOD__ ); @@ -151,15 +153,52 @@ class NamespaceInfo { } /** + * Get a LinkTarget referring to the talk page of $target. + * + * @see canHaveTalkPage * @param LinkTarget $target * @return LinkTarget Talk page for $target - * @throws MWException if $target's namespace doesn't have talk pages (e.g., NS_SPECIAL) + * @throws MWException if $target doesn't have talk pages, e.g. because it's in NS_SPECIAL, + * because it's a relative section-only link, or it's an an interwiki link. */ public function getTalkPage( LinkTarget $target ) : LinkTarget { + if ( $target->getText() === '' ) { + throw new MWException( 'Can\'t determine talk page associated with relative section link' ); + } + + if ( $target->getInterwiki() !== '' ) { + throw new MWException( 'Can\'t determine talk page associated with interwiki link' ); + } + if ( $this->isTalk( $target->getNamespace() ) ) { return $target; } - return new TitleValue( $this->getTalk( $target->getNamespace() ), $target->getDbKey() ); + + // NOTE: getTalk throws on bad namespaces! + return new TitleValue( $this->getTalk( $target->getNamespace() ), $target->getDBkey() ); + } + + /** + * Can the title have a corresponding talk page? + * + * False for relative section-only links (with getText() === ''), + * interwiki links (with getInterwiki() !== ''), and pages in NS_SPECIAL. + * + * @see getTalkPage + * + * @param LinkTarget $target + * @return bool True if this title either is a talk page or can have a talk page associated. + */ + public function canHaveTalkPage( LinkTarget $target ) { + if ( $target->getText() === '' || $target->getInterwiki() !== '' ) { + return false; + } + + if ( $target->getNamespace() < NS_MAIN ) { + return false; + } + + return true; } /** @@ -188,7 +227,7 @@ class NamespaceInfo { if ( $this->isSubject( $target->getNamespace() ) ) { return $target; } - return new TitleValue( $this->getSubject( $target->getNamespace() ), $target->getDbKey() ); + return new TitleValue( $this->getSubject( $target->getNamespace() ), $target->getDBkey() ); } /** @@ -216,8 +255,16 @@ class NamespaceInfo { * @throws MWException if $target's namespace doesn't have talk pages (e.g., NS_SPECIAL) */ public function getAssociatedPage( LinkTarget $target ) : LinkTarget { + if ( $target->getText() === '' ) { + throw new MWException( 'Can\'t determine talk page associated with relative section link' ); + } + + if ( $target->getInterwiki() !== '' ) { + throw new MWException( 'Can\'t determine talk page associated with interwiki link' ); + } + return new TitleValue( - $this->getAssociated( $target->getNamespace() ), $target->getDbKey() ); + $this->getAssociated( $target->getNamespace() ), $target->getDBkey() ); } /** diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index 9b021c40ee..4ffef02d19 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -800,6 +800,65 @@ class TitleTest extends MediaWikiTestCase { 'Virtual namespace cannot have talk page' => [ Title::makeTitle( NS_MEDIA, 'Kitten.jpg' ), false ], + 'Relative link has no talk page' => [ + Title::makeTitle( NS_MAIN, '', 'Kittens' ), false + ], + 'Interwiki link has no talk page' => [ + Title::makeTitle( NS_MAIN, 'Kittens', '', 'acme' ), false + ], + ]; + } + + public function provideIsWatchable() { + return [ + 'User page is watchable' => [ + Title::makeTitle( NS_USER, 'Jane' ), true + ], + 'Talke page is watchable' => [ + Title::makeTitle( NS_TALK, 'Foo' ), true + ], + 'Special page is not watchable' => [ + Title::makeTitle( NS_SPECIAL, 'Thing' ), false + ], + 'Virtual namespace is not watchable' => [ + Title::makeTitle( NS_MEDIA, 'Kitten.jpg' ), false + ], + 'Relative link is not watchable' => [ + Title::makeTitle( NS_MAIN, '', 'Kittens' ), false + ], + 'Interwiki link is not watchable' => [ + Title::makeTitle( NS_MAIN, 'Kittens', '', 'acme' ), false + ], + ]; + } + + public static function provideGetTalkPage_good() { + return [ + [ Title::makeTitle( NS_MAIN, 'Test' ), Title::makeTitle( NS_TALK, 'Test' ) ], + [ Title::makeTitle( NS_TALK, 'Test' ), Title::makeTitle( NS_TALK, 'Test' ) ], + ]; + } + + public static function provideGetTalkPage_bad() { + return [ + [ Title::makeTitle( NS_SPECIAL, 'Test' ) ], + [ Title::makeTitle( NS_MEDIA, 'Test' ) ], + [ Title::makeTitle( NS_MAIN, '', 'Kittens' ) ], + [ Title::makeTitle( NS_MAIN, 'Kittens', '', 'acme' ) ], + ]; + } + + public static function provideGetSubjectPage_good() { + return [ + [ Title::makeTitle( NS_TALK, 'Test' ), Title::makeTitle( NS_MAIN, 'Test' ) ], + [ Title::makeTitle( NS_MAIN, 'Test' ), Title::makeTitle( NS_MAIN, 'Test' ) ], + ]; + } + + public static function provideGetOtherPage_good() { + return [ + [ Title::makeTitle( NS_MAIN, 'Test' ), Title::makeTitle( NS_TALK, 'Test' ) ], + [ Title::makeTitle( NS_TALK, 'Test' ), Title::makeTitle( NS_MAIN, 'Test' ) ], ]; } @@ -815,31 +874,44 @@ class TitleTest extends MediaWikiTestCase { $this->assertSame( $expected, $actual, $title->getPrefixedDBkey() ); } - public static function provideGetTalkPage_good() { - return [ - [ Title::makeTitle( NS_MAIN, 'Test' ), Title::makeTitle( NS_TALK, 'Test' ) ], - [ Title::makeTitle( NS_TALK, 'Test' ), Title::makeTitle( NS_TALK, 'Test' ) ], - ]; + /** + * @dataProvider provideIsWatchable + * @covers Title::isWatchable + * + * @param Title $title + * @param bool $expected + */ + public function testIsWatchable( Title $title, $expected ) { + $actual = $title->canHaveTalkPage(); + $this->assertSame( $expected, $actual, $title->getPrefixedDBkey() ); } /** * @dataProvider provideGetTalkPage_good * @covers Title::getTalkPageIfDefined */ - public function testGetTalkPageIfDefined_good( Title $title ) { - $talk = $title->getTalkPageIfDefined(); - $this->assertInstanceOf( - Title::class, - $talk, - $title->getPrefixedDBKey() - ); + public function testGetTalkPage_good( Title $title, Title $expected ) { + $actual = $title->getTalkPage(); + $this->assertTrue( $expected->equals( $actual ), $title->getPrefixedDBkey() ); } - public static function provideGetTalkPage_bad() { - return [ - [ Title::makeTitle( NS_SPECIAL, 'Test' ) ], - [ Title::makeTitle( NS_MEDIA, 'Test' ) ], - ]; + /** + * @dataProvider provideGetTalkPage_bad + * @covers Title::getTalkPageIfDefined + */ + public function testGetTalkPage_bad( Title $title ) { + $this->setExpectedException( MWException::class ); + $title->getTalkPage(); + } + + /** + * @dataProvider provideGetTalkPage_good + * @covers Title::getTalkPageIfDefined + */ + public function testGetTalkPageIfDefined_good( Title $title, Title $expected ) { + $actual = $title->getTalkPageIfDefined(); + $this->assertNotNull( $actual, $title->getPrefixedDBkey() ); + $this->assertTrue( $expected->equals( $actual ), $title->getPrefixedDBkey() ); } /** @@ -850,10 +922,37 @@ class TitleTest extends MediaWikiTestCase { $talk = $title->getTalkPageIfDefined(); $this->assertNull( $talk, - $title->getPrefixedDBKey() + $title->getPrefixedDBkey() ); } + /** + * @dataProvider provideGetSubjectPage_good + * @covers Title::getSubjectPage + */ + public function testGetSubjectPage_good( Title $title, Title $expected ) { + $actual = $title->getSubjectPage(); + $this->assertTrue( $expected->equals( $actual ), $title->getPrefixedDBkey() ); + } + + /** + * @dataProvider provideGetOtherPage_good + * @covers Title::getOtherPage + */ + public function testGetOtherPage_good( Title $title, Title $expected ) { + $actual = $title->getOtherPage(); + $this->assertTrue( $expected->equals( $actual ), $title->getPrefixedDBkey() ); + } + + /** + * @dataProvider provideGetTalkPage_bad + * @covers Title::getOtherPage + */ + public function testGetOtherPage_bad( Title $title ) { + $this->setExpectedException( MWException::class ); + $title->getOtherPage(); + } + public function provideCreateFragmentTitle() { return [ [ Title::makeTitle( NS_MAIN, 'Test' ), 'foo' ], diff --git a/tests/phpunit/includes/parser/CoreParserFunctionsTest.php b/tests/phpunit/includes/parser/CoreParserFunctionsTest.php index c630447751..ef2f2196fa 100644 --- a/tests/phpunit/includes/parser/CoreParserFunctionsTest.php +++ b/tests/phpunit/includes/parser/CoreParserFunctionsTest.php @@ -1,9 +1,11 @@ assertEquals( $msg, 'f', 'Works escaped' ); } + public function provideTalkpagename() { + yield [ 'Talk:Foo bar', 'foo_bar' ]; + yield [ 'Talk:Foo', ' foo ' ]; + yield [ 'Talk:Foo', 'Talk:Foo' ]; + yield [ 'User talk:Foo', 'User:foo' ]; + yield [ '', 'Special:Foo' ]; + yield [ '', '' ]; + yield [ '', ' ' ]; + yield [ '', '__' ]; + yield [ '', '#xyzzy' ]; + yield [ '', '#' ]; + yield [ '', ':' ]; + yield [ '', ':#' ]; + yield [ '', 'User:' ]; + yield [ '', 'User:#' ]; + } + + /** + * @dataProvider provideTalkpagename + */ + public function testTalkpagename( $expected, $title ) { + $parser = MediaWikiServices::getInstance()->getParser(); + + $this->assertSame( $expected, CoreParserFunctions::talkpagename( $parser, $title ) ); + } + + public function provideSubjectpagename() { + yield [ 'Foo bar', 'Talk:foo_bar' ]; + yield [ 'Foo', ' Talk:foo ' ]; + yield [ 'User:Foo', 'User talk:foo' ]; + yield [ 'Special:Foo', 'Special:Foo' ]; + yield [ '', '' ]; + yield [ '', ' ' ]; + yield [ '', '__' ]; + yield [ '', '#xyzzy' ]; + yield [ '', '#' ]; + yield [ '', ':' ]; + yield [ '', ':#' ]; + yield [ '', 'Talk:' ]; + yield [ '', 'User talk:#' ]; + yield [ '', 'User:#' ]; + } + + /** + * @dataProvider provideTalkpagename + */ + public function testSubjectpagename( $expected, $title ) { + $parser = MediaWikiServices::getInstance()->getParser(); + + $this->assertSame( $expected, CoreParserFunctions::talkpagename( $parser, $title ) ); + } + } diff --git a/tests/phpunit/includes/title/NamespaceInfoTest.php b/tests/phpunit/includes/title/NamespaceInfoTest.php index 7eb8fd5243..c1e258dac0 100644 --- a/tests/phpunit/includes/title/NamespaceInfoTest.php +++ b/tests/phpunit/includes/title/NamespaceInfoTest.php @@ -6,6 +6,7 @@ */ use MediaWiki\Config\ServiceOptions; +use MediaWiki\Linker\LinkTarget; class NamespaceInfoTest extends MediaWikiTestCase { /********************************************************************************************** @@ -688,73 +689,88 @@ class NamespaceInfoTest extends MediaWikiTestCase { /** * @dataProvider provideSpecialNamespaces - * @covers NamespaceInfo::getTalk - * @covers NamespaceInfo::getTalkPage + * @covers NamespaceInfo::getAssociated * @covers NamespaceInfo::isMethodValidFor * * @param int $ns */ - public function testGetTalkPage_special( $ns ) { - $this->setExpectedException( MWException::class, - "NamespaceInfo::getTalk does not make any sense for given namespace $ns" ); - $this->newObj()->getTalkPage( new TitleValue( $ns, 'A' ) ); + public function testGetAssociated_special( $ns ) { + $this->setExpectedException( + MWException::class, + "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" + ); + $this->newObj()->getAssociated( $ns ); + } + + public static function provideCanHaveTalkPage() { + return [ + [ new TitleValue( NS_MAIN, 'Test' ), true ], + [ new TitleValue( NS_TALK, 'Test' ), true ], + [ new TitleValue( NS_USER, 'Test' ), true ], + [ new TitleValue( NS_SPECIAL, 'Test' ), false ], + [ new TitleValue( NS_MEDIA, 'Test' ), false ], + [ new TitleValue( NS_MAIN, '', 'Kittens' ), false ], + [ new TitleValue( NS_MAIN, 'Kittens', '', 'acme' ), false ], + ]; } /** - * @dataProvider provideSpecialNamespaces - * @covers NamespaceInfo::getTalk - * @covers NamespaceInfo::getTalkPage - * @covers NamespaceInfo::isMethodValidFor - * @covers Title::getTalkPage - * - * @param int $ns + * @dataProvider provideCanHaveTalkPage + * @covers NamespaceInfo::canHaveTalkPage */ - public function testTitleGetTalkPage_special( $ns ) { - $this->setExpectedException( MWException::class, - "NamespaceInfo::getTalk does not make any sense for given namespace $ns" ); - Title::makeTitle( $ns, 'A' )->getTalkPage(); + public function testCanHaveTalkPage( LinkTarget $t, $expected ) { + $actual = $this->newObj()->canHaveTalkPage( $t ); + $this->assertEquals( $expected, $actual, $t->getDBkey() ); + } + + public static function provideGetTalkPage_good() { + return [ + [ new TitleValue( NS_MAIN, 'Test' ), new TitleValue( NS_TALK, 'Test' ) ], + [ new TitleValue( NS_TALK, 'Test' ), new TitleValue( NS_TALK, 'Test' ) ], + [ new TitleValue( NS_USER, 'Test' ), new TitleValue( NS_USER_TALK, 'Test' ) ], + ]; } /** - * @dataProvider provideSpecialNamespaces - * @covers NamespaceInfo::getAssociated + * @dataProvider provideGetTalkPage_good + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::getTalkPage * @covers NamespaceInfo::isMethodValidFor - * - * @param int $ns */ - public function testGetAssociated_special( $ns ) { - $this->setExpectedException( MWException::class, - "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); - $this->newObj()->getAssociated( $ns ); + public function testGetTalkPage_good( LinkTarget $t, LinkTarget $expected ) { + $actual = $this->newObj()->getTalkPage( $t ); + $this->assertEquals( $expected, $actual, $t->getDBkey() ); + } + + public static function provideGetTalkPage_bad() { + return [ + [ new TitleValue( NS_SPECIAL, 'Test' ) ], + [ new TitleValue( NS_MEDIA, 'Test' ) ], + [ new TitleValue( NS_MAIN, '', 'Kittens' ) ], + [ new TitleValue( NS_MAIN, 'Kittens', '', 'acme' ) ], + ]; } /** - * @dataProvider provideSpecialNamespaces - * @covers NamespaceInfo::getAssociated - * @covers NamespaceInfo::getAssociatedPage + * @dataProvider provideGetTalkPage_bad + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::getTalkPage * @covers NamespaceInfo::isMethodValidFor - * - * @param int $ns */ - public function testGetAssociatedPage_special( $ns ) { - $this->setExpectedException( MWException::class, - "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); - $this->newObj()->getAssociatedPage( new TitleValue( $ns, 'A' ) ); + public function testGetTalkPage_bad( LinkTarget $t ) { + $this->setExpectedException( MWException::class ); + $this->newObj()->getTalkPage( $t ); } /** - * @dataProvider provideSpecialNamespaces + * @dataProvider provideGetTalkPage_bad * @covers NamespaceInfo::getAssociated * @covers NamespaceInfo::getAssociatedPage * @covers NamespaceInfo::isMethodValidFor - * @covers Title::getOtherPage - * - * @param int $ns */ - public function testTitleGetOtherPage_special( $ns ) { - $this->setExpectedException( MWException::class, - "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); - Title::makeTitle( $ns, 'A' )->getOtherPage(); + public function testGetAssociatedPage_bad( LinkTarget $t ) { + $this->setExpectedException( MWException::class ); + $this->newObj()->getAssociatedPage( $t ); } /**