From d0a51ee5a4c08b43681a94ee37df66c3908b8669 Mon Sep 17 00:00:00 2001
From: Daniel Friesen <dantman@users.mediawiki.org>
Date: Fri, 24 Feb 2012 11:31:36 +0000
Subject: [PATCH] Fix bug 34684 in my PathRouter code: - Update the tests to
 test extra characters and patterns like like \\ and $1 - Also update the
 tests to make sure that matches that don't have enough data to work fail -
 Replace the str_replace and preg_match based code with code based on
 preg_replace_callback.

---
 includes/AutoLoader.php                   |  1 +
 includes/PathRouter.php                   | 56 ++++++++++++++++++-----
 tests/phpunit/includes/PathRouterTest.php | 32 ++++++++++++-
 3 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 4540c7958f..0df2dcba8b 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -165,6 +165,7 @@ $wgAutoloadLocalClasses = array(
 	'Pager' => 'includes/Pager.php',
 	'PasswordError' => 'includes/User.php',
 	'PathRouter' => 'includes/PathRouter.php',
+	'PathRouterPatternReplacer' => 'includes/PathRouter.php',
 	'PermissionsError' => 'includes/Exception.php',
 	'PhpHttpRequest' => 'includes/HttpFunctions.php',
 	'PoolCounter' => 'includes/PoolCounter.php',
diff --git a/includes/PathRouter.php b/includes/PathRouter.php
index 53ddd425e0..29df4b4ae5 100644
--- a/includes/PathRouter.php
+++ b/includes/PathRouter.php
@@ -278,20 +278,14 @@ class PathRouter {
 				} elseif ( isset( $paramData['pattern'] ) ) {
 					// For patterns we have to make value replacements on the string
 					$value = $paramData['pattern'];
-					// For each $# match replace any $# within the value
-					foreach ( $m as $matchKey => $matchValue ) {
-						if ( preg_match( '/^par\d+$/u', $matchKey ) ) {
-							$n = intval( substr( $matchKey, 3 ) );
-							$value = str_replace( '$' . $n, rawurldecode( $matchValue ), $value );
-						}
-					}
-					// If a key was set replace any $key within the value
+					$replacer = new PathRouterPatternReplacer;
+					$replacer->params = $m;
 					if ( isset( $pattern->key ) ) {
-						$value = str_replace( '$key', $pattern->key, $value );
+						$replacer->key = $pattern->key;
 					}
-					if ( preg_match( '/\$(\d+|key)/u', $value ) ) {
-						// Still contains $# or $key patterns after replacement
-						// Seams like we don't have all the data, abort
+					$value = $replacer->replace( $value );
+					if ( $value === false ) {
+						// Pattern required data that wasn't available, abort
 						return null;
 					}
 				}
@@ -317,3 +311,41 @@ class PathRouter {
 	}
 
 }
+
+class PathRouterPatternReplacer {
+
+	public $key, $params, $error;
+
+	/**
+	 * Replace keys inside path router patterns with text.
+	 * We do this inside of a replacement callback because after replacement we can't tell the
+	 * difference between a $1 that was not replaced and a $1 that was part of
+	 * the content a $1 was replaced with.
+	 */
+	public function replace( $value ) {
+		$this->error = false;
+		$value = preg_replace_callback( '/\$(\d+|key)/u', array( $this, 'callback' ), $value );
+		if ( $this->error ) {
+			return false;
+		}
+		return $value;
+	}
+
+	protected function callback( $m ) {
+		if ( $m[1] == "key" ) {
+			if ( is_null( $this->key ) ) {
+				$this->error = true;
+				return '';
+			}
+			return $this->key;
+		} else {
+			$d = $m[1];
+			if ( !isset( $this->params["par$d"] ) ) {
+				$this->error = true;
+				return '';
+			}
+			return rawurldecode( $this->params["par$d"] );
+		}
+	}
+
+}
\ No newline at end of file
diff --git a/tests/phpunit/includes/PathRouterTest.php b/tests/phpunit/includes/PathRouterTest.php
index 3b29ef8481..f62745841d 100644
--- a/tests/phpunit/includes/PathRouterTest.php
+++ b/tests/phpunit/includes/PathRouterTest.php
@@ -124,6 +124,16 @@ class PathRouterTest extends MediaWikiTestCase {
 		) );
 	}
 
+	/**
+	 * Test to ensure that matches are not made if a parameter expects nonexistent input
+	 */
+	public function testFail() {
+		$router = new PathRouter;
+		$router->add( "/wiki/$1", array( 'title' => "$1$2" ) );
+		$matches = $router->parse( "/wiki/A" );
+		$this->assertEquals( array(), $matches );
+	}
+
 	/**
 	 * Test to ensure weight of paths is handled correctly
 	 */
@@ -172,12 +182,30 @@ class PathRouterTest extends MediaWikiTestCase {
 		$this->assertEquals( $matches, array( 'title' => "Title_With Space" ) );
 	}
 
+	public function dataRegexpChars() {
+		return array(
+			array( "$" ),
+			array( "$1" ),
+			array( "\\" ),
+			array( "\\$1" ),
+		);
+	}
+
+	/**
+	 * Make sure the router doesn't break on special characters like $ used in regexp replacements
+	 * @dataProvider dataRegexpChars
+	 */
+	public function testRegexpChars( $char ) {
+		$matches = $this->basicRouter->parse( "/wiki/$char" );
+		$this->assertEquals( $matches, array( 'title' => "$char" ) );
+	}
+
 	/**
 	 * Make sure the router handles characters like +&() properly
 	 */
 	public function testCharacters() {
-		$matches = $this->basicRouter->parse( "/wiki/Plus+And&Stuff()" );
-		$this->assertEquals( $matches, array( 'title' => "Plus+And&Stuff()" ) );
+		$matches = $this->basicRouter->parse( "/wiki/Plus+And&Dollar\\Stuff();[]{}*" );
+		$this->assertEquals( $matches, array( 'title' => "Plus+And&Dollar\\Stuff();[]{}*" ) );
 	}
 
 	/**
-- 
2.20.1