(bug 28840) URLs with dots break because of IE6 security check

* Replace the overly paranoid regex with a function that simulates IE6's behavior
* Remove the UA check in isPathInfoBad(), was causing more problems than it was worth
* Revert r87711, going back to using dots for dots in ResourceLoader URLs, instead of exclamation marks
* Append &* to ResourceLoader URLs. * is an illegal character in extensions, and putting it at the end of the URL ensures that both IE6 and our detection function will deem the URL to have no extension (unless something like .html? appears in the query string, but in that case we're screwed no matter what)
This commit is contained in:
Roan Kattouw 2011-05-26 09:49:45 +00:00
parent 061552c32b
commit 8dab43f703
5 changed files with 76 additions and 18 deletions

View file

@ -2522,6 +2522,9 @@ class OutputPage {
ksort( $query ); ksort( $query );
$url = wfAppendQuery( $wgLoadScript, $query ); $url = wfAppendQuery( $wgLoadScript, $query );
// Prevent the IE6 extension check from being triggered (bug 28840)
// by appending a character that's invalid in Windows extensions ('*')
$url .= '&*';
if ( $useESI && $wgResourceLoaderUseESI ) { if ( $useESI && $wgResourceLoaderUseESI ) {
$esi = Xml::element( 'esi:include', array( 'src' => $url ) ); $esi = Xml::element( 'esi:include', array( 'src' => $url ) );
if ( $only == ResourceLoaderModule::TYPE_STYLES ) { if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
@ -2532,9 +2535,9 @@ class OutputPage {
} else { } else {
// Automatically select style/script elements // Automatically select style/script elements
if ( $only === ResourceLoaderModule::TYPE_STYLES ) { if ( $only === ResourceLoaderModule::TYPE_STYLES ) {
$link = Html::linkedStyle( wfAppendQuery( $wgLoadScript, $query ) ); $link = Html::linkedStyle( $url );
} else { } else {
$link = Html::linkedScript( wfAppendQuery( $wgLoadScript, $query ) ); $link = Html::linkedScript( $url );
} }
} }

View file

@ -785,17 +785,10 @@ class WebRequest {
public function isPathInfoBad() { public function isPathInfoBad() {
global $wgScriptExtension; global $wgScriptExtension;
if ( isset( $_SERVER['QUERY_STRING'] ) if ( isset( $_SERVER['QUERY_STRING'] ) )
&& preg_match( '/\.[^\\/:*?"<>|%]+(#|\?|$)/i', $_SERVER['QUERY_STRING'] ) )
{ {
// Bug 28235 // Bug 28235
// Block only Internet Explorer, and requests with missing UA return strval( self::findIE6Extension( $_SERVER['QUERY_STRING'] ) ) !== '';
// headers that could be IE users behind a privacy proxy.
if ( !isset( $_SERVER['HTTP_USER_AGENT'] )
|| preg_match( '/; *MSIE/', $_SERVER['HTTP_USER_AGENT'] ) )
{
return true;
}
} }
if ( !isset( $_SERVER['PATH_INFO'] ) ) { if ( !isset( $_SERVER['PATH_INFO'] ) ) {
@ -809,6 +802,69 @@ class WebRequest {
$ext = substr( $pi, $dotPos ); $ext = substr( $pi, $dotPos );
return !in_array( $ext, array( $wgScriptExtension, '.php', '.php5' ) ); return !in_array( $ext, array( $wgScriptExtension, '.php', '.php5' ) );
} }
/**
* Determine what extension IE6 will infer from a certain query string.
* If the URL has an extension before the question mark, IE6 will use
* that and ignore the query string, but per the comment at
* isPathInfoBad() we don't have a reliable way to determine the URL,
* so isPathInfoBad() just passes in the query string for $url.
* All entry points have safe extensions (php, php5) anyway, so
* checking the query string is possibly overly paranoid but never
* insecure.
*
* The criteria for finding an extension are as follows:
* * a possible extension is a dot followed by one or more characters not in <>\"/:|?.#
* * if we find a possible extension followed by the end of the string or a #, that's our extension
* * if we find a possible extension followed by a ?, that's our extension
* ** UNLESS it's exe, dll or cgi, in which case we ignore it and continue searching for another possible extension
* * if we find a possible extension followed by a dot or another illegal character, we ignore it and continue searching
*
* @param $url string URL
* @return mixed Detected extension (string), or false if none found
*/
public static function findIE6Extension( $url ) {
$remaining = $url;
while ( $remaining !== '' ) {
// Skip ahead to the next dot
$pos = strcspn( $remaining, '.' );
if ( $pos === strlen( $remaining ) || $remaining[$pos] === '#' ) {
// End of string, we're done
return false;
}
// We found a dot. Skip past it
$remaining = substr( $remaining, $pos + 1 );
// Check for illegal characters in our prospective extension,
// or for another dot, or for a hash sign
$pos = strcspn( $remaining, "<>\\\"/:|?*.#" );
if ( $pos === strlen( $remaining ) ) {
// No illegal character or next dot or hash
// We have our extension
return $remaining;
}
if ( $remaining[$pos] === '#' ) {
// We've found a legal extension followed by a hash
// Trim the hash and everything after it, then return that
return substr( $remaining, 0, $pos );
}
if ( $remaining[$pos] === '?' ) {
// We've found a legal extension followed by a question mark
// If the extension is NOT exe, dll or cgi, return it
$extension = substr( $remaining, 0, $pos );
if ( strcasecmp( $extension, 'exe' ) && strcasecmp( $extension, 'dll' ) &&
strcasecmp( $extension, 'cgi' ) )
{
return $extension;
}
// Else continue looking
}
// We found an illegal character or another dot
// Skip to that character and continue the loop
$remaining = substr( $remaining, $pos );
}
return false;
}
/** /**
* Parse the Accept-Language header sent by the client into an array * Parse the Accept-Language header sent by the client into an array

View file

@ -736,8 +736,7 @@ class ResourceLoader {
* Convert an array of module names to a packed query string. * Convert an array of module names to a packed query string.
* *
* For example, array( 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' ) * For example, array( 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' )
* becomes 'foo!bar,baz|bar!baz,quux' * becomes 'foo.bar,baz|bar.baz,quux'
* The ! is for IE6 being stupid with extensions.
* @param $modules array of module names (strings) * @param $modules array of module names (strings)
* @return string Packed query string * @return string Packed query string
*/ */
@ -756,7 +755,7 @@ class ResourceLoader {
$arr[] = $p . implode( ',', $suffixes ); $arr[] = $p . implode( ',', $suffixes );
} }
$str = implode( '|', $arr ); $str = implode( '|', $arr );
return str_replace( ".", "!", $str ); # bug 28840 return $str;
} }
/** /**

View file

@ -67,13 +67,12 @@ class ResourceLoaderContext {
/** /**
* Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to * Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to
* an array of module names like array( 'jquery.foo', 'jquery.bar', * an array of module names like array( 'jquery.foo', 'jquery.bar',
* 'jquery.ui.baz', 'jquery.ui.quux' ) Also translating ! to . * 'jquery.ui.baz', 'jquery.ui.quux' )
* @param $modules String Packed module name list * @param $modules String Packed module name list
* @return array of module names * @return array of module names
*/ */
public static function expandModuleNames( $modules ) { public static function expandModuleNames( $modules ) {
$retval = array(); $retval = array();
$modules = str_replace( "!", ".", $modules ); # bug 28840 - IE is stupid.
$exploded = explode( '|', $modules ); $exploded = explode( '|', $modules );
foreach ( $exploded as $group ) { foreach ( $exploded as $group ) {
if ( strpos( $group, ',' ) === false ) { if ( strpos( $group, ',' ) === false ) {

View file

@ -626,7 +626,7 @@ window.mediaWiki = new ( function( $ ) {
var p = prefix === '' ? '' : prefix + '.'; var p = prefix === '' ? '' : prefix + '.';
arr.push( p + moduleMap[prefix].join( ',' ) ); arr.push( p + moduleMap[prefix].join( ',' ) );
} }
return arr.join( '|' ).replace( /\./g, '!' ); return arr.join( '|' );
} }
/** /**
@ -780,7 +780,8 @@ window.mediaWiki = new ( function( $ ) {
// Asynchronously append a script tag to the end of the body // Asynchronously append a script tag to the end of the body
for ( var r = 0; r < requests.length; r++ ) { for ( var r = 0; r < requests.length; r++ ) {
requests[r] = sortQuery( requests[r] ); requests[r] = sortQuery( requests[r] );
var src = mw.config.get( 'wgLoadScript' ) + '?' + $.param( requests[r] ); // Append &* to avoid triggering the IE6 extension check
var src = mw.config.get( 'wgLoadScript' ) + '?' + $.param( requests[r] ) + '&*';
addScript( src ); addScript( src );
} }
}; };