From: Ryan Schmidt Date: Thu, 29 Jan 2009 00:29:52 +0000 (+0000) Subject: * (bug 11644) update recursive redirect checking code per CodeReview discussion on... X-Git-Tag: 1.31.0-rc.0~43150 X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=885124020986ee01e705c288731d13b9d091ca9c;p=lhc%2Fweb%2Fwiklou.git * (bug 11644) update recursive redirect checking code per CodeReview discussion on r45973 --- diff --git a/includes/Article.php b/includes/Article.php index d5ed3bd02f..da8e0075d4 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -103,8 +103,7 @@ class Article { * @return Title object */ public function insertRedirect() { - // set noRecurse so that we always get an entry even if redirects are "disabled" - $retval = Title::newFromRedirect( $this->getContent(), false, true ); + $retval = Title::newFromRedirect( $this->getContent() ); if( !$retval ) { return null; } @@ -136,7 +135,7 @@ class Article { * @return mixed false, Title of in-wiki target, or string with URL */ public function followRedirectText( $text ) { - $rt = Title::newFromRedirect( $text ); // only get the final target + $rt = Title::newFromRedirectRecurse( $text ); // recurse through to only get the final target # process if title object is valid and not special:userlogout if( $rt ) { if( $rt->getInterwiki() != '' ) { @@ -607,10 +606,9 @@ class Article { } // Apparently loadPageData was never called $this->loadContent(); - // Only get the next target to reduce load times - $titleObj = Title::newFromRedirect( $this->fetchContent(), false, true ); + $titleObj = Title::newFromRedirectRe( $this->fetchContent() ); } else { - $titleObj = Title::newFromRedirect( $text, false, true ); + $titleObj = Title::newFromRedirect( $text ); } return $titleObj !== NULL; } @@ -943,7 +941,7 @@ class Article { $wgOut->addHTML( htmlspecialchars( $this->mContent ) ); $wgOut->addHTML( "\n\n" ); } - } else if( $rt = Title::newFromRedirect( $text, true ) ) { # get an array of redirect targets + } else if( $rt = Title::newFromRedirectArray( $text ) ) { # get an array of redirect targets # Don't append the subtitle if this was an old revision $wgOut->addHTML( $this->viewRedirect( $rt, !$wasRedirected && $this->isCurrent() ) ); $parseout = $wgParser->parse($text, $this->mTitle, ParserOptions::newFromUser($wgUser)); @@ -3475,9 +3473,9 @@ class Article { public static function getAutosummary( $oldtext, $newtext, $flags ) { # Decide what kind of autosummary is needed. - # Redirect autosummaries -- should only get the next target and not recurse - $ot = Title::newFromRedirect( $oldtext, false, true ); - $rt = Title::newFromRedirect( $newtext, false, true ); + # Redirect autosummaries + $ot = Title::newFromRedirect( $oldtext ); + $rt = Title::newFromRedirect( $newtext ); if( is_object( $rt ) && ( !is_object( $ot ) || !$rt->equals( $ot ) || $ot->getFragment() != $rt->getFragment() ) ) { return wfMsgForContent( 'autoredircomment', $rt->getFullText() ); } diff --git a/includes/EditPage.php b/includes/EditPage.php index b236c0bc87..74ce16e514 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1683,7 +1683,7 @@ END $parserOptions->setTidy(true); $parserOutput = $wgParser->parse( $previewtext, $this->mTitle, $parserOptions ); $previewHTML = $parserOutput->mText; - } elseif ( $rt = Title::newFromRedirect( $this->textbox1, true ) ) { + } elseif ( $rt = Title::newFromRedirectArray( $this->textbox1 ) ) { $previewHTML = $this->mArticle->viewRedirect( $rt, false ); } else { $toparse = $this->textbox1; diff --git a/includes/Title.php b/includes/Title.php index eb83b43c78..fea8588bd7 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -294,21 +294,77 @@ class Title { /** * Extract a redirect destination from a string and return the * Title, or null if the text doesn't contain a valid redirect + * This will only return the very next target, useful for + * the redirect table and other checks that don't need full recursion * * @param $text \type{\string} Text with possible redirect - * @param $getAllTargets \type{\bool} Should we get an array of every target or just the final one? - * @param $noRecurse \type{\bool} This will prevent any and all recursion, only getting the very next target - * This is mainly meant for Article::insertRedirect so that the redirect table still works properly - * (makes double redirect and broken redirect reports display accurate results). * @return \type{Title} The corresponding Title - * @return \type{\array} Array of redirect targets (Title objects), with the destination being last */ - public static function newFromRedirect( $text, $getAllTargets = false, $noRecurse = false ) { + public static function newFromRedirect( $text ) { + return self::newFromRedirectInternal( $text ); + } + + /** + * Extract a redirect destination from a string and return the + * Title, or null if the text doesn't contain a valid redirect + * This will recurse down $wgMaxRedirects times or until a non-redirect target is hit + * in order to provide (hopefully) the Title of the final destination instead of another redirect + * + * @param $text \type{\string} Text with possible redirect + * @return \type{Title} The corresponding Title + */ + public static function newFromRedirectRecurse( $text ) { + $titles = self::newFromRedirectArray( $text ); + return array_pop( $titles ); + } + + /** + * Extract a redirect destination from a string and return an + * array of Titles, or null if the text doesn't contain a valid redirect + * The last element in the array is the final destination after all redirects + * have been resolved (up to $wgMaxRedirects times) + * + * @param $text \type{\string} Text with possible redirect + * @return \type{\array} Array of Titles, with the destination last + */ + public static function newFromRedirectArray( $text ) { global $wgMaxRedirects; // are redirects disabled? - // Note that we should get a Title object if possible if $noRecurse is true so that the redirect table functions properly - if( !$noRecurse && $wgMaxRedirects < 1 ) + if( $wgMaxRedirects < 1 ) return null; + $title = self::newFromRedirectInternal( $text ); + if( is_null( $title ) ) + return null; + // recursive check to follow double redirects + $recurse = $wgMaxRedirects; + $titles = array( $title ); + while( --$recurse >= 0 ) { + if( $title->isRedirect() ) { + $article = new Article( $title, 0 ); + $newtitle = $article->getRedirectTarget(); + } else { + break; + } + // Redirects to some special pages are not permitted + if( $newtitle instanceOf Title && $newtitle->isValidRedirectTarget() ) { + // the new title passes the checks, so make that our current title so that further recursion can be checked + $title = $newtitle; + $titles[] = $newtitle; + } else { + break; + } + } + return $titles; + } + + /** + * Really extract the redirect destination + * Do not call this function directly, use one of the newFromRedirect* functions above + * + * @param $text \type{\string} Text with possible redirect + * @return \type{Title} The corresponding Title + */ + protected static function newFromRedirectInternal( $text ) { $redir = MagicWord::get( 'redirect' ); $text = trim($text); if( $redir->matchStartAndRemove( $text ) ) { @@ -326,38 +382,11 @@ class Title { $m[1] = urldecode( ltrim( $m[1], ':' ) ); } $title = Title::newFromText( $m[1] ); - // If the initial title is a redirect to bad special pages or is invalid, quit early + // If the title is a redirect to bad special pages or is invalid, return null if( !$title instanceof Title || !$title->isValidRedirectTarget() ) { return null; } - // If $noRecurse is true, simply return here - if( $noRecurse ) { - return $title; - } - // recursive check to follow double redirects - $recurse = $wgMaxRedirects; - $targets = array(); - while( --$recurse >= 0 ) { - // Redirects to some special pages are not permitted - if( $title instanceof Title && $title->isValidRedirectTarget() ) { - $targets[] = $title; - if( $title->isRedirect() ) { - $article = new Article( $title, 0 ); - $title = $article->getRedirectTarget(); - } else { - break; - } - } else { - break; - } - } - if( $getAllTargets ) { - // return every target or null if no targets due to invalid title or whatever - return ( $targets === array() ) ? null : $targets; - } else { - // return the final destination -- invalid titles are checked earlier - return $title; - } + return $title; } } return null;