Page MenuHomePhabricator

ResourceLoader incorrectly minifies Wikibase Termbox JS in package module
Closed, ResolvedPublic

Description

To reproduce the issue without Wikibase, apply the following patch to MediaWiki core:

diff --git a/resources/Resources.php b/resources/Resources.php
index d7a633fc2d..ddfafcd9dc 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -3103,4 +3103,12 @@
 		'class' => ResourceLoaderOOUIImageModule::class,
 		'themeImages' => 'icons-wikimedia',
 	],
+	'minify-bug' => [
+		'packageFiles' => [
+			[
+				'name' => '.js',
+				'content' => file_get_contents( 'https://github.com/wikimedia/wikibase-termbox/raw/054162075210e18553a5716a0905eca498fa9f0c/dist/wikibase.termbox.main.js' ),
+			],
+		],
+	],
 ];

Then, pipe the module into Node (just to check the JS syntax) with and without debug mode (your load.php URL will vary a bit, of course):

$ curl -s 'http://localhost/wiki1/load.php?lang=en&modules=minify-bug&debug=true' | node
[stdin]:1
mw.loader.implement( "minify-bug@19vlw", {
^

ReferenceError: mw is not defined
    at [stdin]:1:1
    at Script.runInThisContext (node:vm:132:18)
    at Object.runInThisContext (node:vm:309:38)
    at node:internal/process/execution:77:19
    at [stdin]-wrapper:6:22
    at evalScript (node:internal/process/execution:76:60)
    at node:internal/main/eval_stdin:29:5
    at Socket.<anonymous> (node:internal/process/execution:205:5)
    at Socket.emit (node:events:339:22)
    at endReadableNT (node:internal/streams/readable:1289:12)
$ curl -s 'http://localhost/wiki1/load.php?lang=en&modules=minify-bug' | node
[stdin]:169
getOwnPropertyNames||function(t){return r(t,o)}},fdbf:function(t,e,n){var r=n("4930");t.exports=r&&!Symbol.sham&&"symbol"==typeof Symbol.iterator},fe68:function(t,e,n){var r=n("bb6e");t.exports=function(t,e){if(!r(t))return t;var n,i;if(e&&"function"==typeof(n=t.toString)&&!r(i=n.call(t)))return i;if("function"==typeof(n=t.valueOf)&&!r(i=n.call(t)))return i;if(!e&&"function"==typeof(n=t.toString)&&!r(i=n.call(t)))return i;throw TypeError("Can't convert object to primitive value")}}});
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         

SyntaxError: Unexpected end of input
    at new Script (node:vm:100:7)
    at createScript (node:vm:261:10)
    at Object.runInThisContext (node:vm:309:10)
    at node:internal/process/execution:77:19
    at [stdin]-wrapper:6:22
    at evalScript (node:internal/process/execution:76:60)
    at node:internal/main/eval_stdin:29:5
    at Socket.<anonymous> (node:internal/process/execution:205:5)
    at Socket.emit (node:events:339:22)
    at endReadableNT (node:internal/streams/readable:1289:12)

To see the error in the browser, you can add the module as a dependency of another one to ensure it’s loaded automatically:

diff --git a/resources/Resources.php b/resources/Resources.php
index d7a633fc2d..e125f25fde 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -678,6 +678,7 @@
 			'mediawiki.util',
 			'mediawiki.jqueryMsg',
 			'user.options',
+			'minify-bug',
 		],
 		'messages' => [
 			'api-clientside-error-noconnect',

Firefox reports the error as follows:

Uncaught SyntaxError: missing } after function body (load.php:1101:664)
note: { opened at line 190, column 268 (load.php:190:268)


Here’s the end of the ResourceLoader-sent JS in debug mode:

$ curl -s 'http://localhost/wiki1/load.php?lang=en&modules=minify-bug&debug=true' | tail -c123
throw TypeError("Can't convert object to primitive value")}}});
//# sourceMappingURL=wikibase.termbox.main.js.map
}
}
} );

And here it is in non-debug mode:

$ curl -s 'http://localhost/wiki1/load.php?lang=en&modules=minify-bug' | tail -c63; echo
throw TypeError("Can't convert object to primitive value")}}});

I assume in non-debug mode, ResourceLoader produces a payload ending in

//# sourceMappingURL=wikibase.termbox.main.js.map}}});

mistakenly appending the closing braces to the comment, which Minify then throws away. Accordingly, the following patch makes the issue vanish again:

diff --git a/resources/Resources.php b/resources/Resources.php
index e125f25fde..7d61109cf4 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -3108,7 +3108,7 @@
 		'packageFiles' => [
 			[
 				'name' => '.js',
-				'content' => file_get_contents( 'https://github.com/wikimedia/wikibase-termbox/raw/054162075210e18553a5716a0905eca498fa9f0c/dist/wikibase.termbox.main.js' ),
+				'content' => file_get_contents( 'https://github.com/wikimedia/wikibase-termbox/raw/054162075210e18553a5716a0905eca498fa9f0c/dist/wikibase.termbox.main.js' ) . "\n",
 			],
 		],
 	],

because then the end of the file looks like this:

$ curl -s 'http://localhost/wiki1/load.php?lang=en&modules=minify-bug' | tail -c68; echo
throw TypeError("Can't convert object to primitive value")}}});}}});

ResourceLoader should append this line break by itself.

Details

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

ResourceLoader already learned to work around similar issues in the past, apparently:

ResourceLoaderModule::buildContent()
			$scripts = $this->getScript( $context );
			// Make the script safe to concatenate by making sure there is at least one
			// trailing new line at the end of the content. Previously, this looked for
			// a semi-colon instead, but that breaks concatenation if the semicolon
			// is inside a comment like "// foo();". Instead, simply use a
			// line break as separator which matches JavaScript native logic for implicitly
			// ending statements even if a semi-colon is missing.
			// Bugs: T29054, T162719.
			if ( is_string( $scripts )
				&& strlen( $scripts )
				&& substr( $scripts, -1 ) !== "\n"
			) {
				$scripts .= "\n";
			}

(T29054, T162719)

Change 705644 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] Ensure newlines when wrapping ResourceLoader scripts

https://gerrit.wikimedia.org/r/705644

Krinkle triaged this task as High priority.Jul 26 2021, 7:00 PM

Change 705644 merged by jenkins-bot:

[mediawiki/core@master] Ensure newlines when wrapping ResourceLoader scripts

https://gerrit.wikimedia.org/r/705644

Should be resolved now, I’ll reopen if I notice any further problems while working on T286775.