r81363 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81362‎ | r81363 | r81364 >
Date:01:08, 2 February 2011
Author:mah
Status:resolved (Comments)
Tags:todo 
Comment:
* Remove last bit of code that uses PATH_INFO from img_auth.php so that people who want to use protected images on hosts with sadly mis-shapen PHPs (e.g. GoDaddy) can.
* Mangle PATH_INFO handler in WebRequest so that all the relevant bits are in a (couple of) static functions.
Modified paths:
  • /trunk/phase3/img_auth.php (modified) (history)
  • /trunk/phase3/includes/WebRequest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/WebRequest.php
@@ -55,6 +55,60 @@
5656 $this->data = $_POST + $_GET;
5757 }
5858
 59+ static public function getPathInfo( $want = 'all' ) {
 60+ if ( !empty( $_SERVER['REQUEST_URI'] ) ) {
 61+ // Slurp out the path portion to examine...
 62+ $url = $_SERVER['REQUEST_URI'];
 63+ if ( !preg_match( '!^https?://!', $url ) ) {
 64+ $url = 'http://unused' . $url;
 65+ }
 66+ $a = parse_url( $url );
 67+ if( $a ) {
 68+ $path = isset( $a['path'] ) ? $a['path'] : '';
 69+
 70+ global $wgScript;
 71+ if( $path == $wgScript && $want !== 'all' ) {
 72+ // Script inside a rewrite path?
 73+ // Abort to keep from breaking...
 74+ return;
 75+ }
 76+ // Raw PATH_INFO style
 77+ $matches = self::extractTitle( $path, "$wgScript/$1" );
 78+
 79+ global $wgArticlePath;
 80+ if( !$matches && $wgArticlePath ) {
 81+ $matches = self::extractTitle( $path, $wgArticlePath );
 82+ }
 83+
 84+ global $wgActionPaths;
 85+ if( !$matches && $wgActionPaths ) {
 86+ $matches = self::extractTitle( $path, $wgActionPaths, 'action' );
 87+ }
 88+
 89+ global $wgVariantArticlePath, $wgContLang;
 90+ if( !$matches && $wgVariantArticlePath ) {
 91+ $variantPaths = array();
 92+ foreach( $wgContLang->getVariants() as $variant ) {
 93+ $variantPaths[$variant] =
 94+ str_replace( '$2', $variant, $wgVariantArticlePath );
 95+ }
 96+ $matches = self::extractTitle( $path, $variantPaths, 'variant' );
 97+ }
 98+ }
 99+ } elseif ( isset( $_SERVER['ORIG_PATH_INFO'] ) && $_SERVER['ORIG_PATH_INFO'] != '' ) {
 100+ // Mangled PATH_INFO
 101+ // http://bugs.php.net/bug.php?id=31892
 102+ // Also reported when ini_get('cgi.fix_pathinfo')==false
 103+ $matches['title'] = substr( $_SERVER['ORIG_PATH_INFO'], 1 );
 104+
 105+ } elseif ( isset( $_SERVER['PATH_INFO'] ) && ($_SERVER['PATH_INFO'] != '') ) {
 106+ // Regular old PATH_INFO yay
 107+ $matches['title'] = substr( $_SERVER['PATH_INFO'], 1 );
 108+ }
 109+
 110+ return $matches;
 111+ }
 112+
