mw.loader: Don't unset 'require' after execution in debug mode

Follows-up bc4e07b6f6.

In production mode, 'module' and 'require' are lexically provided
through closure to the executing script and will continue to be
accessible through the script's life-time.

In debug mode, the script executes without wrapper directly from
disk (and as such, in the global scope) and variables are provided
as global variables. Since a variable can only be set to one value,
we swap the object that 'module' points to after each file. And
to avoid leakage, it is removed in-between files and after the
last one. Aside from it being technically infeasible right now,
async access to 'module' is discouraged regardless.

Late access to 'require', however, is not uncommon and should
work as expected.

Bug: T144879
Change-Id: I6ede10fd42676bb035ea26c693c78bcdc1438a7d

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 3122d42..c7715e5 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1217,12 +1217,14 @@
 
                                pendingRequests.push( function () {
                                        if ( moduleName && hasOwn.call( registry, moduleName ) ) {
+                                               // Emulate runScript() part of execute()
                                                window.require = mw.loader.require;
                                                window.module = registry[ moduleName ].module;
                                        }
                                        addScript( src ).always( function () {
-                                               // Clear environment
-                                               delete window.require;
+                                               // 'module.exports' should not persist after the file is executed to
+                                               // avoid leakage to unrelated code. 'require' should be kept, however,
+                                               // as asynchronous access to 'require' is allowed and expected. (T144879)
                                                delete window.module;
                                                r.resolve();
 

diff --git a/tests/qunit/data/defineCallMwLoaderTestCallback.js b/tests/qunit/data/defineCallMwLoaderTestCallback.js
index afd886c..641071a 100644 (file)
--- a/tests/qunit/data/defineCallMwLoaderTestCallback.js
+++ b/tests/qunit/data/defineCallMwLoaderTestCallback.js
@@ -1 +1 @@
-module.exports = 'Define worked.';
+module.exports = 'Defined.';

diff --git a/tests/qunit/data/requireCallMwLoaderTestCallback.js b/tests/qunit/data/requireCallMwLoaderTestCallback.js
index 8bc087b..815a3b4 100644 (file)
--- a/tests/qunit/data/requireCallMwLoaderTestCallback.js
+++ b/tests/qunit/data/requireCallMwLoaderTestCallback.js
@@ -1,2 +1,6 @@
-var x = require( 'test.require.define' );
-module.exports = 'Require worked.' + x;
+module.exports = {
+       immediate: require( 'test.require.define' ),
+       later: function () {
+               return require( 'test.require.define' );
+       }
+};

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
index 41d800a..b69c9e8 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
@@ -664,7 +664,7 @@
                } );
        } );
 
-       QUnit.test( 'require() in debug mode', 1, function ( assert ) {
+       QUnit.test( 'require() in debug mode', function ( assert ) {
                var path = mw.config.get( 'wgScriptPath' );
                mw.loader.register( [
                        [ 'test.require.define', '0' ],
@@ -674,9 +674,15 @@
                mw.loader.implement( 'test.require.define', [ QUnit.fixurl( path + '/tests/qunit/data/defineCallMwLoaderTestCallback.js' ) ] );
 
                return mw.loader.using( 'test.require.callback' ).then( function ( require ) {
-                       var exported = require( 'test.require.callback' );
-                       assert.strictEqual( exported, 'Require worked.Define worked.',
-                               'module.exports worked in debug mode' );
+                       var cb = require( 'test.require.callback' );
+                       assert.strictEqual( cb.immediate, 'Defined.', 'module.exports and require work in debug mode' );
+                       // Must use try-catch because cb.later() will throw if require is undefined,
+                       // which doesn't work well inside Deferred.then() when using jQuery 1.x with QUnit
+                       try {
+                               assert.strictEqual( cb.later(), 'Defined.', 'require works asynchrously in debug mode' );
+                       } catch ( e ) {
+                               assert.equal( null, String( e ), 'require works asynchrously in debug mode' );
+                       }
                }, function () {
                        assert.ok( false, 'Error callback fired while loader.using "test.require.callback" module' );
                } );