From 9b174fd3cc986da79aeb34f57acee7ef007d4b77 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 9 May 2023 10:17:14 +0800 Subject: [PATCH] MDL-78157 js: Be more careful about inserting missing module names It is perfectly legitimate to create and/or use a method named `define` in JS outside of RequireJS. Unfortunately our requirejs.php wrapper is dumb and does not understand this. In the long term we need to stop doing this at all. We really should be able to already, but every time I try to something prevents it. In the interim, this change adds a secondary check to see if there is an existing define which _does_ have the right name in it already. --- lib/requirejs.php | 58 +++++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/lib/requirejs.php b/lib/requirejs.php index 6da961ba892..84404090085 100644 --- a/lib/requirejs.php +++ b/lib/requirejs.php @@ -52,6 +52,43 @@ $file = '/' . min_clean_param($file, 'SAFEPATH'); $jsfiles = array(); list($unused, $component, $module) = explode('/', $file, 3); +/** + * Helper function to fix missing module names in JavaScript. + * + * TODO Remove this function when we find a reliable way to do this in the Grunt task. + * @param string $modulename + * @param string $js + * @return string The modified JavaScript. + */ +function requirejs_fix_define(string $modulename, string $js): string { + // First check whether there is a possible missing module name. That is: + // define (function(Foo) { + // instead of: + // define('mod_foo/bar', function(Foo) { + $missingmodule = preg_match('/define\(\s*(\[|function)/', $js); + + // Now check whether the module name is already defined elsewhere. + // This could be a totally unrelated use of the word define. + // Note: This code needs to die, in a fire. It is evil and wrong. + $missingmodule = $missingmodule && !preg_match("@define\s*\(\s*['\"]{$modulename}['\"]@", $js); + + if ($missingmodule) { + // If the JavaScript module has been defined without specifying a name then we'll + // add the Moodle module name now. + $replace = 'define(\'' . $modulename . '\', '; + + // Replace only the first occurrence. + return implode($replace, explode('define(', $js, 2)); + } else if (!preg_match('/define\s*\(/', $js)) { + echo( + "// JS module '{$modulename}' cannot be loaded, or does not contain a javascript" . + ' module in AMD format. "define()" not found.' + ); + } + + return $js; +} + // Use the caching only for meaningful revision numbers which prevents future cache poisoning. if ($rev > 0 and $rev < (time() + 60 * 60)) { // This is "production mode". @@ -104,14 +141,7 @@ if ($rev > 0 and $rev < (time() + 60 * 60)) { $js = rtrim($js); $js .= "\n"; - if (preg_match('/define\(\s*(\[|function)/', $js)) { - // If the JavaScript module has been defined without specifying a name then we'll - // add the Moodle module name now. - $replace = 'define(\'' . $modulename . '\', '; - $search = 'define('; - // Replace only the first occurrence. - $js = implode($replace, explode($search, $js, 2)); - } + $js = requirejs_fix_define($modulename, $js); $content .= $js; } @@ -157,17 +187,7 @@ if (!empty($jsfiles)) { $js = rtrim($js); } - if (preg_match('/define\(\s*(\[|function)/', $js)) { - // If the JavaScript module has been defined without specifying a name then we'll - // add the Moodle module name now. - $replace = 'define(\'' . $modulename . '\', '; - - // Replace only the first occurrence. - $js = implode($replace, explode('define(', $js, 2)); - } else if (!preg_match('/define\s*\(/', $js)) { - debugging('JS file: ' . $shortfilename . ' cannot be loaded, or does not contain a javascript' . - ' module in AMD format. "define()" not found.', DEBUG_DEVELOPER); - } + $js = requirejs_fix_define($modulename, $js); js_send_uncached($js, 'requirejs.php'); } else {