r104274 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104273‎ | r104274 | r104275 >
Date:16:29, 26 November 2011
Author:dantman
Status:reverted (Comments)
Tags:
Comment:
Implement path routing code.
- Makes extending paths with extensions simpler.
- Should fix bug 32621 by parsing paths based on pattern weight rather than pattern order.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/PathRouter.php (added) (history)
  • /trunk/phase3/includes/WebRequest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/PathRouterTest.php (added) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -2186,14 +2186,8 @@
21872187 $redirect: whether the page is a redirect
21882188 $skin: Skin object
21892189
2190 -'WebRequestGetPathInfoRequestURI': while extracting path info from REQUEST_URI.
2191 - Allows an extension to extend the extraction of titles from paths.
2192 - Implementing hooks should follow the pattern used in core:
2193 - * Use the `$matches = WebRequest::extractTitle` pattern
2194 - * Ensure that you test `if ( !$matches ) {` before you try extracting a title
2195 - from the path so that you don't override an already found match.
2196 -$path: The request path to extract a title from.
2197 -&$matches: The array to apply matches to.
 2190+'WebRequestPathInfoRouter': While building the PathRouter to parse the REQUEST_URI.
 2191+$router: The PathRouter instance
21982192
21992193 'WikiExporter::dumpStableQuery': Get the SELECT query for "stable" revisions
22002194 dumps
Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -93,6 +93,8 @@
9494 * (bug 24037) Add byte length of revision to Special:Contributions.
9595 * (bug 1672) Added $wgDisableUploadScriptChecks to allow uploading of files
9696 containing HTML or JS. DISABLING THESE CHECKS IS VERY DANGEROUS.
 97+* New path mappings can be added using the WebRequestPathInfoRouter hook
 98+ and adding paths to the PathRouter.
