* (bug 28626) Validate JavaScript files and pages loaded via ResourceLoader before...

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

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 8dbe338..e5b05f9 100644 (file)
--- 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 ec9c3f8..01c9b59 100644 (file)
--- 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 0c8f434..5af7c5f 100644 (file)
--- 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 cab97e1..83fa294 100644 (file)
--- 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";
                                }