diff --git a/includes/Title.php b/includes/Title.php index 5bb8ab80e74..b7359aaff7a 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -156,7 +156,8 @@ class Title implements LinkTarget, IDBAccessObject { /** * @var int Namespace index when there is no namespace. Don't change the * following default, NS_MAIN is hardcoded in several places. See T2696. - * Zero except in {{transclusion}} tags. + * Used primarily for {{transclusion}} tags. + * @see newFromText() */ public $mDefaultNamespace = NS_MAIN; @@ -185,6 +186,9 @@ class Title implements LinkTarget, IDBAccessObject { /** @var bool|null Would deleting this page be a big deletion? */ private $mIsBigDeletion = null; + + /** @var bool|null Is the title known to be valid? */ + private $mIsValid = null; // @} /** @@ -227,10 +231,9 @@ class Title implements LinkTarget, IDBAccessObject { */ public static function newFromDBkey( $key ) { $t = new self(); - $t->mDbkeyform = $key; try { - $t->secureAndSplit(); + $t->secureAndSplit( $key ); return $t; } catch ( MalformedTitleException $ex ) { return null; @@ -294,11 +297,10 @@ class Title implements LinkTarget, IDBAccessObject { } /** - * Create a new Title from text, such as what one would find in a link. De- - * codes any HTML entities in the text. - * - * Title objects returned by this method are guaranteed to be valid, and - * thus return true from the isValid() method. + * Create a new Title from text, such as what one would find in a link. + * Decodes any HTML entities in the text. + * Titles returned by this method are guaranteed to be valid. + * Call canExist() to check if the Title represents an editable page. * * @note The Title instance returned by this method is not guaranteed to be a fresh instance. * It may instead be a cached instance created previously, with references to it remaining @@ -311,7 +313,8 @@ class Title implements LinkTarget, IDBAccessObject { * $text might begin with a namespace prefix, use makeTitle() or * makeTitleSafe(). * @throws InvalidArgumentException - * @return Title|null Title or null on an error. + * @return Title|null Title or null if the Title could not be parsed because + * it is invalid. */ public static function newFromText( $text, $defaultNamespace = NS_MAIN ) { // DWIM: Integers can be passed in here when page titles are used as array keys. @@ -323,7 +326,7 @@ class Title implements LinkTarget, IDBAccessObject { } try { - return self::newFromTextThrow( (string)$text, $defaultNamespace ); + return self::newFromTextThrow( (string)$text, (int)$defaultNamespace ); } catch ( MalformedTitleException $ex ) { return null; } @@ -333,10 +336,8 @@ class Title implements LinkTarget, IDBAccessObject { * Like Title::newFromText(), but throws MalformedTitleException when the title is invalid, * rather than returning null. * - * The exception subclasses encode detailed information about why the title is invalid. - * - * Title objects returned by this method are guaranteed to be valid, and - * thus return true from the isValid() method. + * Titles returned by this method are guaranteed to be valid. + * Call canExist() to check if the Title represents an editable page. * * @note The Title instance returned by this method is not guaranteed to be a fresh instance. * It may instead be a cached instance created previously, with references to it remaining @@ -347,7 +348,7 @@ class Title implements LinkTarget, IDBAccessObject { * @since 1.25 * @param string $text Title text to check * @param int $defaultNamespace - * @throws MalformedTitleException If the title is invalid + * @throws MalformedTitleException If the title is invalid. * @return Title */ public static function newFromTextThrow( $text, $defaultNamespace = NS_MAIN ) { @@ -376,10 +377,9 @@ class Title implements LinkTarget, IDBAccessObject { $filteredText = Sanitizer::decodeCharReferencesAndNormalize( $text ); $t = new Title(); - $t->mDbkeyform = strtr( $filteredText, ' ', '_' ); - $t->mDefaultNamespace = (int)$defaultNamespace; + $dbKeyForm = strtr( $filteredText, ' ', '_' ); - $t->secureAndSplit(); + $t->secureAndSplit( $dbKeyForm, (int)$defaultNamespace ); if ( $defaultNamespace == NS_MAIN ) { $titleCache->set( $text, $t ); } @@ -411,10 +411,10 @@ class Title implements LinkTarget, IDBAccessObject { $url = strtr( $url, '+', ' ' ); } - $t->mDbkeyform = strtr( $url, ' ', '_' ); + $dbKeyForm = strtr( $url, ' ', '_' ); try { - $t->secureAndSplit(); + $t->secureAndSplit( $dbKeyForm ); return $t; } catch ( MalformedTitleException $ex ) { return null; @@ -600,9 +600,8 @@ class Title implements LinkTarget, IDBAccessObject { * The parameters will be checked for validity, which is a bit slower * than makeTitle() but safer for user-provided data. * - * Title objects returned by makeTitleSafe() are guaranteed to be valid, - * that is, they return true from the isValid() method. If no valid Title - * can be constructed from the input, this method returns null. + * The Title object returned by this method is guaranteed to be valid. + * Call canExist() to check if the Title represents an editable page. * * @param int $ns The namespace of the article * @param string $title Database key form @@ -619,10 +618,10 @@ class Title implements LinkTarget, IDBAccessObject { } $t = new Title(); - $t->mDbkeyform = self::makeName( $ns, $title, $fragment, $interwiki, true ); + $dbKeyForm = self::makeName( $ns, $title, $fragment, $interwiki, true ); try { - $t->secureAndSplit(); + $t->secureAndSplit( $dbKeyForm ); return $t; } catch ( MalformedTitleException $ex ) { return null; @@ -844,47 +843,51 @@ class Title implements LinkTarget, IDBAccessObject { } /** - * Returns true if the title is valid, false if it is invalid. + * Returns true if the title is a valid link target, and that it has been + * properly normalized. This method checks that the title is syntactically valid, + * and that the namespace it refers to exists. * - * Valid titles can be round-tripped via makeTitle() and newFromText(). - * Their DB key can be used in the database, though it may not have the correct - * capitalization. + * Titles constructed using newFromText() or makeTitleSafe() are always valid. * - * Invalid titles may get returned from makeTitle(), and it may be useful to - * allow them to exist, e.g. in order to process log entries about pages in - * namespaces that belong to extensions that are no longer installed. + * @note Code that wants to check whether the title can represent a page that can + * be created and edited should use canExist() instead. Examples of valid titles + * that cannot "exist" are Special pages, interwiki links, and on-page section links + * that only have the fragment part set. * - * @note This method is relatively expensive. When constructing Title - * objects that need to be valid, use an instantiator method that is guaranteed - * to return valid titles, such as makeTitleSafe() or newFromText(). + * @see canExist() * * @return bool */ public function isValid() { - $services = MediaWikiServices::getInstance(); - if ( !$services->getNamespaceInfo()->exists( $this->mNamespace ) ) { - return false; + if ( $this->mIsValid !== null ) { + return $this->mIsValid; } try { - $services->getTitleParser()->parseTitle( $this->mDbkeyform, $this->mNamespace ); + $text = $this->getFullText(); + $titleCodec = MediaWikiServices::getInstance()->getTitleParser(); + + '@phan-var MediaWikiTitleCodec $titleCodec'; + $parts = $titleCodec->splitTitleString( $text, $this->mNamespace ); + + // Check that nothing changed! + // This ensures that $text was already perperly normalized. + if ( $parts['fragment'] !== $this->mFragment + || $parts['interwiki'] !== $this->mInterwiki + || $parts['local_interwiki'] !== $this->mLocalInterwiki + || $parts['namespace'] !== $this->mNamespace + || $parts['dbkey'] !== $this->mDbkeyform + ) { + $this->mIsValid = false; + return $this->mIsValid; + } } catch ( MalformedTitleException $ex ) { - return false; + $this->mIsValid = false; + return $this->mIsValid; } - try { - // Title value applies basic syntax checks. Should perhaps be moved elsewhere. - new TitleValue( - $this->mNamespace, - $this->mDbkeyform, - $this->mFragment, - $this->mInterwiki - ); - } catch ( InvalidArgumentException $ex ) { - return false; - } - - return true; + $this->mIsValid = true; + return $this->mIsValid; } /** @@ -1179,12 +1182,49 @@ class Title implements LinkTarget, IDBAccessObject { } /** - * Is this in a namespace that allows actual pages? + * Can this title represent a page in the wiki's database? * - * @return bool + * Titles can exist as pages in the database if they are valid, and they + * are not Special pages, interwiki links, or fragment-only links. + * + * @see isValid() + * + * @return bool true if and only if this title can be used to perform an edit. */ public function canExist() { - return $this->mNamespace >= NS_MAIN; + // NOTE: Don't use getArticleID(), we don't want to + // trigger a database query here. This check is supposed to + // act as an optimization, not add extra cost. + if ( $this->mArticleID > 0 ) { + // It exists, so it can exist. + return true; + } + + // NOTE: we call the relatively expensive isValid() method further down, + // but we can bail out early if we already know the title is invalid. + if ( $this->mIsValid === false ) { + // It's invalid, so it can't exist. + return false; + } + + if ( $this->getNamespace() < NS_MAIN ) { + // It's a special page, so it can't exist in the database. + return false; + } + + if ( $this->isExternal() ) { + // If it's external, it's not local, so it can't exist. + return false; + } + + if ( $this->getText() === '' ) { + // The title has no text, so it can't exist in the database. + // It's probably an on-page section link, like "#something". + return false; + } + + // Double check that the title is valid. + return $this->isValid(); } /** @@ -3285,16 +3325,24 @@ class Title implements LinkTarget, IDBAccessObject { /** * Secure and split - main initialisation function for this object * - * Assumes that mDbkeyform has been set, and is urldecoded + * Assumes that $text is urldecoded * and uses underscores, but not otherwise munged. This function * removes illegal characters, splits off the interwiki and * namespace prefixes, sets the other forms, and canonicalizes * everything. * - * @throws MalformedTitleException On invalid titles - * @return bool True on success + * If this method returns normally, the Title is valid. + * + * @param string $text + * @param int|null $defaultNamespace + * + * @throws MalformedTitleException On malformed titles */ - private function secureAndSplit() { + private function secureAndSplit( $text, $defaultNamespace = null ) { + if ( $defaultNamespace === null ) { + $defaultNamespace = $this->mDefaultNamespace; + } + // @note: splitTitleString() is a temporary hack to allow MediaWikiTitleCodec to share // the parsing code with Title, while avoiding massive refactoring. // @todo: get rid of secureAndSplit, refactor parsing code. @@ -3304,25 +3352,27 @@ class Title implements LinkTarget, IDBAccessObject { $titleCodec = MediaWikiServices::getInstance()->getTitleParser(); '@phan-var MediaWikiTitleCodec $titleCodec'; // MalformedTitleException can be thrown here - $parts = $titleCodec->splitTitleString( $this->mDbkeyform, $this->mDefaultNamespace ); + $parts = $titleCodec->splitTitleString( $text, $defaultNamespace ); # Fill fields $this->setFragment( '#' . $parts['fragment'] ); $this->mInterwiki = $parts['interwiki']; $this->mLocalInterwiki = $parts['local_interwiki']; $this->mNamespace = $parts['namespace']; + $this->mDefaultNamespace = $defaultNamespace; $this->mUserCaseDBKey = $parts['user_case_dbkey']; $this->mDbkeyform = $parts['dbkey']; $this->mUrlform = wfUrlencode( $this->mDbkeyform ); $this->mTextform = strtr( $this->mDbkeyform, '_', ' ' ); + // splitTitleString() guarantees that this title is valid. + $this->mIsValid = true; + # We already know that some pages won't be in the database! - if ( $this->isExternal() || $this->isSpecialPage() ) { + if ( $this->isExternal() || $this->isSpecialPage() || $this->mTextform === '' ) { $this->mArticleID = 0; } - - return true; } /** diff --git a/includes/changes/ChangesFeed.php b/includes/changes/ChangesFeed.php index e60cc0937be..6a8d9a9e54a 100644 --- a/includes/changes/ChangesFeed.php +++ b/includes/changes/ChangesFeed.php @@ -94,7 +94,7 @@ class ChangesFeed { $nsInfo = MediaWikiServices::getInstance()->getNamespaceInfo(); foreach ( $sorted as $obj ) { $title = Title::makeTitle( $obj->rc_namespace, $obj->rc_title ); - $talkpage = $nsInfo->hasTalkNamespace( $obj->rc_namespace ) && $title->isValid() + $talkpage = $nsInfo->hasTalkNamespace( $obj->rc_namespace ) && $title->canExist() ? $title->getTalkPage()->getFullURL() : ''; diff --git a/includes/logging/BlockLogFormatter.php b/includes/logging/BlockLogFormatter.php index d49e3c5710d..fc21d880ba2 100644 --- a/includes/logging/BlockLogFormatter.php +++ b/includes/logging/BlockLogFormatter.php @@ -129,7 +129,7 @@ class BlockLogFormatter extends LogFormatter { public function getPreloadTitles() { $title = $this->entry->getTarget(); // Preload user page for non-autoblocks - if ( substr( $title->getText(), 0, 1 ) !== '#' && $title->isValid() ) { + if ( substr( $title->getText(), 0, 1 ) !== '#' && $title->canExist() ) { return [ $title->getTalkPage() ]; } return []; diff --git a/includes/title/MediaWikiTitleCodec.php b/includes/title/MediaWikiTitleCodec.php index 3ebb4430ece..484c51e5c8a 100644 --- a/includes/title/MediaWikiTitleCodec.php +++ b/includes/title/MediaWikiTitleCodec.php @@ -167,17 +167,11 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { // Convert things like é ā or 〗 into normalized (T16952) text $filteredText = Sanitizer::decodeCharReferencesAndNormalize( $text ); - // NOTE: this is an ugly cludge that allows this class to share the + // NOTE: this is an ugly kludge that allows this class to share the // code for parsing with the old Title class. The parser code should // be refactored to avoid this. $parts = $this->splitTitleString( $filteredText, $defaultNamespace ); - // Fragment-only is okay, but only with no namespace - if ( $parts['dbkey'] === '' && - ( $parts['fragment'] === '' || $parts['namespace'] !== NS_MAIN ) ) { - throw new MalformedTitleException( 'title-invalid-empty', $text ); - } - return new TitleValue( $parts['namespace'], $parts['dbkey'], @@ -284,7 +278,8 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { } /** - * Normalizes and splits a title string. + * Validates, normalizes and splits a title string. + * This is the "source of truth" for title validity. * * This function removes illegal characters, splits off the interwiki and * namespace prefixes, sets the other forms, and canonicalizes @@ -494,9 +489,22 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { throw new MalformedTitleException( 'title-invalid-leading-colon', $text ); } - # Fill fields + // Fill fields $parts['dbkey'] = $dbkey; + // Sanity check to ensure that the return value can be used to construct a TitleValue. + // All issues should in theory be caught above, this is here to enforce consistency. + try { + TitleValue::assertValidSpec( + $parts['namespace'], + $parts['dbkey'], + $parts['fragment'], + $parts['interwiki'] + ); + } catch ( InvalidArgumentException $ex ) { + throw new MalformedTitleException( 'title-invalid', $text, [ $ex->getMessage() ] ); + } + return $parts; } diff --git a/includes/title/TitleValue.php b/includes/title/TitleValue.php index 7abe21ba67a..109659fd440 100644 --- a/includes/title/TitleValue.php +++ b/includes/title/TitleValue.php @@ -86,6 +86,28 @@ class TitleValue implements LinkTarget { * @throws InvalidArgumentException */ public function __construct( $namespace, $title, $fragment = '', $interwiki = '' ) { + self::assertValidSpec( $namespace, $title, $fragment, $interwiki ); + + $this->namespace = $namespace; + $this->dbkey = strtr( $title, ' ', '_' ); + $this->fragment = $fragment; + $this->interwiki = $interwiki; + } + + /** + * Asserts that the given parameters could be used to construct a TitleValue object. + * Performs basic syntax and consistency checks. Does not perform full validation, + * use TitleParser::makeTitleValueSafe() for that. + * + * @param int $namespace + * @param string $title + * @param string $fragment + * @param string $interwiki + * + * @throws InvalidArgumentException if the combination of parameters is not valid for + * constructing a TitleValue. + */ + public static function assertValidSpec( $namespace, $title, $fragment = '', $interwiki = '' ) { if ( !is_int( $namespace ) ) { throw new ParameterTypeException( '$namespace', 'int' ); } @@ -99,20 +121,19 @@ class TitleValue implements LinkTarget { throw new ParameterTypeException( '$interwiki', 'string' ); } - // Sanity check, no full validation or normalization applied here! Assert::parameter( !preg_match( '/^[_ ]|[\r\n\t]|[_ ]$/', $title ), '$title', "invalid name '$title'" ); - Assert::parameter( - $title !== '' || - ( $namespace === NS_MAIN && ( $fragment !== '' || $interwiki !== '' ) ), - '$title', - 'should not be empty unless namespace is main and fragment or interwiki is non-empty' - ); - $this->namespace = $namespace; - $this->dbkey = strtr( $title, ' ', '_' ); - $this->fragment = $fragment; - $this->interwiki = $interwiki; + // NOTE: As of MW 1.34, [[#]] is rendered as a valid link, pointing to the empty + // page title, effectively leading to the wiki's main page. This means that a completely + // empty TitleValue has to be considered valid, for consistency with Title. + // Also note that [[#foo]] is a valid on-page section links, and that [[acme:#foo]] is + // a valid interwiki link. + Assert::parameter( + $title !== '' || $namespace === NS_MAIN, + '$title', + 'should not be empty unless namespace is main' + ); } /** diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 63830b50cdf..ca0983ccfa0 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -341,6 +341,7 @@ "no-null-revision": "Could not create new null revision for page \"$1\"", "badtitle": "Bad title", "badtitletext": "The requested page title was invalid, empty, or an incorrectly linked inter-language or inter-wiki title.\nIt may contain one or more characters that cannot be used in titles.", + "title-invalid": "The requested page title is invalid", "title-invalid-empty": "The requested page title is empty or contains only the name of a namespace.", "title-invalid-utf8": "The requested page title contains an invalid UTF-8 sequence.", "title-invalid-interwiki": "The requested page title contains an interwiki link which cannot be used in titles.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 26462e7ea03..06d0c5fba2a 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -553,6 +553,7 @@ "no-null-revision": "Error message shown when no null revision could be created to reflect a protection level change.\n\nAbout \"null revision\":\n* Create a new null-revision for insertion into a page's history. This will not re-save the text, but simply refer to the text from the previous version.\n* Such revisions can for instance identify page rename operations and other such meta-modifications.\n\nParameters:\n* $1 - page title", "badtitle": "The page title when a user requested a page with invalid page name. The content will be {{msg-mw|badtitletext}}.", "badtitletext": "The message shown when a user requested a page with invalid page name. The page title will be {{msg-mw|badtitle}}.\n\nSee also:\n* {{msg-mw|selfmove}}\n* {{msg-mw|immobile-source-namespace}}\n* {{msg-mw|immobile-target-namespace-iw}}\n* {{msg-mw|immobile-target-namespace}}", + "title-invalid": "Used as text of error message: invalid title", "title-invalid-empty": "Used as text of error message: empty title", "title-invalid-utf8": "Used as text of error message: invalid UTF8 sequence", "title-invalid-interwiki": "Used as text of error message: invalid interwiki link", diff --git a/maintenance/uppercaseTitlesForUnicodeTransition.php b/maintenance/uppercaseTitlesForUnicodeTransition.php index f5bafdeb838..c89cdd6e28f 100644 --- a/maintenance/uppercaseTitlesForUnicodeTransition.php +++ b/maintenance/uppercaseTitlesForUnicodeTransition.php @@ -316,7 +316,7 @@ class UppercaseTitlesForUnicodeTransition extends Maintenance { return false; } - if ( !$newTitle->isValid() ) { + if ( !$newTitle->canExist() ) { $this->error( "Cannot move {$oldTitle->getPrefixedText()} → $nt: " . "$munge and munged title '{$newTitle->getPrefixedText()}' is not valid" diff --git a/tests/phpunit/includes/MovePageTest.php b/tests/phpunit/includes/MovePageTest.php index c33fef44578..56828518674 100644 --- a/tests/phpunit/includes/MovePageTest.php +++ b/tests/phpunit/includes/MovePageTest.php @@ -1,6 +1,7 @@ getMock( InterwikiLookup::class ); + $iwLookup->method( 'isValidInterwiki' ) + ->willReturn( true ); + + $this->setService( + 'InterwikiLookup', + $iwLookup + ); + if ( is_string( $old ) ) { $old = Title::newFromText( $old ); } @@ -335,6 +345,15 @@ class MovePageTest extends MediaWikiTestCase { * @param array $extraOptions */ public function testMove( $old, $new, array $expectedErrors, array $extraOptions = [] ) { + $iwLookup = $this->getMock( InterwikiLookup::class ); + $iwLookup->method( 'isValidInterwiki' ) + ->willReturn( true ); + + $this->setService( + 'InterwikiLookup', + $iwLookup + ); + if ( is_string( $old ) ) { $old = Title::newFromText( $old ); } diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index bf1d7e509d7..b246419492f 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -718,7 +718,16 @@ class TitleTest extends MediaWikiTestCase { * @param bool $isValid */ public function testIsValid( Title $title, $isValid ) { - $this->assertEquals( $isValid, $title->isValid(), $title->getPrefixedText() ); + $iwLookup = $this->getMock( InterwikiLookup::class ); + $iwLookup->method( 'isValidInterwiki' ) + ->willReturn( true ); + + $this->setService( + 'InterwikiLookup', + $iwLookup + ); + + $this->assertEquals( $isValid, $title->isValid(), $title->getFullText() ); } public static function provideIsValid() { @@ -727,6 +736,42 @@ class TitleTest extends MediaWikiTestCase { [ Title::makeTitle( NS_MAIN, '<>' ), false ], [ Title::makeTitle( NS_MAIN, '|' ), false ], [ Title::makeTitle( NS_MAIN, '#' ), false ], + [ Title::makeTitle( NS_PROJECT, '#' ), false ], + [ Title::makeTitle( NS_MAIN, '', 'test' ), true ], + [ Title::makeTitle( NS_PROJECT, '#test' ), false ], + [ Title::makeTitle( NS_MAIN, '', 'test', 'wikipedia' ), true ], + [ Title::makeTitle( NS_MAIN, 'Test', '', 'wikipedia' ), true ], + [ Title::makeTitle( NS_MAIN, 'Test' ), true ], + [ Title::makeTitle( NS_SPECIAL, 'Test' ), true ], + [ Title::makeTitle( NS_MAIN, ' Test' ), false ], + [ Title::makeTitle( NS_MAIN, '_Test' ), false ], + [ Title::makeTitle( NS_MAIN, 'Test ' ), false ], + [ Title::makeTitle( NS_MAIN, 'Test_' ), false ], + [ Title::makeTitle( NS_MAIN, "Test\nthis" ), false ], + [ Title::makeTitle( NS_MAIN, "Test\tthis" ), false ], + [ Title::makeTitle( -33, 'Test' ), false ], + [ Title::makeTitle( 77663399, 'Test' ), false ], + ]; + } + + /** + * @covers Title::canExist + * @dataProvider provideCanExist + * @param Title $title + * @param bool $canExist + */ + public function testCanExist( Title $title, $canExist ) { + $this->assertEquals( $canExist, $title->canExist(), $title->getFullText() ); + } + + public static function provideCanExist() { + return [ + [ Title::makeTitle( NS_MAIN, '' ), false ], + [ Title::makeTitle( NS_MAIN, '<>' ), false ], + [ Title::makeTitle( NS_MAIN, '|' ), false ], + [ Title::makeTitle( NS_MAIN, '#' ), false ], + [ Title::makeTitle( NS_PROJECT, '#test' ), false ], + [ Title::makeTitle( NS_MAIN, '', 'test', 'acme' ), false ], [ Title::makeTitle( NS_MAIN, 'Test' ), true ], [ Title::makeTitle( NS_MAIN, ' Test' ), false ], [ Title::makeTitle( NS_MAIN, '_Test' ), false ], @@ -736,6 +781,11 @@ class TitleTest extends MediaWikiTestCase { [ Title::makeTitle( NS_MAIN, "Test\tthis" ), false ], [ Title::makeTitle( -33, 'Test' ), false ], [ Title::makeTitle( 77663399, 'Test' ), false ], + + // Valid but can't exist + [ Title::makeTitle( NS_MAIN, '', 'test' ), false ], + [ Title::makeTitle( NS_SPECIAL, 'Test' ), false ], + [ Title::makeTitle( NS_MAIN, 'Test', '', 'acme' ), false ], ]; } diff --git a/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php b/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php index 4d88bae9656..7310cda2892 100644 --- a/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php +++ b/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php @@ -20,7 +20,6 @@ */ use MediaWiki\Interwiki\InterwikiLookup; -use Wikimedia\TestingAccessWrapper; /** * @covers MediaWikiTitleCodec @@ -366,6 +365,7 @@ class MediaWikiTitleCodecTest extends MediaWikiTestCase { [ 'X#ñ', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'ñ' ) ], // target section parsing 'empty fragment' => [ 'X#', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X' ) ], + 'only fragment' => [ '#', NS_MAIN, 'en', new TitleValue( NS_MAIN, '' ) ], 'double hash' => [ 'X##', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', '#' ) ], 'fragment with hash' => [ 'X#z#z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z#z' ) ], 'fragment with space' => [ 'X#z z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z z' ) ], @@ -396,7 +396,7 @@ class MediaWikiTitleCodecTest extends MediaWikiTestCase { // TODO: test unicode errors return [ - [ '#' ], + [ 'User:#' ], [ '::' ], [ '::xx' ], [ '::##' ], @@ -490,14 +490,13 @@ class MediaWikiTitleCodecTest extends MediaWikiTestCase { $this->setService( 'TitleFormatter', $codec ); $actual = Title::makeTitleSafe( $ns, $text, $fragment, $interwiki ); - // We need to reset some members so equality testing works - if ( $actual ) { - $wrapper = TestingAccessWrapper::newFromObject( $actual ); - $wrapper->mArticleID = $actual->getNamespace() === NS_SPECIAL ? 0 : -1; - $wrapper->mLocalInterwiki = false; - $wrapper->mUserCaseDBKey = null; + + if ( $expected ) { + $this->assertNotNull( $actual ); + $this->assertTrue( Title::castFromLinkTarget( $expected )->equals( $actual ) ); + } else { + $this->assertNull( $actual ); } - $this->assertEquals( Title::castFromLinkTarget( $expected ), $actual ); } public static function provideMakeTitleValueSafe() { diff --git a/tests/phpunit/unit/includes/title/TitleValueTest.php b/tests/phpunit/unit/includes/title/TitleValueTest.php index 39e264bc63b..8b91ee5cb8d 100644 --- a/tests/phpunit/unit/includes/title/TitleValueTest.php +++ b/tests/phpunit/unit/includes/title/TitleValueTest.php @@ -35,6 +35,7 @@ class TitleValueTest extends \MediaWikiUnitTestCase { [ NS_USER, 'TestThis', '', 'baz', false, true ], [ NS_MAIN, 'foo bar', '', '', false, false ], [ NS_MAIN, 'foo_bar', '', '', false, false ], + [ NS_MAIN, '', '', '', false, false ], ]; } @@ -56,6 +57,14 @@ class TitleValueTest extends \MediaWikiUnitTestCase { $this->assertEquals( $hasInterwiki, $title->isExternal() ); } + /** + * @dataProvider goodConstructorProvider + */ + public function testAssertValidSpec( $ns, $text, $fragment, $interwiki ) { + TitleValue::assertValidSpec( $ns, $text, $fragment, $interwiki ); + $this->assertTrue( true ); // we are just checking that no exception is thrown + } + public function badConstructorProvider() { return [ [ 'foo', 'title', 'fragment', '' ], @@ -88,6 +97,14 @@ class TitleValueTest extends \MediaWikiUnitTestCase { new TitleValue( $ns, $text, $fragment, $interwiki ); } + /** + * @dataProvider badConstructorProvider + */ + public function testAssertValidSpecErrors( $ns, $text, $fragment, $interwiki ) { + $this->setExpectedException( InvalidArgumentException::class ); + TitleValue::assertValidSpec( $ns, $text, $fragment, $interwiki ); + } + public function fragmentTitleProvider() { return [ [ new TitleValue( NS_MAIN, 'Test' ), 'foo' ],