9799
98100 === Bug fixes in 1.19 ===
99101 * $wgUploadNavigationUrl should be used for file redlinks if.
Index: trunk/phase3/tests/phpunit/includes/PathRouterTest.php
@@ -0,0 +1,155 @@
 2+<?php
 3+/**
 4+ * Tests for the PathRouter parsing
 5+ */
 6+
 7+class PathRouterTest extends MediaWikiTestCase {
 8+
 9+ /**
 10+ * Test basic path parsing
 11+ */
 12+ public function testBasic() {
 13+ $router = new PathRouter;
 14+ $router->add("/$1");
 15+ $matches = $router->parse( "/Foo" );
 16+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 17+ }
 18+
 19+ /**
 20+ * Test loose path auto-$1
 21+ */
 22+ public function testLoose() {
 23+ $router = new PathRouter;
 24+ $router->add("/"); # Should be the same as "/$1"
 25+ $matches = $router->parse( "/Foo" );
 26+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 27+
 28+ $router = new PathRouter;
 29+ $router->add("/wiki"); # Should be the same as /wiki/$1
 30+ $matches = $router->parse( "/wiki/Foo" );
 31+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 32+
 33+ $router = new PathRouter;
 34+ $router->add("/wiki/"); # Should be the same as /wiki/$1
 35+ $matches = $router->parse( "/wiki/Foo" );
 36+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 37+ }
 38+
 39+ /**
 40+ * Test to ensure that path is based on specifity, not order
 41+ */
 42+ public function testOrder() {
 43+ $router = new PathRouter;
 44+ $router->add("/$1");
 45+ $router->add("/a/$1");
 46+ $router->add("/b/$1");
 47+ $matches = $router->parse( "/a/Foo" );
 48+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 49+
 50+ $router = new PathRouter;
 51+ $router->add("/b/$1");
 52+ $router->add("/a/$1");
 53+ $router->add("/$1");
 54+ $matches = $router->parse( "/a/Foo" );
 55+ $this->assertEquals( $matches, array( 'title' => "Foo" ) );
 56+ }
 57+
 58+ /**
 59+ * Test the handling of key based arrays with a url parameter
 60+ */
 61+ public function testKeyParameter() {
 62+ $router = new PathRouter;
 63+ $router->add( array( 'edit' => "/edit/$1" ), array( 'action' => '$key' ) );
 64+ $matches = $router->parse( "/edit/Foo" );
 65+ $this->assertEquals( $matches, array( 'title' => "Foo", 'action' => 'edit' ) );
 66+ }
 67+
 68+ /**
 69+ * Test the handling of $2 inside paths
 70+ */
 71+ public function testAdditionalParameter() {
 72+ // Basic $2
 73+ $router = new PathRouter;
 74+ $router->add( '/$2/$1', array( 'test' => '$2' ) );
 75+ $matches = $router->parse( "/asdf/Foo" );
 76+ $this->assertEquals( $matches, array( 'title' => "Foo", 'test' => 'asdf' ) );
 77+ }
 78+
 79+ /**
 80+ * Test additional restricted value parameter
 81+ */
 82+ public function testRestrictedValue() {
 83+ $router = new PathRouter;
 84+ $router->add( '/$2/$1',
 85+ array( 'test' => '$2' ),
 86+ array( '$2' => array( 'a', 'b' ) )
 87+ );
 88+ $router->add( '/$2/$1',
 89+ array( 'test2' => '$2' ),
 90+ array( '$2' => 'c' )
 91+ );
 92+ $router->add( '/$1' );
 93+
 94+ $matches = $router->parse( "/asdf/Foo" );
 95+ $this->assertEquals( $matches, array( 'title' => "asdf/Foo" ) );
 96+
 97+ $matches = $router->parse( "/a/Foo" );
 98+ $this->assertEquals( $matches, array( 'title' => "Foo", 'test' => 'a' ) );
 99+
 100+ $matches = $router->parse( "/c/Foo" );
 101+ $this->assertEquals( $matches, array( 'title' => "Foo", 'test2' => 'c' ) );
 102+ }
 103+
 104+ public function callbackForTest( &$matches, $data ) {
 105+ $matches['x'] = $data['$1'];
 106+ $matches['foo'] = $data['foo'];
 107+ }
 108+
 109+ public function testCallback() {
 110+ $router = new PathRouter;
 111+ $router->add( "/$1",
 112+ array( 'a' => 'b', 'data:foo' => 'bar' ),
 113+ array( 'callback' => array( $this, 'callbackForTest' ) )
 114+ );
 115+ $matches = $router->parse( '/Foo');
 116+ $this->assertEquals( $matches, array(
 117+ 'title' => "Foo",
 118+ 'x' => 'Foo',
 119+ 'a' => 'b',
 120+ 'foo' => 'bar'
 121+ ) );
 122+ }
 123+
 124+ /**
 125+ * Test to ensure weight of paths is handled correctly
 126+ */
 127+ public function testWeight() {
 128+ $router = new PathRouter;
 129+ $router->addStrict( "/Bar", array( 'ping' => 'pong' ) );
 130+ $router->add( "/asdf-$1", array( 'title' => 'qwerty-$1' ) );
 131+ $router->add( "/$1" );
 132+ $router->add( "/qwerty-$1", array( 'title' => 'asdf-$1' ) );
 133+ $router->addStrict( "/Baz", array( 'marco' => 'polo' ) );
 134+ $router->add( "/a/$1" );
 135+ $router->add( "/asdf/$1" );
 136+ $router->add( "/$2/$1", array( 'unrestricted' => '$2' ) );
 137+ $router->add( array( 'qwerty' => "/qwerty/$1" ), array( 'qwerty' => '$key' ) );
 138+ $router->add( "/$2/$1", array( 'restricted-to-y' => '$2' ), array( '$2' => 'y' ) );
 139+
 140+ foreach( array(
 141+ "/Foo" => array( 'title' => "Foo" ),
 142+ "/Bar" => array( 'ping' => 'pong' ),
 143+ "/Baz" => array( 'marco' => 'polo' ),
 144+ "/asdf-foo" => array( 'title' => "qwerty-foo" ),
 145+ "/qwerty-bar" => array( 'title' => "asdf-bar" ),
 146+ "/a/Foo" => array( 'title' => "Foo" ),
 147+ "/asdf/Foo" => array( 'title' => "Foo" ),
 148+ "/qwerty/Foo" => array( 'title' => "Foo", 'qwerty' => 'qwerty' ),
 149+ "/baz/Foo" => array( 'title' => "Foo", 'unrestricted' => 'baz' ),
 150+ "/y/Foo" => array( 'title' => "Foo", 'restricted-to-y' => 'y' ),
 151+ ) as $path => $result ) {
 152+ $this->assertEquals( $router->parse( $path ), $result );
 153+ }
 154+ }
 155+
 156+}
