From 0f201b19f4750790d0f33db57f1f2c88a60468c0 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 6 Jul 2011 21:48:09 +0000 Subject: [PATCH] * (bug 28626) Validate JavaScript files and pages loaded via ResourceLoader before minification, protecting separate modules from interference This is possibly not perfect but seems to serve for a start; follows up on r91591 that adds JSMin+ to use it in some unit tests. May want to adjust some related bits. - $wgResourceLoaderValidateJs on by default (can be disabled) - when loading a JS file through ResourceLoaderFileModule or ResourceLoaderWikiModule, parse it using JSMinPlus's JSParser class. If the parser throws an exception, the JS code of the offending file will be replaced by a JS exception throw listing the file or page name, line number (in original form), and description of the error from the parser. - parsing results are cached based on md5 of content to avoid re-parsing identical text - for JS pages loaded via direct load.php request, the parse error is thrown and visible in the JS console/error log Issues: - the primary use case for this is when a single load.php request implements multiple modules via mw.loader.implement() -- the loader catches the exception and skips on to the next module (good) but doesn't re-throw the exception for the JS console. It does log to console if present, but it'll only show up as a regular debug message, not an error. This can suppress visibility of errors in a module that's loaded together with other modules (such as a gadget). - have not done performance testing on the JSParser - have not done thorough unit testing with the JSParser --- includes/DefaultSettings.php | 6 +++ .../ResourceLoaderFileModule.php | 1 + .../resourceloader/ResourceLoaderModule.php | 50 +++++++++++++++++++ .../ResourceLoaderWikiModule.php | 1 + 4 files changed, 58 insertions(+) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 8dbe338920..e5b05f9514 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -2519,6 +2519,12 @@ $wgLegacyJavaScriptGlobals = true; */ $wgResourceLoaderMaxQueryLength = -1; +/** + * If set to true, JavaScript will be parsed prior to minification to validate it. + * Parse errors will result in a JS exception being thrown during module load. + */ +$wgResourceLoaderValidateJS = true; + /** @} */ # End of resource loader settings } diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php index ec9c3f8bd0..01c9b59543 100644 --- a/includes/resourceloader/ResourceLoaderFileModule.php +++ b/includes/resourceloader/ResourceLoaderFileModule.php @@ -495,6 +495,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { if ( $contents === false ) { throw new MWException( __METHOD__.": script file not found: \"$localPath\"" ); } + $contents = $this->validateScriptFile( $fileName, $contents ); $js .= $contents . "\n"; } return $js; diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 0c8f434fa8..5af7c5fedc 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -304,4 +304,54 @@ abstract class ResourceLoaderModule { public function isKnownEmpty( ResourceLoaderContext $context ) { return false; } + + + /** @var JSParser lazy-initialized; use self::javaScriptParser() */ + private static $jsParser; + private static $parseCacheVersion = 1; + + /** + * Validate a given script file; if valid returns the original source. + * If invalid, returns replacement JS source that throws an exception. + * + * @param string $fileName + * @param string $contents + * @return string JS with the original, or a replacement error + */ + protected function validateScriptFile( $fileName, $contents ) { + global $wgResourceLoaderValidateJS; + if ( $wgResourceLoaderValidateJS ) { + // Try for cache hit + // Use CACHE_ANYTHING since filtering is very slow compared to DB queries + $key = wfMemcKey( 'resourceloader', 'jsparse', self::$parseCacheVersion, md5( $contents ) ); + $cache = wfGetCache( CACHE_ANYTHING ); + $cacheEntry = $cache->get( $key ); + if ( is_string( $cacheEntry ) ) { + return $cacheEntry; + } + + $parser = self::javaScriptParser(); + try { + $parser->parse( $contents, $fileName, 1 ); + $result = $contents; + } catch (Exception $e) { + // We'll save this to cache to avoid having to validate broken JS over and over... + $err = $e->getMessage(); + $result = "throw new Error(" . Xml::encodeJsVar("JavaScript parse error: $err") . ");"; + } + + $cache->set( $key, $result ); + return $result; + } else { + return $contents; + } + } + + protected static function javaScriptParser() { + if ( !self::$jsParser ) { + self::$jsParser = new JSParser(); + } + return self::$jsParser; + } + } diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php index cab97e19fc..83fa294096 100644 --- a/includes/resourceloader/ResourceLoaderWikiModule.php +++ b/includes/resourceloader/ResourceLoaderWikiModule.php @@ -82,6 +82,7 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { } $script = $this->getContent( $title ); if ( strval( $script ) !== '' ) { + $script = $this->validateScriptFile( $titleText, $script ); if ( strpos( $titleText, '*/' ) === false ) { $scripts .= "/* $titleText */\n"; } -- 2.20.1