(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)

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 0838e2a..c205cc4 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2522,6 +2522,9 @@ class OutputPage {
                        ksort( $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 ) {
                                $esi = Xml::element( 'esi:include', array( 'src' => $url ) );
                                if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
@@ -2532,9 +2535,9 @@ class OutputPage {
                        } else {
                                // Automatically select style/script elements
                                if ( $only === ResourceLoaderModule::TYPE_STYLES ) {
-                                       $link = Html::linkedStyle( wfAppendQuery( $wgLoadScript, $query ) );
+                                       $link = Html::linkedStyle( $url );
                                } else {
-                                       $link = Html::linkedScript( wfAppendQuery( $wgLoadScript, $query ) );
+                                       $link = Html::linkedScript( $url );
                                }
                        }
 

diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 53e6bf5..209b1b4 100644 (file)
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -785,17 +785,10 @@ class WebRequest {
        public function isPathInfoBad() {
                global $wgScriptExtension;
 
-               if ( isset( $_SERVER['QUERY_STRING'] ) 
-                       && preg_match( '/\.[^\\/:*?"<>|%]+(#|\?|$)/i', $_SERVER['QUERY_STRING'] ) )
+               if ( isset( $_SERVER['QUERY_STRING'] ) )
                {
                        // Bug 28235
-                       // Block only Internet Explorer, and requests with missing UA 
-                       // 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;
-                       }
+                       return strval( self::findIE6Extension( $_SERVER['QUERY_STRING'] ) ) !== '';
                }
 
                if ( !isset( $_SERVER['PATH_INFO'] ) ) {
@@ -809,6 +802,69 @@ class WebRequest {
                $ext = substr( $pi, $dotPos );
                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

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

diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php
index 9e52e3b..33859f0 100644 (file)
--- a/includes/resourceloader/ResourceLoaderContext.php
+++ b/includes/resourceloader/ResourceLoaderContext.php
@@ -67,13 +67,12 @@ class ResourceLoaderContext {
        /**
         * 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',
-        * 'jquery.ui.baz', 'jquery.ui.quux' ) Also translating ! to .
+        * 'jquery.ui.baz', 'jquery.ui.quux' )
         * @param $modules String Packed module name list
         * @return array of module names
         */
        public static function expandModuleNames( $modules ) {
                $retval = array();
-               $modules = str_replace( "!", ".", $modules ); # bug 28840 - IE is stupid.
                $exploded = explode( '|', $modules );
                foreach ( $exploded as $group ) {
                        if ( strpos( $group, ',' ) === false ) {

diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index 8dd0052..1c3e4d6 100644 (file)
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -626,7 +626,7 @@ window.mediaWiki = new ( function( $ ) {
                                var p = prefix === '' ? '' : prefix + '.';
                                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
                        for ( var r = 0; r < requests.length; 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 );
                        }
                };