Index: trunk/phase3/includes/WebRequest.php
@@ -93,40 +93,49 @@
9494 // Abort to keep from breaking...
9595 return $matches;
9696 }
 97+
 98+ $router = new PathRouter;
 99+
97100 // Raw PATH_INFO style
98 - $matches = self::extractTitle( $path, "$wgScript/$1" );
 101+ $router->add( "$wgScript/$1" );
99102
100 - if( !$matches
101 - && isset( $_SERVER['SCRIPT_NAME'] )
 103+ if( isset( $_SERVER['SCRIPT_NAME'] )
102104 && preg_match( '/\.php5?/', $_SERVER['SCRIPT_NAME'] ) )
103105 {
104106 # Check for SCRIPT_NAME, we handle index.php explicitly
105107 # But we do have some other .php files such as img_auth.php
106108 # Don't let root article paths clober the parsing for them
107 - $matches = self::extractTitle( $path, $_SERVER['SCRIPT_NAME'] . "/$1" );
 109+ $router->add( $_SERVER['SCRIPT_NAME'] . "/$1" );
108110 }
109111
110112 global $wgArticlePath;
111 - if( !$matches && $wgArticlePath ) {
112 - $matches = self::extractTitle( $path, $wgArticlePath );
 113+ if( $wgArticlePath ) {
 114+ $router->add( $wgArticlePath );
113115 }
114116
115117 global $wgActionPaths;
116 - if( !$matches && $wgActionPaths ) {
117 - $matches = self::extractTitle( $path, $wgActionPaths, 'action' );
 118+ if( $wgActionPaths ) {
 119+ $router->add( $wgActionPaths, array( 'action' => '$key' ) );
118120 }
119121
120122 global $wgVariantArticlePath, $wgContLang;
121 - if( !$matches && $wgVariantArticlePath ) {
122 - $variantPaths = array();
 123+ if( $wgVariantArticlePath ) {
 124+ /*$variantPaths = array();
123125 foreach( $wgContLang->getVariants() as $variant ) {
124126 $variantPaths[$variant] =
125127 str_replace( '$2', $variant, $wgVariantArticlePath );
126128 }
127 - $matches = self::extractTitle( $path, $variantPaths, 'variant' );
 129+ $router->add( $variantPaths, array( 'parameter' => 'variant' ) );*/
 130+ // Maybe actually this?
 131+ $router->add( $wgVariantArticlePath,
 132+ array( 'variant' => '$2'),
 133+ array( '$2' => $wgContLang->getVariants() )
 134+ );
128135 }
129136
130 - wfRunHooks( 'WebRequestGetPathInfoRequestURI', array( $path, &$matches ) );
 137+ wfRunHooks( 'WebRequestPathInfoRouter', array( $router ) );
 138+
 139+ $matches = $router->parse( $path );
131140 }
132141 } elseif ( isset( $_SERVER['ORIG_PATH_INFO'] ) && $_SERVER['ORIG_PATH_INFO'] != '' ) {
133142 // Mangled PATH_INFO
Index: trunk/phase3/includes/PathRouter.php
@@ -0,0 +1,269 @@
 2+<?php
 3+/**
 4+ * PathRouter class.
 5+ * This class can take patterns such as /wiki/$1 and use them to
 6+ * parse query parameters out of REQUEST_URI paths.
 7+ *
 8+ * $router->add( "/wiki/$1" );
 9+ * - Matches /wiki/Foo style urls and extracts the title
 10+ * $router->add( array( 'edit' => "/edit/$1" ), array( 'action' => '$key' ) );
 11+ * - Matches /edit/Foo style urls and sets action=edit
 12+ * $router->add( '/$2/$1',
 13+ * array( 'variant' => '$2' ),
 14+ * array( '$2' => array( 'zh-hant', 'zh-hans' )
 15+ * );
 16+ * - Matches /zh-hant/Foo or /zh-hans/Foo
 17+ * $router->addStrict( "/foo/Bar", array( 'title' => 'Baz' ) );
 18+ * - Matches /foo/Bar explicitly and uses "Baz" as the title
 19+ * $router->add( '/help/$1', array( 'title' => 'Help:$1' ) );
 20+ * - Matches /help/Foo with "Help:Foo" as the title
 21+ * $router->add( '/help/$1', array( 'title' => 'Help:$1' ) );
 22+ * - Matches
 23+ * $router->add( '/$1', array( 'foo' => array( 'value' => 'bar$2' ) );
 24+ * - Matches /Foo and sets 'foo=bar$2' without $2 being replaced
 25+ * $router->add( '/$1', array( 'data:foo' => 'bar' ), array( 'callback' => 'functionname' ) );
 26+ * - Matches /Foo, adds the key 'foo' with the value 'bar' to the data array
 27+ * and calls functionname( &$matches, $data );
 28+ *
 29+ * Path patterns:
 30+ * - Paths may contain $# patterns such as $1, $2, etc...
 31+ * - $1 will match 0 or more while the rest will match 1 or more
 32+ * - Unless you use addStrict "/wiki" and "/wiki/" will be expanded to "/wiki/$1"
 33+ *
 34+ * Params:
 35+ * - In a pattern $1, $2, etc... will be replaced with the relevant contents
 36+ * - If you used a keyed array as a path pattern $key will be replaced with the relevant contents
 37+ * - The default behavior is equivalent to `array( 'title' => '$1' )`, if you don't want the title parameter you can explicitly use `array( 'title' => false )`
 38+ * - You can specify a value that won't have replacements in it using `'foo' => array( 'value' => 'bar' );`
 39+ *
 40+ * Options:
 41+ * - The option keys $1, $2, etc... can be specified to restrict the possible values of that variable.
 42+ * A string can be used for a single value, or an array for multiple.
 43+ * - When the option key 'strict' is set (Using addStrict is simpler than doing this directly)
 44+ * the path won't have $1 implicitly added to it.
 45+ * - The option key 'callback' can specify a callback that will be run when a path is matched.
 46+ * The callback will have the arguments ( &$matches, $data ) and the matches array can be modified.
 47+ *
 48+ * @since 1.19
 49+ * @author Daniel Friesen
 50+ */
 51+class PathRouter {
 52+
 53+ protected function doAdd( $path, $params, $options, $key = null ) {
 54+ if ( $path[0] !== '/' ) {
 55+ $path = '/' . $path;
 56+ }
 57+
 58+ if ( !isset( $options['strict'] ) || !$options['strict'] ) {
 59+ // Unless this is a strict path make sure that the path has a $1
 60+ if ( strpos( $path, '$1' ) === false ) {
 61+ if ( substr( $path, -1 ) !== '/' ) {
 62+ $path .= '/';
 63+ }
 64+ $path .= '$1';
 65+ }
 66+ }
 67+
 68+ if ( !isset( $params['title'] ) && strpos( $path, '$1' ) !== false ) {
 69+ $params['title'] = '$1';
 70+ }
 71+ if ( isset( $params['title'] ) && $params['title'] === false ) {
 72+ unset( $params['title'] );
 73+ }
 74+
 75+ foreach ( $params as $paramName => $paramData ) {
 76+ if ( is_string( $paramData ) ) {
 77+ if ( preg_match( '/\$(\d+|key)/', $paramData ) ) {
 78+ $paramArrKey = 'pattern';
 79+ } else {
 80+ // If there's no replacement use a value instead
 81+ // of a pattern for a little more efficiency
 82+ $paramArrKey = 'value';
 83+ }
 84+ $params[$paramName] = array(
 85+ $paramArrKey => $paramData
 86+ );
 87+ }
 88+ }
 89+
 90+ foreach ( $options as $optionName => $optionData ) {
 91+ if ( preg_match( '/^\$\d+$/', $optionName ) ) {
 92+ if ( !is_array( $optionData ) ) {
 93+ $options[$optionName] = array( $optionData );
 94+ }
 95+ }
 96+ }
 97+
 98+ $pattern = (object)array(
 99+ 'path' => $path,
 100+ 'params' => $params,
 101+ 'options' => $options,
 102+ 'key' => $key,
 103+ );
 104+ $pattern->weight = self::makeWeight( $pattern );
 105+ $this->patterns[] = $pattern;
 106+ }
 107+
 108+ /**
 109+ * Add a new path pattern to the path router
 110+ *
 111+ * @param $path The path pattern to add
 112+ * @param $params The params for this path pattern
 113+ * @param $options The options for this path pattern
 114+ */
 115+ public function add( $path, $params = array(), $options = array() ) {
 116+ if ( is_array( $path ) ) {
 117+ foreach ( $path as $key => $onePath ) {
 118+ $this->doAdd( $onePath, $params, $options, $key );
 119+ }
 120+ } else {
 121+ $this->doAdd( $path, $params, $options );
 122+ }
 123+ }
 124+
 125+ /**
 126+ * Add a new path pattern to the path router with the strict option on
 127+ * @see self::add
 128+ */
 129+ public function addStrict( $path, $params = array(), $options = array() ) {
 130+ $options['strict'] = true;
 131+ $this->add( $path, $params, $options );
 132+ }
 133+
 134+ protected function sortByWeight() {
 135+ $weights = array();
 136+ foreach( $this->patterns as $key => $pattern ) {
 137+ $weights[$key] = $pattern->weight;
 138+ }
 139+ array_multisort( $weights, SORT_DESC, SORT_NUMERIC, $this->patterns );
 140+ }
 141+
 142+ public static function makeWeight( $pattern ) {
 143+ # Start with a weight of 0
 144+ $weight = 0;
 145+
 146+ // Explode the path to work with
 147+ $path = explode( '/', $pattern->path );
 148+
 149+ # For each level of the path
 150+ foreach( $path as $piece ) {
 151+ if ( preg_match( '/^\$(\d+|key)$/', $piece ) ) {
 152+ # For a piece that is only a $1 variable add 1 points of weight
 153+ $weight += 1;
 154+ } elseif ( preg_match( '/\$(\d+|key)/', $piece ) ) {
 155+ # For a piece that simply contains a $1 variable add 2 points of weight
 156+ $weight += 2;
 157+ } else {
 158+ # For a solid piece add a full 3 points of weight
 159+ $weight += 3;
 160+ }
 161+ }
 162+
 163+ foreach ( $pattern->options as $key => $option ) {
 164+ if ( preg_match( '/^\$\d+$/', $key ) ) {
 165+ # Add 0.5 for restrictions to values
 166+ # This way given two separate "/$2/$1" patterns the
 167+ # one with a limited set of $2 values will dominate
 168+ # the one that'll match more loosely
 169+ $weight += 0.5;
 170+ }
 171+ }
 172+
 173+ return $weight;
 174+ }
 175+
 176+ /**
 177+ * Parse a path and return the query matches for the path
 178+ *
 179+ * @param $path The path to parse
 180+ * @return Array The array of matches for the path
 181+ */
 182+ public function parse( $path ) {
 183+ $this->sortByWeight();
 184+
 185+ $matches = null;
 186+
 187+ foreach ( $this->patterns as $pattern ) {
 188+ $matches = self::extractTitle( $path, $pattern );
 189+ if ( !is_null( $matches ) ) {
 190+ break;
 191+ }
 192+ }
 193+
 194+ return is_null( $matches ) ? array() : $matches;
 195+ }
 196+
 197+ protected static function extractTitle( $path, $pattern ) {
 198+ $regexp = preg_quote( $pattern->path, '#' );
 199+ $regexp = preg_replace( '#\\\\\$1#', '(?P<par1>.*)', $regexp );
 200+ $regexp = preg_replace( '#\\\\\$(\d+)#', '(?P<par$1>.+?)', $regexp );
 201+ $regexp = "#^{$regexp}$#";
 202+
 203+ $matches = array();
 204+ $data = array();
 205+
 206+ if ( preg_match( $regexp, $path, $m ) ) {
 207+ foreach ( $pattern->options as $key => $option ) {
 208+ if ( preg_match( '/^\$\d+$/', $key ) ) {
 209+ $n = intval( substr( $key, 1 ) );
 210+ $value = $m["par{$n}"];
 211+ if ( !in_array( $value, $option ) ) {
 212+ return null;
 213+ }
 214+ }
 215+ }
 216+
 217+ foreach ( $m as $matchKey => $matchValue ) {
 218+ if ( preg_match( '/^par\d+$/', $matchKey ) ) {
 219+ $n = intval( substr( $matchKey, 3 ) );
 220+ $data['$'.$n] = $matchValue;
 221+ }
 222+ }
 223+
 224+ foreach ( $pattern->params as $paramName => $paramData ) {
 225+ $value = null;
 226+ if ( preg_match( '/^data:/', $paramName ) ) {
 227+ $isData = true;
 228+ $key = substr( $paramName, 5 );
 229+ } else {
 230+ $isData = false;
 231+ $key = $paramName;
 232+ }
 233+
 234+ if ( isset( $paramData['value'] ) ) {
 235+ $value = $paramData['value'];
 236+ } elseif ( isset( $paramData['pattern'] ) ) {
 237+ $value = $paramData['pattern'];
 238+ foreach ( $m as $matchKey => $matchValue ) {
 239+ if ( preg_match( '/^par\d+$/', $matchKey ) ) {
 240+ $n = intval( substr( $matchKey, 3 ) );
 241+ $value = str_replace( '$' . $n, $matchValue, $value );
 242+ }
 243+ }
 244+ if ( isset( $pattern->key ) ) {
 245+ $value = str_replace( '$key', $pattern->key, $value );
 246+ }
 247+ if ( preg_match( '/\$(\d+|key)/', $value ) ) {
 248+ // Still contains $# or $key patterns after replacement
 249+ // Seams like we don't have all the data, abort
 250+ return null;
 251+ }
 252+ }
 253+
 254+ if ( $isData ) {
 255+ $data[$key] = $value;
 256+ } else {
 257+ $matches[$key] = $value;
 258+ }
 259+ }
 260+
 261+ if ( isset( $pattern->options['callback'] ) ) {
 262+ call_user_func_array( $pattern->options['callback'], array( &$matches, $data ) );
 263+ }
 264+ } else {
 265+ return null;
 266+ }
 267+ return $matches;
 268+ }
 269+
 270+}
Index: trunk/phase3/includes/AutoLoader.php
@@ -163,6 +163,7 @@
164164 'PageQueryPage' => 'includes/PageQueryPage.php',
165165 'Pager' => 'includes/Pager.php',
166166 'PasswordError' => 'includes/User.php',
 167+ 'PathRouter' => 'includes/PathRouter.php',
167168 'PermissionsError' => 'includes/Exception.php',
168169 'PhpHttpRequest' => 'includes/HttpFunctions.php',
169170 'PoolCounter' => 'includes/PoolCounter.php',

Follow-up revisions

RevisionCommit summaryAuthorDate
r104284Fix svn:eol-style native for r104274reedy17:27, 26 November 2011
r104285Followup r104274; Add '$key' to the $data array as well.dantman17:43, 26 November 2011
r104474Revert r104274, r104284, r104285 -- breaks special pages on non-englishbrion19:55, 28 November 2011
r104676Recommit PathRouter code from r104274, r104284, r104285 without the WebReques...dantman13:37, 30 November 2011
r104688Followup r104274, r104676. Fix the bug that broke fr. Forgot to rawurldecode ...dantman15:09, 30 November 2011

Comments

#Comment by Reedy (talk | contribs)   22:42, 26 November 2011

RELEASE-NOTES should probably suggest that the "WebRequestGetPathInfoRequestURI" hook has been removed

#Comment by Dantman (talk | contribs)   05:05, 27 November 2011

WebRequestGetPathInfoRequestURI was introduced in 1.19 (r94373), and removed in 1.19. As far as RELEASE-NOTES is concerned that hook never existed.

#Comment by Reedy (talk | contribs)   14:43, 27 November 2011

Ah, ok. Just making sure :)

#Comment by Brion VIBBER (talk | contribs)   19:46, 28 November 2011

Causes "Bad title" errors on all Special: pages when English is not the content language.

Steps to repro:

  1. set $wgLanguageCode = 'fr';
  2. navigate to Special:Blankpage
  3. endure the terror
#Comment by Dantman (talk | contribs)   15:15, 30 November 2011

Erm... I figured out the bug.

It had nothing to do directly with fr or unicode. I simply forgot to rawurldecode the stuff before putting it into $matches. So /wiki/Main%20Page would end up as array( 'title' => "Main%20Page" ) instead of array( 'title' => "Main Page" ) and the % would trigger badtitle. And of course fr uses unicode, so that triggers urlencoding which triggers the issue indirectly.

Fixed, added tests for more edge cases I could think of, and re-added.

If you can think of any other edge cases, putting them into the test suite would be good.

#Comment by Krinkle (talk | contribs)   06:02, 5 March 2012

See also bug 34684

Status & tagging log