From d6cf8c57b4f5b0fdab7010a2e58a4baaec56f42f Mon Sep 17 00:00:00 2001 From: Daniel Friesen Date: Wed, 30 Nov 2011 15:09:08 +0000 Subject: [PATCH] Followup r104274, r104676. Fix the bug that broke fr. Forgot to rawurldecode path contents. Also add /u just for sanity sake. Add new tests for url encoding, unicode, and length edge cases. --- includes/PathRouter.php | 30 ++++++------ tests/phpunit/includes/PathRouterTest.php | 58 +++++++++++++++++++++-- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/includes/PathRouter.php b/includes/PathRouter.php index 32d2152257..f2b3143dd3 100644 --- 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.*)', $regexp ); - $regexp = preg_replace( '#\\\\\$(\d+)#', '(?P.+?)', $regexp ); + $regexp = preg_replace( '#\\\\\$1#u', '(?P.*)', $regexp ); + $regexp = preg_replace( '#\\\\\$(\d+)#u', '(?P.+?)', $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 07fa65d3c6..623804af49 100644 --- 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." ) ); + } + } -- 2.20.1