Followup r104274, r104676. Fix the bug that broke fr. Forgot to rawurldecode path...

Also add /u just for sanity sake.
Add new tests for url encoding, unicode, and length edge cases.

diff --git a/includes/PathRouter.php b/includes/PathRouter.php
index 32d2152..f2b3143 100644 (file)
--- a/includes/PathRouter.php
+++ b/includes/PathRouter.php
@@ -73,7 +73,7 @@ class PathRouter {
 
                foreach ( $params as $paramName => $paramData ) {
                        if ( is_string( $paramData ) ) {
-                               if ( preg_match( '/\$(\d+|key)/', $paramData ) ) {
+                               if ( preg_match( '/\$(\d+|key)/u', $paramData ) ) {
                                        $paramArrKey = 'pattern';
                                } else {
                                        // If there's no replacement use a value instead
@@ -87,7 +87,7 @@ class PathRouter {
                }
 
                foreach ( $options as $optionName => $optionData ) {
-                       if ( preg_match( '/^\$\d+$/', $optionName ) ) {
+                       if ( preg_match( '/^\$\d+$/u', $optionName ) ) {
                                if ( !is_array( $optionData ) ) {
                                        $options[$optionName] = array( $optionData );
                                }
@@ -147,10 +147,10 @@ class PathRouter {
 
                # For each level of the path
                foreach( $path as $piece ) {
-                       if ( preg_match( '/^\$(\d+|key)$/', $piece ) ) {
+                       if ( preg_match( '/^\$(\d+|key)$/u', $piece ) ) {
                                # For a piece that is only a $1 variable add 1 points of weight
                                $weight += 1;
-                       } elseif ( preg_match( '/\$(\d+|key)/', $piece ) ) {
+                       } elseif ( preg_match( '/\$(\d+|key)/u', $piece ) ) {
                                # For a piece that simply contains a $1 variable add 2 points of weight
                                $weight += 2;
                        } else {
@@ -160,7 +160,7 @@ class PathRouter {
                }
 
                foreach ( $pattern->options as $key => $option ) {
-                       if ( preg_match( '/^\$\d+$/', $key ) ) {
+                       if ( preg_match( '/^\$\d+$/u', $key ) ) {
                                # Add 0.5 for restrictions to values
                                # This way given two separate "/$2/$1" patterns the
                                # one with a limited set of $2 values will dominate
@@ -195,8 +195,8 @@ class PathRouter {
 
        protected static function extractTitle( $path, $pattern ) {
                $regexp = preg_quote( $pattern->path, '#' );
-               $regexp = preg_replace( '#\\\\\$1#', '(?P<par1>.*)', $regexp );
-               $regexp = preg_replace( '#\\\\\$(\d+)#', '(?P<par$1>.+?)', $regexp );
+               $regexp = preg_replace( '#\\\\\$1#u', '(?P<par1>.*)', $regexp );
+               $regexp = preg_replace( '#\\\\\$(\d+)#u', '(?P<par$1>.+?)', $regexp );
                $regexp = "#^{$regexp}$#";
 
                $matches = array();
@@ -204,9 +204,9 @@ class PathRouter {
 
                if ( preg_match( $regexp, $path, $m ) ) {
                        foreach ( $pattern->options as $key => $option ) {
-                               if ( preg_match( '/^\$\d+$/', $key ) ) {
+                               if ( preg_match( '/^\$\d+$/u', $key ) ) {
                                        $n = intval( substr( $key, 1 ) );
-                                       $value = $m["par{$n}"];
+                                       $value = rawurldecode( $m["par{$n}"] );
                                        if ( !in_array( $value, $option ) ) {
                                                return null;
                                        }
@@ -214,9 +214,9 @@ class PathRouter {
                        }
 
                        foreach ( $m as $matchKey => $matchValue ) {
-                               if ( preg_match( '/^par\d+$/', $matchKey ) ) {
+                               if ( preg_match( '/^par\d+$/u', $matchKey ) ) {
                                        $n = intval( substr( $matchKey, 3 ) );
-                                       $data['$'.$n] = $matchValue;
+                                       $data['$'.$n] = rawurldecode( $matchValue );
                                }
                        }
                        if ( isset( $pattern->key ) ) {
@@ -225,7 +225,7 @@ class PathRouter {
 
                        foreach ( $pattern->params as $paramName => $paramData ) {
                                $value = null;
-                               if ( preg_match( '/^data:/', $paramName ) ) {
+                               if ( preg_match( '/^data:/u', $paramName ) ) {
                                        $isData = true;
                                        $key = substr( $paramName, 5 );
                                } else {
@@ -238,15 +238,15 @@ class PathRouter {
                                } elseif ( isset( $paramData['pattern'] ) ) {
                                        $value = $paramData['pattern'];
                                        foreach ( $m as $matchKey => $matchValue ) {
-                                               if ( preg_match( '/^par\d+$/', $matchKey ) ) {
+                                               if ( preg_match( '/^par\d+$/u', $matchKey ) ) {
                                                        $n = intval( substr( $matchKey, 3 ) );
-                                                       $value = str_replace( '$' . $n, $matchValue, $value );
+                                                       $value = str_replace( '$' . $n, rawurldecode( $matchValue ), $value );
                                                }
                                        }
                                        if ( isset( $pattern->key ) ) {
                                                $value = str_replace( '$key', $pattern->key, $value );
                                        }
-                                       if ( preg_match( '/\$(\d+|key)/', $value ) ) {
+                                       if ( preg_match( '/\$(\d+|key)/u', $value ) ) {
                                                // Still contains $# or $key patterns after replacement
                                                // Seams like we don't have all the data, abort
                                                return null;

diff --git a/tests/phpunit/includes/PathRouterTest.php b/tests/phpunit/includes/PathRouterTest.php
index 07fa65d..623804a 100644 (file)
--- a/tests/phpunit/includes/PathRouterTest.php
+++ b/tests/phpunit/includes/PathRouterTest.php
@@ -5,13 +5,17 @@
 
 class PathRouterTest extends MediaWikiTestCase {
 
+       public function setUp() {
+               $router = new PathRouter;
+               $router->add("/wiki/$1");
+               $this->basicRouter = $router;
+       }
+
        /**
         * Test basic path parsing
         */
        public function testBasic() {
-               $router = new PathRouter;
-               $router->add("/$1");
-               $matches = $router->parse( "/Foo" );
+               $matches = $this->basicRouter->parse( "/wiki/Foo" );
                $this->assertEquals( $matches, array( 'title' => "Foo" ) );
        }
 
@@ -111,7 +115,7 @@ class PathRouterTest extends MediaWikiTestCase {
                        array( 'a' => 'b', 'data:foo' => 'bar' ),
                        array( 'callback' => array( $this, 'callbackForTest' ) )
                );
-               $matches = $router->parse( '/Foo');
+               $matches = $router->parse( '/Foo' );
                $this->assertEquals( $matches, array(
                        'title' => "Foo",
                        'x' => 'Foo',
@@ -152,4 +156,50 @@ class PathRouterTest extends MediaWikiTestCase {
                }
        }
 
+       /**
+        * Make sure the router handles titles like Special:Recentchanges correctly
+        */
+       public function testSpecial() {
+               $matches = $this->basicRouter->parse( "/wiki/Special:Recentchanges" );
+               $this->assertEquals( $matches, array( 'title' => "Special:Recentchanges" ) );
+       }
+
+       /**
+        * Make sure the router decodes urlencoding properly
+        */
+       public function testUrlencoding() {
+               $matches = $this->basicRouter->parse( "/wiki/Title_With%20Space" );
+               $this->assertEquals( $matches, array( 'title' => "Title_With Space" ) );
+       }
+
+       /**
+        * 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()" ) );
+       }
+
+       /**
+        * Make sure the router handles unicode characters correctly
+        * @depends testSpecial
+        * @depends testUrlencoding
+        * @depends testCharacters
+        */
+       public function testUnicode() {
+               $matches = $this->basicRouter->parse( "/wiki/Spécial:Modifications_récentes" );
+               $this->assertEquals( $matches, array( 'title' => "Spécial:Modifications_récentes" ) );
+
+               $matches = $this->basicRouter->parse( "/wiki/Sp%C3%A9cial:Modifications_r%C3%A9centes" );
+               $this->assertEquals( $matches, array( 'title' => "Spécial:Modifications_récentes" ) );
+       }
+
+       /**
+        * Ensure the router doesn't choke on long paths.
+        */
+       public function testLength() {
+               $matches = $this->basicRouter->parse( "/wiki/Lorem_ipsum_dolor_sit_amet,_consectetur_adipisicing_elit,_sed_do_eiusmod_tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum." );
+               $this->assertEquals( $matches, array( 'title' => "Lorem_ipsum_dolor_sit_amet,_consectetur_adipisicing_elit,_sed_do_eiusmod_tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum." ) );
+       }
+
 }