From 2d8b0272191bc6f4de09144b1ff0c2f19e8d0a5d Mon Sep 17 00:00:00 2001 From: Trevor Parscal Date: Fri, 24 Sep 2010 22:10:25 +0000 Subject: [PATCH] * Fixed bug #25281 by adding special treatment for modules in the "private" group * Added $wgResourceLoaderInlinePrivateModules to allow private modules to be either embedded in the HTML output or accessed through ResourceLoader (which will bypass squid cache and check the user paramter against $wgUser) * Moved more generated javascript functionality to ResourceLoader * Fixed comment typo made in r73673 * Added documentation for ResoruceLoaderRegisterModules hook --- RELEASE-NOTES | 5 +++ docs/hooks.txt | 3 ++ includes/DefaultSettings.php | 6 +++ includes/OutputPage.php | 29 ++++++++++--- includes/ResourceLoader.php | 70 ++++++++++++++++++++---------- includes/ResourceLoaderContext.php | 2 +- includes/ResourceLoaderModule.php | 34 ++++++++------- includes/Skin.php | 4 +- 8 files changed, 107 insertions(+), 46 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 4c4e024314..62878c82c5 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -62,6 +62,11 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN version parameter or not. * $wgResourceLoaderDebug was added to specify the default state of debug mode; this will still be overridden with the debug URL parameter a la $wgLanguageCode. +* $wgResourceLoaderInlinePrivateModules was added to specify whether private + modules such as user.options should be embedded in the HTML output or delivered + through a resource loader request, which bypasses server cache (like squid) and + checks the user parameter against $wgUser. The former adds more data to all + pages, while the latter adds a request which cannot be cached server side. === New features in 1.17 === * (bug 10183) Users can now add personal styles and scripts to all skins via diff --git a/docs/hooks.txt b/docs/hooks.txt index e9c972e2ba..9463288863 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -1337,6 +1337,9 @@ $errorMsg: an html message string of an error $article: the page the form is shown for $out: OutputPage object +'ResourceLoaderRegisterModules': Right before modules information is required, such as when responding to a resource +loader request or generating HTML output. + 'RawPageViewBeforeOutput': Right before the text is blown out in action=raw &$obj: RawPage object &$text: The text that's going to be the output diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index c3bb2c75d7..4b395f2589 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -1660,6 +1660,12 @@ $wgResourceLoaderMaxage = array( ), ); +/** + * Whether to embed private modules inline with HTML output or to bypass caching and check the user parameter against + * $wgUser to prevent unauthorized access to private modules. + */ +$wgResourceLoaderInlinePrivateModules = true; + /** * The default debug mode (on/off) for of ResourceLoader requests. This will still * be overridden when the debug URL parameter is used. diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 401ec50516..a32b04e98a 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2282,7 +2282,8 @@ class OutputPage { // TODO: Document static function makeResourceLoaderLink( $skin, $modules, $only, $useESI = false ) { - global $wgUser, $wgLang, $wgRequest, $wgLoadScript, $wgResourceLoaderDebug, $wgResourceLoaderUseESI; + global $wgUser, $wgLang, $wgRequest, $wgLoadScript, $wgResourceLoaderDebug, $wgResourceLoaderUseESI, + $wgResourceLoaderInlinePrivateModules; // TODO: Should this be a static function of ResourceLoader instead? // TODO: Divide off modules starting with "user", and add the user parameter to them $query = array( @@ -2308,20 +2309,38 @@ class OutputPage { $links = ''; foreach ( $groups as $group => $modules ) { $query['modules'] = implode( '|', array_keys( $modules ) ); - // Special handling for user group - if ( $group === 'user' && $wgUser->isLoggedIn() ) { + // Special handling for user-specific groups + if ( ( $group === 'user' || $group === 'private' ) && $wgUser->isLoggedIn() ) { $query['user'] = $wgUser->getName(); } + // Support inlining of private modules if configured as such + if ( $group === 'private' && $wgResourceLoaderInlinePrivateModules ) { + $context = new ResourceLoaderContext( new FauxRequest( $query ) ); + if ( $only == 'styles' ) { + $links .= Html::inlineStyle( + ResourceLoader::makeLoaderConditionalScript( + ResourceLoader::makeModuleResponse( $context, $modules ) + ) + ); + } else { + $links .= Html::inlineScript( + ResourceLoader::makeLoaderConditionalScript( + ResourceLoader::makeModuleResponse( $context, $modules ) + ) + ); + } + continue; + } // Special handling for user and site groups; because users might change their stuff on-wiki like site or // user pages, or user preferences; we need to find the highest timestamp of these user-changable modules so // we can ensure cache misses on change if ( $group === 'user' || $group === 'site' ) { // Create a fake request based on the one we are about to make so modules return correct times - $request = new ResourceLoaderContext( new FauxRequest( $query ) ); + $context = new ResourceLoaderContext( new FauxRequest( $query ) ); // Get the maximum timestamp $timestamp = 0; foreach ( $modules as $module ) { - $timestamp = max( $timestamp, $module->getModifiedTime( $request ) ); + $timestamp = max( $timestamp, $module->getModifiedTime( $context ) ); } // Add a version parameter so cache will break when things change $query['version'] = wfTimestamp( TS_ISO_8601, round( $timestamp, -2 ) ); diff --git a/includes/ResourceLoader.php b/includes/ResourceLoader.php index c6f9ce2a05..484329b8c8 100644 --- a/includes/ResourceLoader.php +++ b/includes/ResourceLoader.php @@ -59,7 +59,7 @@ class ResourceLoader { * This is not inside the module code because it's so much more performant to request all of the information at once * than it is to have each module requests it's own information. * - * @param $modules array list of modules to preload information for + * @param $modules array list of module names to preload information for * @param $context ResourceLoaderContext context to load the information within */ protected static function preloadModuleInfo( array $modules, ResourceLoaderContext $context ) { @@ -268,10 +268,10 @@ class ResourceLoader { // Split requested modules into two groups, modules and missing $modules = array(); $missing = array(); - + foreach ( $context->getModules() as $name ) { if ( isset( self::$modules[$name] ) ) { - $modules[] = $name; + $modules[$name] = self::$modules[$name]; } else { $missing[] = $name; } @@ -291,14 +291,19 @@ class ResourceLoader { } // Preload information needed to the mtime calculation below - self::preloadModuleInfo( $modules, $context ); + self::preloadModuleInfo( array_keys( $modules ), $context ); // To send Last-Modified and support If-Modified-Since, we need to detect // the last modified time wfProfileIn( __METHOD__.'-getModifiedTime' ); $mtime = 1; - foreach ( $modules as $name ) { - $mtime = max( $mtime, self::$modules[$name]->getModifiedTime( $context ) ); + foreach ( $modules as $module ) { + // Bypass squid cache if the request includes any private modules + if ( $module->getGroup() === 'private' ) { + $smaxage = 0; + } + // Calculate maximum modified time + $mtime = max( $mtime, $module->getModifiedTime( $context ) ); } wfProfileOut( __METHOD__.'-getModifiedTime' ); @@ -316,26 +321,34 @@ class ResourceLoader { return; } + echo self::makeModuleResponse( $context, $modules, $missing ); + + wfProfileOut( __METHOD__ ); + } + + public static function makeModuleResponse( ResourceLoaderContext $context, array $modules, $missing = null ) { + global $wgUser; + // Pre-fetch blobs $blobs = $context->shouldIncludeMessages() ? - MessageBlobStore::get( $modules, $context->getLanguage() ) : array(); + MessageBlobStore::get( array_keys( $modules ), $context->getLanguage() ) : array(); // Generate output $out = ''; - foreach ( $modules as $name ) { + foreach ( $modules as $name => $module ) { wfProfileIn( __METHOD__ . '-' . $name ); // Scripts $scripts = ''; if ( $context->shouldIncludeScripts() ) { - $scripts .= self::$modules[$name]->getScript( $context ) . "\n"; + $scripts .= $module->getScript( $context ) . "\n"; } // Styles $styles = array(); if ( $context->shouldIncludeStyles() && - ( count( $styles = self::$modules[$name]->getStyles( $context ) ) ) + ( count( $styles = $module->getStyles( $context ) ) ) ) { // Flip CSS on a per-module basis if ( self::$modules[$name]->getFlip( $context ) ) { @@ -360,7 +373,7 @@ class ResourceLoader { $out .= self::makeMessageSetScript( $messages ); break; default: - // Minify CSS, unless in debug mode, before embedding in implment script + // Minify CSS before embedding in mediaWiki.loader.implement call (unless in debug mode) if ( !$context->getDebug() ) { foreach ( $styles as $media => $style ) { $styles[$media] = self::filter( 'minify-css', $style ); @@ -376,29 +389,28 @@ class ResourceLoader { // Update module states if ( $context->shouldIncludeScripts() ) { // Set the state of modules loaded as only scripts to ready - if ( count( $modules ) && $context->getOnly() === 'scripts' && !in_array( 'startup', $modules ) ) { - $out .= self::makeLoaderStateScript( array_fill_keys( $modules, 'ready' ) ); + if ( count( $modules ) && $context->getOnly() === 'scripts' && !isset( $modules['startup'] ) ) { + $out .= self::makeLoaderStateScript( array_fill_keys( array_keys( $modules ), 'ready' ) ); } // Set the state of modules which were requested but unavailable as missing - if ( count( $missing ) ) { + if ( is_array( $missing ) && count( $missing ) ) { $out .= self::makeLoaderStateScript( array_fill_keys( $missing, 'missing' ) ); } } - // Send output if ( $context->getDebug() ) { - echo $out; + return $out; } else { if ( $context->getOnly() === 'styles' ) { - echo self::filter( 'minify-css', $out ); + return self::filter( 'minify-css', $out ); } else { - echo self::filter( 'minify-js', $out ); + return self::filter( 'minify-js', $out ); } } - - wfProfileOut( __METHOD__ ); } - + + // Client code generation methods + public static function makeLoaderImplementScript( $name, $scripts, $styles, $messages ) { if ( is_array( $scripts ) ) { $scripts = implode( $scripts, "\n" ); @@ -437,7 +449,7 @@ class ResourceLoader { return "mediaWiki.loader.state( '$name', '$state' );\n"; } } - + public static function makeCustomLoaderScript( $name, $version, $dependencies, $group, $script ) { $name = Xml::escapeJsString( $name ); $version = (int) $version > 1 ? (int) $version : 1; @@ -454,10 +466,10 @@ class ResourceLoader { $group = 'null'; } $script = str_replace( "\n", "\n\t", trim( $script ) ); - return "( function( name, version, dependencies ) {\t$script\t} )" . + return "( function( name, version, dependencies ) {\n\t$script\n} )" . "( '$name', $version, $dependencies, $group );\n"; } - + public static function makeLoaderRegisterScript( $name, $version = null, $dependencies = null, $group = null ) { if ( is_array( $name ) ) { $registrations = FormatJson::encode( $name ); @@ -480,4 +492,14 @@ class ResourceLoader { return "mediaWiki.loader.register( '$name', $version, $dependencies, $group );\n"; } } + + public static function makeLoaderConditionalScript( $script ) { + $script = str_replace( "\n", "\n\t", trim( $script ) ); + return "if ( window.mediaWiki ) {\n\t$script\n}\n"; + } + + public static function makeConfigSetScript( array $configuration ) { + $configuration = FormatJson::encode( $configuration ); + return "mediaWiki.config.set( $configuration );\n"; + } } diff --git a/includes/ResourceLoaderContext.php b/includes/ResourceLoaderContext.php index 3e4bac87ec..c8b3baf235 100644 --- a/includes/ResourceLoaderContext.php +++ b/includes/ResourceLoaderContext.php @@ -27,7 +27,7 @@ defined( 'MEDIAWIKI' ) || die( 1 ); * of a specific loader request */ class ResourceLoaderContext { - + /* Protected Members */ protected $request; diff --git a/includes/ResourceLoaderModule.php b/includes/ResourceLoaderModule.php index 5b9f900d41..751cb1bba2 100644 --- a/includes/ResourceLoaderModule.php +++ b/includes/ResourceLoaderModule.php @@ -205,7 +205,6 @@ abstract class ResourceLoaderModule { $this->msgBlobMtime[$lang] = $mtime; } - /* Abstract Methods */ /** @@ -905,21 +904,20 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule { } global $wgUser; - $username = $context->getUser(); - // Avoid extra db query by using $wgUser if possible - $user = $wgUser->getName() === $username ? $wgUser : User::newFromName( $username ); - if ( $user ) { - return $this->modifiedTime[$hash] = $user->getTouched(); + if ( $context->getUser() === $wgUser->getName() ) { + return $this->modifiedTime[$hash] = $wgUser->getTouched(); } else { return 1; } } public function getScript( ResourceLoaderContext $context ) { - $user = User::newFromName( $context->getUser() ); - if ( $user instanceof User ) { - $options = FormatJson::encode( $user->getOptions() ); + global $wgUser; + + // Verify identity -- this is a private module + if ( $context->getUser() === $wgUser->getName() ) { + $options = FormatJson::encode( $wgUser->getOptions() ); } else { $options = FormatJson::encode( User::getDefaultOptions() ); } @@ -927,11 +925,17 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule { } public function getStyles( ResourceLoaderContext $context ) { - global $wgAllowUserCssPrefs; + global $wgUser, $wgAllowUserCssPrefs; + if ( $wgAllowUserCssPrefs ) { - $user = User::newFromName( $context->getUser() ); - $options = $user instanceof User ? $user->getOptions() : User::getDefaultOptions(); - + // Verify identity -- this is a private module + if ( $context->getUser() === $wgUser->getName() ) { + $options = FormatJson::encode( $wgUser->getOptions() ); + } else { + $options = FormatJson::encode( User::getDefaultOptions() ); + } + + // Build CSS rules $rules = array(); if ( $options['underline'] < 2 ) { $rules[] = "a { text-decoration: " . ( $options['underline'] ? 'underline' : 'none' ) . "; }"; @@ -965,9 +969,9 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule { return $wgContLang->getDir() !== $context->getDirection(); } - + public function getGroup() { - return 'user'; + return 'private'; } } diff --git a/includes/Skin.php b/includes/Skin.php index 27746df4dd..7e0bb3dc60 100644 --- a/includes/Skin.php +++ b/includes/Skin.php @@ -358,7 +358,9 @@ class Skin extends Linker { static function makeVariablesScript( $data ) { if ( $data ) { - return Html::inlineScript( 'mediaWiki.config.set(' . FormatJson::encode( $data ) . ');' ); + return Html::inlineScript( + ResourceLoader::makeLoaderConditionalScript( ResourceLoader::makeConfigSetScript( $data ) ) + ); } else { return ''; } -- 2.20.1