r87203 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r87202‎ | r87203 | r87204 >
Date:18:41, 1 May 2011
Author:catrope
Status:resolved (Comments)
Tags:
Comment:
(bug 28738) Implement request splitting in mw.loader so ResourceLoader requests with query strings longer than a certain value are split up. The maximum query string length is configurable, and this behavior is disabled by default. It's needed in rare cases where there is a query string length or GET variable length limit in place that the wiki admin can't raise.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderStartUpModule.php (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -37,7 +37,8 @@
3838 $wgArticlePath, $wgScriptPath, $wgServer, $wgContLang,
3939 $wgVariantArticlePath, $wgActionPaths, $wgUseAjax, $wgVersion,
4040 $wgEnableAPI, $wgEnableWriteAPI, $wgDBname, $wgEnableMWSuggest,
41 - $wgSitename, $wgFileExtensions, $wgExtensionAssetsPath, $wgProto, $wgCookiePrefix;
 41+ $wgSitename, $wgFileExtensions, $wgExtensionAssetsPath, $wgProto,
 42+ $wgCookiePrefix, $wgResourceLoaderMaxQueryLength;
4243
4344 // Pre-process information
4445 $separatorTransTable = $wgContLang->separatorTransformTable();
@@ -92,6 +93,7 @@
9394 'wgProto' => $wgProto,
9495 // mediawiki sets cookies to have this prefix by default
9596 'wgCookiePrefix' => $wgCookiePrefix,
 97+ 'wgResourceLoaderMaxQueryLength' => $wgResourceLoaderMaxQueryLength,
9698 );
9799 if ( $wgUseAjax && $wgEnableMWSuggest ) {
98100 $vars['wgMWSuggestTemplate'] = SearchEngine::getMWSuggestTemplate();
Index: trunk/phase3/includes/DefaultSettings.php
@@ -2537,6 +2537,19 @@
25382538 */
25392539 $wgIncludeLegacyJavaScript = true;
25402540
 2541+/**
 2542+ * If set to a positive number, ResourceLoader will not generate URLs whose
 2543+ * query string is more than this many characters long, and will instead use
 2544+ * multiple requests with shorter query strings. This degrades performance,
 2545+ * but may be needed if your web server has a low (less than, say 1024)
 2546+ * query string length limit or a low value for suhosin.get.max_value_length
 2547+ * that you can't increase.
 2548+ *
 2549+ * If set to a negative number, ResourceLoader will assume there is no query
 2550+ * string length limit.
 2551+ */
 2552+$wgResourceLoaderMaxQueryLength = -1;
 2553+
25412554 /** @} */ # End of resource loader settings }
25422555
25432556
Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -922,9 +922,38 @@
923923 version = registry[groups[group][g]].version;
924924 }
925925 }
926 - requests[requests.length] = $.extend(
927 - { 'modules': groups[group].join( '|' ), 'version': formatVersionNumber( version ) }, base
928 - );
 926+ var reqBase = $.extend( { 'version': formatVersionNumber( version ) }, base );
 927+ var reqBaseLength = $.param( reqBase ).length;
 928+ var reqs = [];
 929+ var limit = mw.config.get( 'wgResourceLoaderMaxQueryLength', -1 );
 930+ if ( limit > 0 ) {
 931+ // We may need to split up the request to honor the query string length limit
 932+ // So build it piece by piece
 933+ var l = reqBaseLength + 9; // '&modules='.length == 9
 934+ var r = 0;
 935+ reqs[0] = [];
 936+ for ( var i = 0; i < groups[group].length; i++ ) {
 937+ // If the request would become too long, create a new one,
 938+ // but don't create empty requests
 939+ // '%7C'.length == 3
 940+ if ( reqs[r].length > 0 && l + 3 + groups[group][i].length > limit ) {
 941+ // This request would become too long, create a new one
 942+ r++;
 943+ reqs[r] = [];
 944+ l = reqBaseLength + 9;
 945+ }
 946+ reqs[r][reqs[r].length] = groups[group][i];
 947+ l += groups[group][i].length + 3;
 948+ }
 949+ } else {
 950+ // No splitting needed
 951+ reqs = [ groups[group] ];
 952+ }
 953+ for ( var r in reqs ) {
 954+ requests[requests.length] = $.extend(
 955+ { 'modules': reqs[r].join( '|' ) }, reqBase
 956+ );
 957+ }
929958 }
930959 // Clear the batch - this MUST happen before we append the
931960 // script element to the body or it's possible that the script

Follow-up revisions

RevisionCommit summaryAuthorDate
r87265Fix r87203: don't use a for..in loop on an arraycatrope17:57, 2 May 2011
r87494For bug 28738, have the installer check for the Suhosin GET variable length l...catrope11:52, 5 May 2011
r87497Per bug 28738 comment 4, pack ResourceLoader URLs by encoding foo.bar|foo.baz...catrope13:46, 5 May 2011
r896761.17: MFT r82247, r87203, r87265, r87494, r87497, r87711, r87840, r88076, r89615catrope19:09, 7 June 2011

Comments

#Comment by (talk | contribs)   17:53, 2 May 2011

As "reqs" is an array, please don't run through it with a for-in-loop.

+ for ( var i=0; i<reqs.length; i++ ) { + requests[requests.length] = $.extend( + { 'modules': reqs[i].join( '|' ) }, reqBase + ); + }

#Comment by Catrope (talk | contribs)   17:56, 2 May 2011

/me cries

I guess I shouldn't code when I'm tired

Status & tagging log