From 7fee86c38ea6b42d9606db2e0b9dd7990316f14d Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Thu, 5 May 2011 13:46:47 +0000 Subject: [PATCH] Per bug 28738 comment 4, pack ResourceLoader URLs by encoding foo.bar|foo.baz|bar.baz|bar.quux as foo.bar,baz|bar.baz,quux * Expand these URLs in ResourceLoaderContext * Build and emit these URLs in OutputPage::makeResourceLoaderLink() and in mw.loader * Throw an exception in ResourceLoader::register() for module names that contain pipe characters or commas. Commas need to be forbidden for this packing feature to work. Pipes were already forbidden but weren't checked for --- includes/OutputPage.php | 3 +- includes/resourceloader/ResourceLoader.php | 31 +++++++++ .../resourceloader/ResourceLoaderContext.php | 30 ++++++++- resources/mediawiki/mediawiki.js | 64 ++++++++++++------- 4 files changed, 103 insertions(+), 25 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 9385c1f505..2f8d85bcd1 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2396,7 +2396,6 @@ class OutputPage { $wgResourceLoaderInlinePrivateModules; // Lazy-load ResourceLoader // TODO: Should this be a static function of ResourceLoader instead? - // TODO: Divide off modules starting with "user", and add the user parameter to them $baseQuery = array( 'lang' => $this->getContext()->getLang()->getCode(), 'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false', @@ -2475,7 +2474,7 @@ class OutputPage { continue; } - $query['modules'] = implode( '|', array_keys( $modules ) ); + $query['modules'] = ResourceLoader::makePackedModulesString( array_keys( $modules ) ); // Support inlining of private modules if configured as such if ( $group === 'private' && $wgResourceLoaderInlinePrivateModules ) { diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 18a050478e..3afe69cb68 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -199,6 +199,7 @@ class ResourceLoader { * this may also be a ResourceLoaderModule object. Optional when using * multiple-registration calling style. * @throws MWException: If a duplicate module registration is attempted + * @throws MWException: If a module name contains illegal characters (pipes or commas) * @throws MWException: If something other than a ResourceLoaderModule is being registered * @return Boolean: False if there were any errors, in which case one or more modules were not * registered @@ -223,6 +224,11 @@ class ResourceLoader { 'Another module has already been registered as ' . $name ); } + + // Check $name for illegal characters + if ( preg_match( '/[|,]/', $name ) ) { + throw new MWException( "ResourceLoader module name '$name' is invalid. Names may not contain pipes (|) or commas (,)" ); + } // Attach module if ( is_object( $info ) ) { @@ -698,6 +704,31 @@ class ResourceLoader { return Xml::encodeJsCall( 'mediaWiki.config.set', array( $configuration ) ); } + /** + * 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' + * @param $modules array of module names (strings) + * @return string Packed query string + */ + public static function makePackedModulesString( $modules ) { + $groups = array(); // array( prefix => array( suffixes ) ) + foreach ( $modules as $module ) { + $pos = strrpos( $module, '.' ); + $prefix = $pos === false ? '' : substr( $module, 0, $pos ); + $suffix = $pos === false ? $module : substr( $module, $pos + 1 ); + $groups[$prefix][] = $suffix; + } + + $arr = array(); + foreach ( $groups as $prefix => $suffixes ) { + $p = $prefix === '' ? '' : $prefix . '.'; + $arr[] = $p . implode( ',', $suffixes ); + } + return implode( '|', $arr ); + } + /** * Determine whether debug mode was requested * Order of priority is 1) request param, 2) cookie, 3) $wg setting diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php index b40c3a9e76..5600304518 100644 --- a/includes/resourceloader/ResourceLoaderContext.php +++ b/includes/resourceloader/ResourceLoaderContext.php @@ -51,7 +51,7 @@ class ResourceLoaderContext { // Interpret request // List of modules $modules = $request->getVal( 'modules' ); - $this->modules = $modules ? explode( '|', $modules ) : array(); + $this->modules = $modules ? self::expandModuleNames( $modules ) : array(); // Various parameters $this->skin = $request->getVal( 'skin' ); $this->user = $request->getVal( 'user' ); @@ -63,6 +63,34 @@ class ResourceLoaderContext { $this->skin = $wgDefaultSkin; } } + + /** + * 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' ) + * @param $modules String Packed module name list + * @return array of module names + */ + public static function expandModuleNames( $modules ) { + $retval = array(); + $exploded = explode( '|', $modules ); + foreach ( $exploded as $group ) { + if ( strpos( $group, ',' ) === false ) { + // This is not a set of modules in foo.bar,baz notation + // but a single module + $retval[] = $group; + } else { + // This is a set of modules in foo.bar,baz notation + $pos = strrpos( $group, '.' ); + $prefix = substr( $group, 0, $pos ); // 'foo' + $suffixes = explode( ',', substr( $group, $pos + 1 ) ); // array( 'bar', 'baz' ) + foreach ( $suffixes as $suffix ) { + $retval[] = "$prefix.$suffix"; + } + } + } + return $retval; + } public function getResourceLoader() { return $this->resourceLoader; diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js index fe27fbadf2..4630b76c65 100644 --- a/resources/mediawiki/mediawiki.js +++ b/resources/mediawiki/mediawiki.js @@ -863,6 +863,20 @@ window.mediaWiki = new ( function( $ ) { } return sorted; } + + /** + * Converts a module map of the form { foo: [ 'bar', 'baz' ], bar: [ 'baz, 'quux' ] } + * to a query string of the form foo.bar,baz|bar.baz,quux + */ + function buildModulesString( moduleMap ) { + var arr = []; + for ( var prefix in moduleMap ) { + var p = prefix === '' ? '' : prefix + '.'; + arr.push( p + moduleMap[prefix].join( ',' ) ); + } + return arr.join( '|' ); + } + /* Public Methods */ @@ -920,32 +934,38 @@ window.mediaWiki = new ( function( $ ) { var reqBaseLength = $.param( reqBase ).length; var reqs = []; var limit = mw.config.get( 'wgResourceLoaderMaxQueryLength', -1 ); - if ( limit > 0 ) { - // We may need to split up the request to honor the query string length limit - // So build it piece by piece - var l = reqBaseLength + 9; // '&modules='.length == 9 - var r = 0; - reqs[0] = []; - for ( var i = 0; i < groups[group].length; i++ ) { - // If the request would become too long, create a new one, - // but don't create empty requests - // '%7C'.length == 3 - if ( reqs[r].length > 0 && l + 3 + groups[group][i].length > limit ) { - // This request would become too long, create a new one - r++; - reqs[r] = []; - l = reqBaseLength + 9; - } - reqs[r][reqs[r].length] = groups[group][i]; - l += groups[group][i].length + 3; + // We may need to split up the request to honor the query string length limit + // So build it piece by piece + var l = reqBaseLength + 9; // '&modules='.length == 9 + var r = 0; + reqs[0] = {}; // { prefix: [ suffixes ] } + for ( var i = 0; i < groups[group].length; i++ ) { + // Determine how many bytes this module would add to the query string + var lastDotIndex = groups[group][i].lastIndexOf( '.' ); + // Note that these substr() calls work even if lastDotIndex == -1 + var prefix = groups[group][i].substr( 0, lastDotIndex ); + var suffix = groups[group][i].substr( lastDotIndex + 1 ); + var bytesAdded = prefix in reqs[r] ? + suffix.length + 3 : // '%2C'.length == 3 + groups[group][i].length + 3; // '%7C'.length == 3 + + // If the request would become too long, create a new one, + // but don't create empty requests + if ( limit > 0 && reqs[r] != {} && l + bytesAdded > limit ) { + // This request would become too long, create a new one + r++; + reqs[r] = {}; + l = reqBaseLength + 9; } - } else { - // No splitting needed - reqs = [ groups[group] ]; + if ( !( prefix in reqs[r] ) ) { + reqs[r][prefix] = []; + } + reqs[r][prefix].push( suffix ); + l += bytesAdded; } for ( var r = 0; r < reqs.length; r++ ) { requests[requests.length] = $.extend( - { 'modules': reqs[r].join( '|' ) }, reqBase + { 'modules': buildModulesString( reqs[r] ) }, reqBase ); } } -- 2.20.1