Setup: Move wgArticlePath validation to its main consumer (PathRouter)

The variable is also read in a few other places, such as to
export the value from api.php (siteinfo) and load.php (mw.config)
but those requests don't need to be held back by this extra
logic.

Alternatively, if we really want to require this for all consumption,
we should probably let PathRouter provide the value and require
consumers to use it. E.g. services->getPathRouter->getArticlePath,
or something like that.

As easy first step, I'm moving it to PathRouter, called from
WebRequest::getPathInfo which is still called on all index.php
requests for any wiki page action in any namespace (incl Special)
when the wiki uses anything other than the default 'index.php?title='
article path.

Test Plan:
* Set '$wgArticlePath = 'bla';`
* View /mediawiki/index.php/Main_Page, and observe the fatal
  error message (same as before this change).

Bug: T189966
Change-Id: Id06c2557e2addb58faeef0b6f7767a65b8de55a5

includes/PathRouter.php

@@ -165,6 +165,25 @@ class PathRouter {
 		}
 	}
 
+	/**
+	 * @internal For use by WebRequest::getPathInfo
+	 * @param string $path To be given to add()
+	 * @param string $varName Full name of configuration variable for use
+	 *  in error message and url to mediawiki.org Manual (e.g. "wgExample").
+	 * @throws FatalError If path is invalid
+	 */
+	public function validateRoute( $path, $varName ) {
+		if ( $path && !preg_match( '/^(https?:\/\/|\/)/', $path ) ) {
+			// T48998: Bail out early if path is non-absolute
+			throw new FatalError(
+				"If you use a relative URL for \$$varName, it must start " .
+				'with a slash (<code>/</code>).<br><br>See ' .
+				"<a href=\"https://www.mediawiki.org/wiki/Manual:\$$varName\">" .
+				"https://www.mediawiki.org/wiki/Manual:\$$varName</a>."
+			);
+		}
+	}
+
 	/**
 	 * Add a new path pattern to the path router with the strict option on
 	 * @see self::add

includes/Setup.php

@@ -629,18 +629,6 @@ define( 'MW_SERVICE_BOOTSTRAP_COMPLETE', 1 );
 
 MWExceptionHandler::installHandler();
 
-// T48998: Bail out early if $wgArticlePath is non-absolute
-foreach ( [ 'wgArticlePath', 'wgVariantArticlePath' ] as $varName ) {
-	if ( $$varName && !preg_match( '/^(https?:\/\/|\/)/', $$varName ) ) {
-		throw new FatalError(
-			"If you use a relative URL for \$$varName, it must start " .
-			'with a slash (<code>/</code>).<br><br>See ' .
-			"<a href=\"https://www.mediawiki.org/wiki/Manual:\$$varName\">" .
-			"https://www.mediawiki.org/wiki/Manual:\$$varName</a>."
-		);
-	}
-}
-
 if ( $wgCanonicalServer === false ) {
 	$wgCanonicalServer = wfExpandUrl( $wgServer, PROTO_HTTP );
 }

includes/WebRequest.php

@@ -137,6 +137,7 @@ class WebRequest {
 	 * inside a rewrite path.
 	 *
 	 * @return array Any query arguments found in path matches.
+	 * @throws FatalError If invalid routes are configured (T48998)
 	 */
 	public static function getPathInfo( $want = 'all' ) {
 		// PATH_INFO is mangled due to https://bugs.php.net/bug.php?id=31892
@@ -179,6 +180,7 @@ class WebRequest {
 
 			global $wgArticlePath;
 			if ( $wgArticlePath ) {
+				$router->validateRoute( $wgArticlePath, 'wgArticlePath' );
 				$router->add( $wgArticlePath );
 			}
 
@@ -190,6 +192,7 @@ class WebRequest {
 
 			global $wgVariantArticlePath;
 			if ( $wgVariantArticlePath ) {
+				$router->validateRoute( $wgVariantArticlePath, 'wgVariantArticlePath' );
 				$router->add( $wgVariantArticlePath,
 					[ 'variant' => '$2' ],
 					[ '$2' => MediaWikiServices::getInstance()->getContentLanguage()->

includes/exception/FatalError.php

@@ -19,8 +19,15 @@
  */
 
 /**
- * Exception class which takes an HTML error message, and does not
- * produce a backtrace. Replacement for OutputPage::fatalError().
+ * Abort the web request with a custom HTML string that will represent
+ * the entire response.
+ *
+ * This is not caught anywhere in MediaWiki code. It is handled through PHP's
+ * exception handler, which calls MWExceptionHandler::report.
+ *
+ * Unlike MWException, this will not provide error IDs or stack traces.
+ * It is intended for early installation and configuration problems where
+ * the exception is all the site administrator needs to know.
  *
  * @since 1.7
  * @ingroup Exception
@@ -28,6 +35,9 @@
 class FatalError extends MWException {
 
 	/**
+	 * Replace our usual detailed HTML response for uncaught exceptions,
+	 * with just the bare message as HTML.
+	 *
 	 * @return string
 	 */
 	public function getHTML() {

tests/phpunit/unit/includes/PathRouterTest.php

@@ -322,4 +322,27 @@ class PathRouterTest extends MediaWikiUnitTestCase {
 
 		$this->assertEquals( $router->parse( $path ), $expected );
 	}
+
+	public static function provideValidateRoute() {
+		yield [ 'https://test/wiki/$1' ];
+		yield [ 'http://test/wiki/$1' ];
+		yield [ '//test/wiki/$1' ];
+		yield [ '/w/index.php?title=$1' ];
+		yield [ '/w/index.php/$1' ];
+		yield [ '/wiki/$1' ];
+
+		// T48998
+		yield [ 'wiki/$1', false ];
+	}
+
+	/**
+	 * @dataProvider provideValidateRoute
+	 */
+	public function testValidateRoute( $path, $valid = true ) {
+		$router = new PathRouter;
+		if ( !$valid ) {
+			$this->setExpectedException( FatalError::class );
+		}
+		$this->assertNull( $router->validateRoute( $path, 'wgExample' ) );
+	}
 }