r63228 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r63227‎ | r63228 | r63229 >
Date:22:01, 3 March 2010
Author:platonides
Status:ok (Comments)
Tags:
Comment:
(Bug 22709) IIS 7 mishandles redirects generated by OutputPage::output() when
the URL contains a colon.
Avoid tinifying colons for IIS 7.

It's a pity how it pollutes a clean function like wfUrlencode.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -299,16 +299,27 @@
300300 *
301301 * ;:@$!*(),/
302302 *
 303+ * However, IIS7 redirects fail when the url contains a colon (Bug 22709),
 304+ * so no fancy : for IIS7.
 305+ *
303306 * %2F in the page titles seems to fatally break for some reason.
304307 *
305308 * @param $s String:
306309 * @return string
307310 */
308311 function wfUrlencode( $s ) {
 312+ static $needle;
 313+ if ( is_null( $needle ) ) {
 314+ $needle = array( '%3B','%40','%24','%21','%2A','%28','%29','%2C','%2F' );
 315+ if (! isset($_SERVER['SERVER_SOFTWARE']) || ( strpos($_SERVER['SERVER_SOFTWARE'], "Microsoft-IIS/7") === false)) {
 316+ $needle[] = '%3A';
 317+ }
 318+ }
 319+
309320 $s = urlencode( $s );
310321 $s = str_ireplace(
311 - array( '%3B','%3A','%40','%24','%21','%2A','%28','%29','%2C','%2F' ),
312 - array( ';', ':', '@', '$', '!', '*', '(', ')', ',', '/' ),
 322+ $needle,
 323+ array( ';', '@', '$', '!', '*', '(', ')', ',', '/', ':' ),
313324 $s
314325 );
315326
Index: trunk/phase3/RELEASE-NOTES
——@@ -36,6 +36,8 @@
3737 when the address changed
3838 * (bug 22664) Special:Userrights now accepts '0' as a valid user name
3939 * (bug 5210) preload parser now parses <noinclude>, <includeonly> and redirects
 40+* (bug 22709) IIS7 mishandles redirects generated by OutputPage::output() when
 41+the URL contains a colon.
4042
4143 == API changes in 1.17 ==
4244

Comments

#Comment by Lhridley (talk | contribs)   22:17, 3 March 2010

So basically, instead of encoding the colon in the title when the title prefix or namespace has special characters, we're going to encode it all the time?

#Comment by Platonides (talk | contribs)   23:02, 3 March 2010

Mmmh, yes. Since your code didn't take that into account, I missed that part.

We should probably reopen bug 22709 asking for a better implementation.

#Comment by Lhridley (talk | contribs)   23:31, 3 March 2010

Well, it sort of did....by testing for the presence of a % sign. Not a clean way of doing it, I know, but it was working. That was part of my reasoning for not generating a patch or pushing an update myself.

#Comment by Lhridley (talk | contribs)   23:35, 3 March 2010

if(!(strpos($s, '%')===false)) { 

   $s = str_ireplace(array(':'), array('%3A'), $s);

}

Status & tagging log