https://www.mediawiki.org/wiki/Special:Code/MediaWiki/112313
Revision: 112313
Author: dantman
Date: 2012-02-24 11:31:36 +0000 (Fri, 24 Feb 2012)
Log Message:
-----------
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.
Modified Paths:
--------------
trunk/phase3/includes/AutoLoader.php
trunk/phase3/includes/PathRouter.php
trunk/phase3/tests/phpunit/includes/PathRouterTest.php
Modified: trunk/phase3/includes/AutoLoader.php
===================================================================
--- trunk/phase3/includes/AutoLoader.php 2012-02-24 11:15:28 UTC (rev
112312)
+++ trunk/phase3/includes/AutoLoader.php 2012-02-24 11:31:36 UTC (rev
112313)
@@ -165,6 +165,7 @@
'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',
Modified: trunk/phase3/includes/PathRouter.php
===================================================================
--- trunk/phase3/includes/PathRouter.php 2012-02-24 11:15:28 UTC (rev
112312)
+++ trunk/phase3/includes/PathRouter.php 2012-02-24 11:31:36 UTC (rev
112313)
@@ -278,20 +278,14 @@
} 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 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
Modified: trunk/phase3/tests/phpunit/includes/PathRouterTest.php
===================================================================
--- trunk/phase3/tests/phpunit/includes/PathRouterTest.php 2012-02-24
11:15:28 UTC (rev 112312)
+++ trunk/phase3/tests/phpunit/includes/PathRouterTest.php 2012-02-24
11:31:36 UTC (rev 112313)
@@ -125,6 +125,16 @@
}
/**
+ * 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
*/
public function testWeight() {
@@ -172,12 +182,30 @@
$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();[]{}*" ) );
}
/**
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
