Remove support for $wgMaxRedirect

Redirect chains have never worked as intended.

Bug: T296430
Change-Id: If0e514c57b8f3d857d956a581f1b549518ecb099

RELEASE-NOTES-1.39

@@ -43,6 +43,8 @@ For notes on 1.38.x and older releases, see HISTORY.
   purpose is deprecated.
 * $wgAllowJavaUploads - To allow uploads of JAR files, remove application/java
   from $wgMimeTypeExclusions.
+* $wgMaxRedirects has been removed. This feature never worked as intended,
+  see T296430.
 * …
 
 === New user-facing features in 1.39 ===
@@ -201,6 +203,11 @@ because of Phabricator reports.
      to pass one to wfUrlProtocols anyway);
   3) they use type hints (don't try passing null instead of string, etc.).
 * MaintainableDBConnRef is deprecated, use DBConnRef instead.
+* AbstractContent::getRedirectChain() and
+  AbstractContent::getUltimateRedirectTarget() are now emitting deprecation
+  warnings (T296430).
+* Passing an array of targets to Article::getRedirectHeaderHtml() is
+  deprecated. Supply a single redirect target instead (T296430).
 * Skin::getAction is deprecated. Use IContextSource::getActionName instead.
 * ILBFactory::forEachLB() is deprecated. Use ::getAllLBs().
 * LoadBalancer::forEachOpenConnection() and ::forEachOpenPrimaryConnection()

docs/config-schema.yaml

@@ -3780,12 +3780,6 @@ config-schema:
             Signature button on the edit toolbar, and may also be used by extensions.
             For example, "traditional" style wikis, where content and discussion are
             intermixed, could place NS_MAIN and NS_PROJECT namespaces in this array.
-    MaxRedirects:
-        default: 1
-        description: |-
-            Max number of redirects to follow when resolving redirects.
-            1 means only the first redirect is followed (default behavior).
-            0 or less means no redirects are followed.
     InvalidRedirectTargets:
         default:
             - Filepath

includes/DefaultSettings.php

@@ -3060,13 +3060,6 @@ $wgShortPagesNamespaceExclusions = [
 $wgExtraSignatureNamespaces = [
 ];
 
-/**
- * Variable for the MaxRedirects setting, for use in LocalSettings.php
- * @see MainConfigSchema::MaxRedirects
- * @note Do not change manually, generated by maintenance/generateConfigDefaultSettings.php!
- */
-$wgMaxRedirects = 1;
-
 /**
  * Variable for the InvalidRedirectTargets setting, for use in LocalSettings.php
  * @see MainConfigSchema::InvalidRedirectTargets

includes/MainConfigNames.php

@@ -2208,12 +2208,6 @@ class MainConfigNames {
 	 */
 	public const ExtraSignatureNamespaces = 'ExtraSignatureNamespaces';
 
-	/**
-	 * Name constant for the MaxRedirects setting, for use with Config::get()
-	 * @see MainConfigSchema::MaxRedirects
-	 */
-	public const MaxRedirects = 'MaxRedirects';
-
 	/**
 	 * Name constant for the InvalidRedirectTargets setting, for use with Config::get()
 	 * @see MainConfigSchema::InvalidRedirectTargets

includes/MainConfigSchema.php

@@ -5723,16 +5723,6 @@ class MainConfigSchema {
 		'type' => 'list',
 	];
 
-	/**
-	 * Max number of redirects to follow when resolving redirects.
-	 *
-	 * 1 means only the first redirect is followed (default behavior).
-	 * 0 or less means no redirects are followed.
-	 */
-	public const MaxRedirects = [
-		'default' => 1,
-	];
-
 	/**
 	 * Array of invalid page redirect targets.
 	 *

includes/actions/ViewAction.php

@@ -61,14 +61,11 @@ class ViewAction extends FormlessAction {
 			$redirFromTitle = $this->getArticle()->getRedirectedFrom();
 			if ( !$redirFromTitle ) {
 				$touched = $this->getWikiPage()->getTouched();
-			} elseif ( $config->get( MainConfigNames::MaxRedirects ) <= 1 ) {
+			} else {
 				$touched = max(
 					$this->getWikiPage()->getTouched(),
 					$redirFromTitle->getTouched()
 				);
-			} else {
-				// Don't bother following the chain and getting the max mtime
-				$touched = null;
 			}
 
 			// Send HTTP 304 if the IMS matches or otherwise set expiry/last-modified headers

includes/config-schema.php

@@ -719,7 +719,6 @@ return [
 			],
 			'ExtraSignatureNamespaces' => [
 			],
-			'MaxRedirects' => 1,
 			'InvalidRedirectTargets' => [
 				0 => 'Filepath',
 				1 => 'Mypage',

includes/content/AbstractContent.php

@@ -31,7 +31,6 @@ use MediaWiki\Content\Renderer\ContentParseParams;
 use MediaWiki\Content\Transform\PreloadTransformParamsValue;
 use MediaWiki\Content\Transform\PreSaveTransformParamsValue;
 use MediaWiki\Content\ValidationParams;
-use MediaWiki\MainConfigNames;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -269,43 +268,23 @@ abstract class AbstractContent implements Content {
 
 	/**
 	 * @since 1.21
-	 * @deprecated since 1.38 Support for $wgMaxRedirect will be removed
-	 *   soon so this will go away with it. See T296430.
+	 * @deprecated since 1.38, use getRedirectTarget() instead.
+	 *   Emitting deprecation warnings since 1.39.
+	 *   Support for redirect chains has been removed.
 	 *
 	 * @return Title[]|null
 	 *
 	 * @see Content::getRedirectChain
 	 */
 	public function getRedirectChain() {
-		$maxRedirects = MediaWikiServices::getInstance()->getMainConfig()->get(
-			MainConfigNames::MaxRedirects );
+		wfDeprecated( __METHOD__, '1.38' );
+
 		$title = $this->getRedirectTarget();
 		if ( $title === null ) {
 			return null;
+		} else {
+			return [ $title ];
 		}
-		$wikiPageFactory = MediaWikiServices::getInstance()->getWikiPageFactory();
-		// recursive check to follow double redirects
-		$recurse = $maxRedirects;
-		$titles = [ $title ];
-		while ( --$recurse > 0 ) {
-			if ( $title->isRedirect() ) {
-				$page = $wikiPageFactory->newFromTitle( $title );
-				$newtitle = $page->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;
 	}
 
 	/**
@@ -326,17 +305,18 @@ abstract class AbstractContent implements Content {
 	 * @note Migrated here from Title::newFromRedirectRecurse.
 	 *
 	 * @since 1.21
-	 * @deprecated since 1.38 Support for $wgMaxRedirect will be removed
-	 *   soon so this will go away with it. See T296430.
+	 * @deprecated since 1.38, use getRedirectTarget() instead.
+	 *   Emitting deprecation warnings since 1.39.
+	 *   Support for redirect chains has been removed.
 	 *
 	 * @return Title|null
 	 *
 	 * @see Content::getUltimateRedirectTarget
 	 */
 	public function getUltimateRedirectTarget() {
-		$titles = $this->getRedirectChain();
+		wfDeprecated( __METHOD__, '1.38' );
 
-		return $titles ? array_pop( $titles ) : null;
+		return $this->getRedirectTarget();
 	}
 
 	/**

includes/content/Content.php

@@ -287,25 +287,9 @@ interface Content {
 
 	// TODO: make RenderOutput and RenderOptions base classes
 
-	/**
-	 * Construct the redirect destination from this content and return an
-	 * array of Titles, or null if this content doesn't represent a redirect.
-	 * The last element in the array is the final destination after all redirects
-	 * have been resolved (up to $wgMaxRedirects times).
-	 *
-	 * @since 1.21
-	 * @deprecated since 1.38 Support for $wgMaxRedirect will be removed
-	 *   soon so this will go away with it. See T296430.
-	 *
-	 * @return Title[]|null List of Titles, with the destination last.
-	 */
-	public function getRedirectChain();
-
 	/**
 	 * Construct the redirect destination from this content and return a Title,
 	 * or null if this content doesn't represent a redirect.
-	 * This will only return the immediate redirect target, useful for
-	 * the redirect table and other checks that don't need full recursion.
 	 *
 	 * @since 1.21
 	 *
@@ -313,25 +297,6 @@ interface Content {
 	 */
 	public function getRedirectTarget();
 
-	/**
-	 * Construct the redirect destination from this content and return the
-	 * Title, or null if this content doesn't represent a 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.
-	 *
-	 * There is usually no need to override the default behavior, subclasses that
-	 * want to implement redirects should override getRedirectTarget().
-	 *
-	 * @since 1.21
-	 * @deprecated since 1.38 Support for $wgMaxRedirect will be removed
-	 *   soon so this will go away with it. See T296430.
-	 *
-	 * @return Title|null
-	 */
-	public function getUltimateRedirectTarget();
-
 	/**
 	 * Returns whether this Content represents a redirect.
 	 * Shorthand for getRedirectTarget() !== null.

includes/content/WikitextContent.php

@@ -147,18 +147,10 @@ class WikitextContent extends TextContent {
 	 * @return array List of two elements: Title|null and string.
 	 */
 	public function getRedirectTargetAndText() {
-		$maxRedirects = MediaWikiServices::getInstance()->getMainConfig()->get( MainConfigNames::MaxRedirects );
-
 		if ( $this->redirectTargetAndText !== null ) {
 			return $this->redirectTargetAndText;
 		}
 
-		if ( $maxRedirects < 1 ) {
-			// redirects are disabled, so quit early
-			$this->redirectTargetAndText = [ null, $this->getText() ];
-			return $this->redirectTargetAndText;
-		}
-
 		$redir = MediaWikiServices::getInstance()->getMagicWordFactory()->get( 'redirect' );
 		$text = ltrim( $this->getText() );
 		if ( $redir->matchStartAndRemove( $text ) ) {

includes/page/Article.php

@@ -1733,39 +1733,38 @@ class Article implements Page {
 	 *
 	 * @since 1.23
 	 * @param Language $lang
-	 * @param Title|Title[] $target Destination(s) to redirect
+	 * @param Title $target Destination to redirect
 	 * @param bool $forceKnown Should the image be shown as a bluelink regardless of existence?
 	 * @return string Containing HTML with redirect link
 	 */
 	public static function getRedirectHeaderHtml( Language $lang, $target, $forceKnown = false ) {
-		if ( !is_array( $target ) ) {
-			$target = [ $target ];
+		if ( is_array( $target ) ) {
+			// Up until 1.39, $target was allowed to be an array.
+			wfDeprecatedMsg( 'The $target parameter can no longer be an array', '1.39' );
+			$target = reset( $target ); // There really can only be one element (T296430)
 		}
 
 		$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
 
 		$html = '<ul class="redirectText">';
-		/** @var Title $title */
-		foreach ( $target as $title ) {
-			if ( $forceKnown ) {
-				$link = $linkRenderer->makeKnownLink(
-					$title,
-					$title->getFullText(),
-					[],
-					// Make sure wiki page redirects are not followed
-					$title->isRedirect() ? [ 'redirect' => 'no' ] : []
-				);
-			} else {
-				$link = $linkRenderer->makeLink(
-					$title,
-					$title->getFullText(),
-					[],
-					// Make sure wiki page redirects are not followed
-					$title->isRedirect() ? [ 'redirect' => 'no' ] : []
-				);
-			}
-			$html .= '<li>' . $link . '</li>';
+		if ( $forceKnown ) {
+			$link = $linkRenderer->makeKnownLink(
+				$target,
+				$target->getFullText(),
+				[],
+				// Make sure wiki page redirects are not followed
+				$target->isRedirect() ? [ 'redirect' => 'no' ] : []
+			);
+		} else {
+			$link = $linkRenderer->makeLink(
+				$target,
+				$target->getFullText(),
+				[],
+				// Make sure wiki page redirects are not followed
+				$target->isRedirect() ? [ 'redirect' => 'no' ] : []
+			);
 		}
+		$html .= '<li>' . $link . '</li>';
 		$html .= '</ul>';
 
 		$redirectToText = wfMessage( 'redirectto' )->inLanguage( $lang )->escaped();

includes/resourceloader/ResourceLoaderWikiModule.php

@@ -213,13 +213,14 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
 	/**
 	 * @param PageIdentity $page
 	 * @param ResourceLoaderContext $context
-	 * @param int|null $maxRedirects Maximum number of redirects to follow. If
-	 *  null, uses $wgMaxRedirects
+	 * @param int $maxRedirects Maximum number of redirects to follow.
+	 *        Either 0 or 1.
 	 * @return Content|null
 	 * @since 1.32 added the $context and $maxRedirects parameters
+	 * @internal for testing
 	 */
 	protected function getContentObj(
-		PageIdentity $page, ResourceLoaderContext $context, $maxRedirects = null
+		PageIdentity $page, ResourceLoaderContext $context, $maxRedirects = 1
 	) {
 		$overrideCallback = $context->getContentOverrideCallback();
 		$content = $overrideCallback ? call_user_func( $overrideCallback, $page ) : null;
@@ -249,14 +250,9 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
 			}
 		}
 
-		if ( $content->isRedirect() ) {
-			if ( $maxRedirects === null ) {
-				$maxRedirects = $this->getConfig()->get( MainConfigNames::MaxRedirects ) ?: 0;
-			}
-			if ( $maxRedirects > 0 ) {
-				$newTitle = $content->getRedirectTarget();
-				return $newTitle ? $this->getContentObj( $newTitle, $context, $maxRedirects - 1 ) : null;
-			}
+		if ( $maxRedirects > 0 && $content->isRedirect() ) {
+			$newTitle = $content->getRedirectTarget();
+			return $newTitle ? $this->getContentObj( $newTitle, $context, 0 ) : null;
 		}
 
 		return $content;

tests/phpunit/ResourceLoaderTestCase.php

@@ -67,9 +67,6 @@ abstract class ResourceLoaderTestCase extends MediaWikiIntegrationTestCase {
 			// For ResourceLoaderModule
 			MainConfigNames::ResourceLoaderValidateJS => false,
 
-			// For ResourceLoaderWikiModule
-			MainConfigNames::MaxRedirects => 1,
-
 			// For ResourceLoaderSkinModule
 			MainConfigNames::Logos => false,
 			MainConfigNames::Logo => '/logo.png',