Merge "resourceloader: Add tests for disallowing access to private modules"

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 9892b15..0785225 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -661,8 +661,9 @@ class ResourceLoader implements LoggerAwareInterface {
                                // Do not allow private modules to be loaded from the web.
                                // This is a security issue, see T36907.
                                if ( $module->getGroup() === 'private' ) {
+                                       // Not a serious error, just means something is trying to access it (T101806)
                                        $this->logger->debug( "Request for private module '$name' denied" );
-                                       $this->errors[] = "Cannot show private module \"$name\"";
+                                       $this->errors[] = "Cannot build private module \"$name\"";
                                        continue;
                                }
                                $modules[$name] = $module;

diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js
index ad05c6f..ab7f3a0 100644 (file)
--- a/resources/src/startup/mediawiki.js
+++ b/resources/src/startup/mediawiki.js
@@ -1211,15 +1211,9 @@
 
                                dependencies.forEach( function ( module ) {
                                        // Only queue modules that are still in the initial 'registered' state
-                                       // (not ones already loading, ready or error).
+                                       // (e.g. not ones already loading or loaded etc.).
                                        if ( registry[ module ].state === 'registered' && queue.indexOf( module ) === -1 ) {
-                                               // Private modules must be embedded in the page. Don't bother queuing
-                                               // these as the server will deny them anyway (T101806).
-                                               if ( registry[ module ].group === 'private' ) {
-                                                       setAndPropagate( module, 'error' );
-                                               } else {
-                                                       queue.push( module );
-                                               }
+                                               queue.push( module );
                                        }
                                } );
 

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index 86c2e9f..ac4a1ca 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -1095,6 +1095,32 @@ END
                $rl->respond( $context );
        }
 
+       /**
+        * Refuse requests for private modules.
+        *
+        * @covers ResourceLoader::respond
+        */
+       public function testRespondErrorPrivate() {
+               $rl = $this->getMockBuilder( EmptyResourceLoader::class )
+                       ->setMethods( [
+                               'measureResponseTime',
+                               'tryRespondNotModified',
+                               'sendResponseHeaders',
+                       ] )
+                       ->getMock();
+               $rl->register( [
+                       'foo' => [ 'class' => ResourceLoaderTestModule::class ],
+                       'bar' => [ 'class' => ResourceLoaderTestModule::class, 'group' => 'private' ],
+               ] );
+               $context = $this->getResourceLoaderContext(
+                       [ 'modules' => 'foo|bar', 'only' => null ],
+                       $rl
+               );
+
+               $this->expectOutputRegex( '/^\/\*.+Cannot build private module/s' );
+               $rl->respond( $context );
+       }
+
        /**
         * @covers ResourceLoader::respond
         */