59113 /**
60114 * Check for title, action, and/or variant data in the URL
61115 * and interpolate it into the GET variables.
@@ -70,64 +124,9 @@
71125 return;
72126 }
73127
74 - if ( $wgUsePathInfo ) {
75 - // PATH_INFO is mangled due to http://bugs.php.net/bug.php?id=31892
76 - // And also by Apache 2.x, double slashes are converted to single slashes.
77 - // So we will use REQUEST_URI if possible.
78 - $matches = array();
79 -
80 - if ( !empty( $_SERVER['REQUEST_URI'] ) ) {
81 - // Slurp out the path portion to examine...
82 - $url = $_SERVER['REQUEST_URI'];
83 - if ( !preg_match( '!^https?://!', $url ) ) {
84 - $url = 'http://unused' . $url;
85 - }
86 - $a = parse_url( $url );
87 - if( $a ) {
88 - $path = isset( $a['path'] ) ? $a['path'] : '';
89 -
90 - global $wgScript;
91 - if( $path == $wgScript ) {
92 - // Script inside a rewrite path?
93 - // Abort to keep from breaking...
94 - return;
95 - }
96 - // Raw PATH_INFO style
97 - $matches = $this->extractTitle( $path, "$wgScript/$1" );
98 -
99 - global $wgArticlePath;
100 - if( !$matches && $wgArticlePath ) {
101 - $matches = $this->extractTitle( $path, $wgArticlePath );
102 - }
103 -
104 - global $wgActionPaths;
105 - if( !$matches && $wgActionPaths ) {
106 - $matches = $this->extractTitle( $path, $wgActionPaths, 'action' );
107 - }
108 -
109 - global $wgVariantArticlePath, $wgContLang;
110 - if( !$matches && $wgVariantArticlePath ) {
111 - $variantPaths = array();
112 - foreach( $wgContLang->getVariants() as $variant ) {
113 - $variantPaths[$variant] =
114 - str_replace( '$2', $variant, $wgVariantArticlePath );
115 - }
116 - $matches = $this->extractTitle( $path, $variantPaths, 'variant' );
117 - }
118 - }
119 - } elseif ( isset( $_SERVER['ORIG_PATH_INFO'] ) && $_SERVER['ORIG_PATH_INFO'] != '' ) {
120 - // Mangled PATH_INFO
121 - // http://bugs.php.net/bug.php?id=31892
122 - // Also reported when ini_get('cgi.fix_pathinfo')==false
123 - $matches['title'] = substr( $_SERVER['ORIG_PATH_INFO'], 1 );
124 -
125 - } elseif ( isset( $_SERVER['PATH_INFO'] ) && ($_SERVER['PATH_INFO'] != '') ) {
126 - // Regular old PATH_INFO yay
127 - $matches['title'] = substr( $_SERVER['PATH_INFO'], 1 );
128 - }
129 - foreach( $matches as $key => $val) {
130 - $this->data[$key] = $_GET[$key] = $_REQUEST[$key] = $val;
131 - }
 128+ $matches = self::getPathInfo( 'title' );
 129+ foreach( $matches as $key => $val) {
 130+ $this->data[$key] = $_GET[$key] = $_REQUEST[$key] = $val;
132131 }
133132 }
134133
@@ -141,7 +140,7 @@
142141 * passed on as the value of this URL parameter
143142 * @return array of URL variables to interpolate; empty if no match
144143 */
145 - private function extractTitle( $path, $bases, $key=false ) {
 144+ private static function extractTitle( $path, $bases, $key=false ) {
146145 foreach( (array)$bases as $keyValue => $base ) {
147146 // Find the part after $wgArticlePath
148147 $base = str_replace( '$1', '', $base );
Index: trunk/phase3/img_auth.php
@@ -30,6 +30,7 @@
3131 wfProfileIn( 'img_auth.php' );
3232 require_once( dirname( __FILE__ ) . '/includes/StreamFile.php' );
3333
 34+$wgActionPaths[] = $_SERVER['SCRIPT_NAME'];
3435 // See if this is a public Wiki (no protections)
3536 if ( $wgImgAuthPublicTest
3637 && in_array( 'read', User::getGroupPermissions( array( '*' ) ), true ) )
@@ -37,17 +38,8 @@
3839 wfForbidden('img-auth-accessdenied','img-auth-public');
3940 }
4041
41 -// Extract path and image information
42 -if( !isset( $_SERVER['PATH_INFO'] ) ) {
43 - $path = $wgRequest->getText( 'path' );
44 - if( !$path ) {
45 - wfForbidden( 'img-auth-accessdenied', 'img-auth-nopathinfo' );
46 - }
47 - $path = "/$path";
48 -} else {
49 - $path = $_SERVER['PATH_INFO'];
50 -}
51 -
 42+$matches = WebRequest::getPathInfo();
 43+$path = $matches['title'];
5244 $filename = realpath( $wgUploadDirectory . $path );
5345 $realUpload = realpath( $wgUploadDirectory );
5446

Follow-up revisions

RevisionCommit summaryAuthorDate
r81373follow up r81363 and fix Bug#27099mah03:14, 2 February 2011
r81387Followup r81373, r81363...reedy13:02, 2 February 2011
r81396* recover dropped check of $wgUsePathInfo from r81363...mah15:44, 2 February 2011
r110822* Fix for r81363: instead of giving a PHP notice when PATH_INFO is missing, s...tstarling03:43, 7 February 2012

Comments

#Comment by Brion VIBBER (talk | contribs)   03:04, 2 February 2011

PHP warning is emitted on every page request where no PATH_INFO stuff is available: bugzilla:27099

#Comment by MarkAHershberger (talk | contribs)   03:05, 2 February 2011

Thanks for trying this out. Working on a fix now.

#Comment by Raymond (talk | contribs)   07:37, 2 February 2011

r81373 does not help. Still seeing this error on Translatewiki.

#Comment by MarkAHershberger (talk | contribs)   15:12, 2 February 2011

Think r81387 fixed this. Thanks, reedy!

#Comment by Tim Starling (talk | contribs)   03:29, 7 February 2012

Why did you modify a configuration global variable in img_auth.php instead of using a parameter to WebRequest::getPathInfo()?

Status & tagging log