From 71883c2adde49545bd7dd8f421f9d013bc57a227 Mon Sep 17 00:00:00 2001 From: Matteo Scaramuccia Date: Sat, 29 Oct 2022 18:49:05 +0200 Subject: [PATCH] MDL-72461 core: Required JS files in $PAGE served by the Moodle handler --- lib/outputrequirementslib.php | 19 +- lib/tests/outputrequirementslib_test.php | 264 +++++++++++++++++++++++ lib/tests/weblib_test.php | 134 ++++++++++++ lib/weblib.php | 22 +- 4 files changed, 431 insertions(+), 8 deletions(-) diff --git a/lib/outputrequirementslib.php b/lib/outputrequirementslib.php index 07428ae29b8c8..b744d1691e62a 100644 --- a/lib/outputrequirementslib.php +++ b/lib/outputrequirementslib.php @@ -710,15 +710,28 @@ protected function get_jquery_headcode() { } /** - * Returns the actual url through which a script is served. + * Returns the actual url through which a JavaScript file is served. * - * @param moodle_url|string $url full moodle url, or shortened path to script + * @param moodle_url|string $url full moodle url, or shortened path to script. + * @throws coding_exception if the given $url isn't a shortened url starting with / or a moodle_url instance. * @return moodle_url */ protected function js_fix_url($url) { global $CFG; if ($url instanceof moodle_url) { + // If the URL is external to Moodle, it won't be handled by Moodle (!). + if ($url->is_local_url()) { + $localurl = $url->out_as_local_url(); + // Check if the URL points to a Moodle PHP resource. + if (strpos($localurl, '.php') !== false) { + // It's a Moodle PHP resource e.g. a resource already served by the proper Moodle Handler. + return $url; + } + // It's a local resource: we need to further examine it. + return $this->js_fix_url($url->out_as_local_url(false)); + } + // The URL is not a Moodle resource. return $url; } else if (strpos($url, '/') === 0) { // Fix the admin links if needed. @@ -736,7 +749,7 @@ protected function js_fix_url($url) { if (substr($url, -3) === '.js') { $jsrev = $this->get_jsrev(); if (empty($CFG->slasharguments)) { - return new moodle_url('/lib/javascript.php', array('rev'=>$jsrev, 'jsfile'=>$url)); + return new moodle_url('/lib/javascript.php', ['rev' => $jsrev, 'jsfile' => $url]); } else { $returnurl = new moodle_url('/lib/javascript.php'); $returnurl->set_slashargument('/'.$jsrev.$url); diff --git a/lib/tests/outputrequirementslib_test.php b/lib/tests/outputrequirementslib_test.php index a3e32a66b1dca..1b615b950e750 100644 --- a/lib/tests/outputrequirementslib_test.php +++ b/lib/tests/outputrequirementslib_test.php @@ -135,4 +135,268 @@ public function test_js_call_amd() { $modname = 'theme_foobar/demo_two'; $this->assertStringContainsString("M.util.js_pending('{$modname}'); require(['{$modname}'], function(amd) {amd.init(\"foo\", \"baz\", [42,\"xyz\"]); M.util.js_complete('{$modname}');});", $html); } + + /** + * Test the actual URL through which a JavaScript file is served. + * + * @param \moodle_url $moodleurl The moodle_url instance pointing to a web resource. + * @param int $cfgslashargs The value to force $CFG->slasharguments. + * @param string $expected The expected output URL. + * @throws ReflectionException if the class does not exist. + * @see \page_requirements_manager::js_fix_url() + * @see \moodle_url + * @covers \page_requirements_manager::js_fix_url + * @dataProvider js_fix_url_moodle_url_provider + */ + public function test_js_fix_url_moodle_url(\moodle_url $moodleurl, int $cfgslashargs, string $expected) { + global $CFG; + $defaultslashargs = $CFG->slasharguments; + + $CFG->slasharguments = $cfgslashargs; + $rc = new \ReflectionClass(\page_requirements_manager::class); + $rcm = $rc->getMethod('js_fix_url'); + $rcm->setAccessible(true); + $requires = new \page_requirements_manager(); + $actualmoodleurl = $rcm->invokeArgs($requires, [$moodleurl]); + $this->assertEquals($expected, $actualmoodleurl->out(false)); + + $CFG->slasharguments = $defaultslashargs; + } + + /** + * Data provider for JavaScript proper Handler using a \moodle_url. + * + * @return array + * @see \page_requirements_manager::js_fix_url() + * @see \moodle_url + */ + public function js_fix_url_moodle_url_provider() { + global $CFG; + $wwwroot = rtrim($CFG->wwwroot, '/'); + $libdir = rtrim($CFG->libdir, '/'); + $admin = "/{$CFG->admin}/"; // Deprecated, just for coverage purposes. + + require_once($libdir . '/editor/tinymce/lib.php'); + $tiny = new \tinymce_texteditor(); + $tinyversion = $tiny->version; + + // Note: $CFG->slasharguments is enabled by default; it will be a forced setting one day (MDL-62640). + return [ + 'Environment XML file' => [ + new \moodle_url('/admin/environment.xml'), + 0, + $wwwroot . $admin . 'environment.xml' + ], + 'Google Maps CDN (HTTPS)' => [ + new \moodle_url('https://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + 1, + 'https://maps.googleapis.com/maps/api/js?key=googlemapkey3&sensor=false' + ], + 'Google Maps CDN (HTTP)' => [ + new \moodle_url('http://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + 0, + 'http://maps.googleapis.com/maps/api/js?key=googlemapkey3&sensor=false' + ], + 'H5P JS internal resource (slasharguments on)' => [ + new \moodle_url('/h5p/js/embed.js'), + 1, + $wwwroot . '/lib/javascript.php/1/h5p/js/embed.js' + ], + 'H5P JS internal resource (slasharguments off)' => [ + new \moodle_url('/h5p/js/embed.js'), + 0, + $wwwroot . '/lib/javascript.php?rev=1&jsfile=%2Fh5p%2Fjs%2Fembed.js' + ], + 'TinyMCE internal resource' => [ + new \moodle_url('/lib/editor/tinymce/tiny_mce/' . $tinyversion . '/tiny_mce.js'), + 1, + $wwwroot . '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/' . $tinyversion . '/tiny_mce.js' + ], + 'A Moodle JS resource using the full path including the proper JS Handler' => [ + new \moodle_url($wwwroot . '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/' . $tinyversion . '/tiny_mce.js'), + 1, + $wwwroot . '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/' . $tinyversion . '/tiny_mce.js' + ], + 'A custom Moodle CSS Handler' => [ + new \moodle_url('/mod/data/css.php?d=1234567890'), + 1, + $wwwroot . '/mod/data/css.php?d=1234567890' + ], + 'A custom Moodle JS Handler (slasharguments on)' => [ + new \moodle_url('/mod/data/js.php?d=1234567890'), + 1, + $wwwroot . '/mod/data/js.php?d=1234567890' + ], + 'A custom Moodle JS Handler (slasharguments off)' => [ + new \moodle_url('/mod/data/js.php?d=1234567890'), + 0, + $wwwroot . '/mod/data/js.php?d=1234567890' + ], + ]; + } + + /** + * Test the actual url through which a JavaScript file is served. + * + * @param string $url The URL pointing to a web resource. + * @param int $cfgslashargs The value to force $CFG->slasharguments. + * @param string $expected The expected output URL. + * @throws ReflectionException if the class does not exist. + * @see \page_requirements_manager::js_fix_url() + * @covers \page_requirements_manager::js_fix_url + * @dataProvider js_fix_url_plain_string_provider + */ + public function test_js_fix_url_plain_string(string $url, int $cfgslashargs, string $expected) { + global $CFG; + $defaultslashargs = $CFG->slasharguments; + + $CFG->slasharguments = $cfgslashargs; + $rc = new \ReflectionClass(\page_requirements_manager::class); + $rcm = $rc->getMethod('js_fix_url'); + $rcm->setAccessible(true); + $requires = new \page_requirements_manager(); + $actualmoodleurl = $rcm->invokeArgs($requires, [$url]); + $this->assertEquals($expected, $actualmoodleurl->out(false)); + + $CFG->slasharguments = $defaultslashargs; + } + + /** + * Data provider for JavaScript proper Handler using a plain relative string. + * + * @return array + * @see \page_requirements_manager::js_fix_url() + */ + public function js_fix_url_plain_string_provider() { + global $CFG; + $wwwroot = rtrim($CFG->wwwroot, '/'); + $admin = "/{$CFG->admin}/"; // Deprecated, just for coverage purposes. + + // Note: $CFG->slasharguments is enabled by default; it will be a forced setting one day (MDL-62640). + return [ + 'Environment XML file' => [ + '/admin/environment.xml', + 0, + $wwwroot . $admin . 'environment.xml' + ], + 'Course Format JS (slasharguments on)' => [ + '/course/format/topics/format.js', + 1, + $wwwroot . '/lib/javascript.php/1/course/format/topics/format.js' + ], + 'Course Format JS (slasharguments off)' => [ + '/course/format/topics/format.js', + 0, + $wwwroot . '/lib/javascript.php?rev=1&jsfile=%2Fcourse%2Fformat%2Ftopics%2Fformat.js' + ], + 'Data JS' => [ + '/mod/data/data.js', + 1, + $wwwroot . '/lib/javascript.php/1/mod/data/data.js' + ], + 'SCORM Request JS' => [ + '/mod/scorm/request.js', + 1, + $wwwroot . '/lib/javascript.php/1/mod/scorm/request.js' + ], + 'Wiki Editors Buttons JS' => [ + '/mod/wiki/editors/wiki/buttons.js', + 1, + $wwwroot . '/lib/javascript.php/1/mod/wiki/editors/wiki/buttons.js' + ], + 'A non-JS internal resource' => [ + '/theme/boost/pix/favicon.ico', + 0, + $wwwroot . '/theme/boost/pix/favicon.ico' + ], + 'A custom Moodle CSS Handler' => [ + '/mod/data/css.php?d=1234567890', + 1, + $wwwroot . '/mod/data/css.php?d=1234567890' + ], + 'A custom Moodle JS Handler (slasharguments on)' => [ + '/mod/data/js.php?d=1234567890', + 1, + $wwwroot . '/mod/data/js.php?d=1234567890' + ], + 'A custom Moodle JS Handler (slasharguments off)' => [ + '/mod/data/js.php?d=1234567890', + 0, + $wwwroot . '/mod/data/js.php?d=1234567890' + ], + ]; + } + + /** + * Test the coding exceptions when trying to get the actual URL through which a JavaScript file is served. + * + * @param moodle_url|string|null $url The URL pointing to a web resource. + * @param string $exmessage The expected output URL. + * @throws ReflectionException if the class does not exist. + * @see \page_requirements_manager::js_fix_url() + * @covers \page_requirements_manager::js_fix_url + * @dataProvider js_fix_url_coding_exception_provider + */ + public function test_js_fix_url_coding_exception($url, string $exmessage) { + $rc = new \ReflectionClass(\page_requirements_manager::class); + $rcm = $rc->getMethod('js_fix_url'); + $rcm->setAccessible(true); + $requires = new \page_requirements_manager(); + $this->expectException(\coding_exception::class); + $this->expectExceptionMessage($exmessage); + $actualmoodleurl = $rcm->invokeArgs($requires, [$url]); + } + + /** + * Data provider for throwing coding exceptions in \page_requirements_manager::js_fix_url(). + * + * @return array + * @see \page_requirements_manager::js_fix_url() + */ + public function js_fix_url_coding_exception_provider() { + global $CFG; + $wwwroot = rtrim($CFG->wwwroot, '/'); + + return [ + 'Provide a null argument' => [ + null, + 'Coding error detected, it must be fixed by a programmer: ' + . 'Invalid JS url, it has to be shortened url starting with / or moodle_url instance.' + ], + 'Provide an internal absolute URL' => [ + $wwwroot . '/lib/javascript.php/1/h5p/js/embed.js', + 'Coding error detected, it must be fixed by a programmer: ' + . 'Invalid JS url, it has to be shortened url starting with / or moodle_url instance. ' + . '(' . $wwwroot . '/lib/javascript.php/1/h5p/js/embed.js)' + ], + 'Provide an external absolute URL' => [ + 'https://maps.googleapis.com/maps/api/js?key=googlemapkey3&sensor=false', + 'Coding error detected, it must be fixed by a programmer: ' + . 'Invalid JS url, it has to be shortened url starting with / or moodle_url instance. ' + . '(https://maps.googleapis.com/maps/api/js?key=googlemapkey3&sensor=false)' + ], + 'A non-JS internal resource using an absolute URL' => [ + $wwwroot . '/theme/boost/pix/favicon.ico', + 'Coding error detected, it must be fixed by a programmer: ' + . 'Invalid JS url, it has to be shortened url starting with / or moodle_url instance. (' + . $wwwroot . '/theme/boost/pix/favicon.ico)' + ], + 'A non-existant internal resource using an absolute URL' => [ + $wwwroot . '/path/to/file.ext', + 'Coding error detected, it must be fixed by a programmer: ' + . 'Invalid JS url, it has to be shortened url starting with / or moodle_url instance. (' + . $wwwroot . '/path/to/file.ext)' + ], + 'A non-existant internal resource. WARN the developer!' => [ + '/path/to/file1.ext', + 'Coding error detected, it must be fixed by a programmer: ' + . 'Attempt to require a JavaScript file that does not exist. (/path/to/file1.ext)' + ], + 'A non-existant internal resource using moodle_url. WARN the developer!' => [ + new \moodle_url('/path/to/file2.ext'), + 'Coding error detected, it must be fixed by a programmer: ' + . 'Attempt to require a JavaScript file that does not exist. (/path/to/file2.ext)' + ], + ]; + } } diff --git a/lib/tests/weblib_test.php b/lib/tests/weblib_test.php index 6b93afa3f1b2c..9dc91dbac758b 100644 --- a/lib/tests/weblib_test.php +++ b/lib/tests/weblib_test.php @@ -985,4 +985,138 @@ public function get_html_lang_attribute_value_provider() { public function test_get_html_lang_attribute_value(string $langcode, string $expected): void { $this->assertEquals($expected, get_html_lang_attribute_value($langcode)); } + + /** + * Test the coding exceptions when returning URL as relative path from $CFG->wwwroot. + * + * @param moodle_url $url The URL pointing to a web resource. + * @param string $exmessage The expected output URL. + * @throws coding_exception If called on a non-local URL. + * @see \moodle_url::out_as_local_url() + * @covers \moodle_url::out_as_local_url + * @dataProvider out_as_local_url_coding_exception_provider + */ + public function test_out_as_local_url_coding_exception(\moodle_url $url, string $exmessage) { + $this->expectException(\coding_exception::class); + $this->expectExceptionMessage($exmessage); + $localurl = $url->out_as_local_url(); + } + + /** + * Data provider for throwing coding exceptions in \moodle_url::out_as_local_url(). + * + * @return array + * @throws moodle_exception On seriously malformed URLs (parse_url). + * @see \moodle_url::out_as_local_url() + * @see parse_url() + */ + public function out_as_local_url_coding_exception_provider() { + return [ + 'Google Maps CDN (HTTPS)' => [ + new \moodle_url('https://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + 'Coding error detected, it must be fixed by a programmer: out_as_local_url called on a non-local URL' + ], + 'Google Maps CDN (HTTP)' => [ + new \moodle_url('http://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + 'Coding error detected, it must be fixed by a programmer: out_as_local_url called on a non-local URL' + ], + ]; + } + + /** + * Test URL as relative path from $CFG->wwwroot. + * + * @param moodle_url $url The URL pointing to a web resource. + * @param string $expected The expected local URL. + * @throws coding_exception If called on a non-local URL. + * @see \moodle_url::out_as_local_url() + * @covers \moodle_url::out_as_local_url + * @dataProvider out_as_local_url_provider + */ + public function test_out_as_local_url(\moodle_url $url, string $expected) { + $this->assertEquals($expected, $url->out_as_local_url(false)); + } + + /** + * Data provider for returning local paths via \moodle_url::out_as_local_url(). + * + * @return array + * @throws moodle_exception On seriously malformed URLs (parse_url). + * @see \moodle_url::out_as_local_url() + * @see parse_url() + */ + public function out_as_local_url_provider() { + global $CFG; + $wwwroot = rtrim($CFG->wwwroot, '/'); + + return [ + 'Environment XML file' => [ + new \moodle_url('/admin/environment.xml'), + '/admin/environment.xml' + ], + 'H5P JS internal resource' => [ + new \moodle_url('/h5p/js/embed.js'), + '/h5p/js/embed.js' + ], + 'A Moodle JS resource using the full path including the proper JS Handler' => [ + new \moodle_url($wwwroot . '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/M.m.p/tiny_mce.js'), + '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/M.m.p/tiny_mce.js' + ], + ]; + } + + /** + * Test URL as relative path from $CFG->wwwroot. + * + * @param moodle_url $url The URL pointing to a web resource. + * @param bool $expected The expected result. + * @see \moodle_url::is_local_url() + * @covers \moodle_url::is_local_url + * @dataProvider is_local_url_provider + */ + public function test_is_local_url(\moodle_url $url, bool $expected) { + $this->assertEquals($expected, $url->is_local_url(), "'{$url}' is not a local URL!"); + } + + /** + * Data provider for testing \moodle_url::is_local_url(). + * + * @return array + * @see \moodle_url::is_local_url() + */ + public function is_local_url_provider() { + global $CFG; + $wwwroot = rtrim($CFG->wwwroot, '/'); + + return [ + 'Google Maps CDN (HTTPS)' => [ + new \moodle_url('https://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + false + ], + 'Google Maps CDN (HTTP)' => [ + new \moodle_url('http://maps.googleapis.com/maps/api/js', ['key' => 'googlemapkey3', 'sensor' => 'false']), + false + ], + 'wwwroot' => [ + new \moodle_url($wwwroot), + true + ], + 'wwwroot/' => [ + new \moodle_url($wwwroot . '/'), + true + ], + 'Environment XML file' => [ + new \moodle_url('/admin/environment.xml'), + true + ], + 'H5P JS internal resource' => [ + new \moodle_url('/h5p/js/embed.js'), + true + ], + 'A Moodle JS resource using the full path including the proper JS Handler' => [ + new \moodle_url($wwwroot . '/lib/javascript.php/1/lib/editor/tinymce/tiny_mce/M.m.p/tiny_mce.js'), + true + ], + ]; + } } diff --git a/lib/weblib.php b/lib/weblib.php index 16ebfb5ea0176..a2036998d92f9 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -872,7 +872,20 @@ public static function make_legacyfile_url($courseid, $filepath, $forcedownload } /** - * Returns URL a relative path from $CFG->wwwroot + * Checks if URL is relative to $CFG->wwwroot. + * + * @return bool True if URL is relative to $CFG->wwwroot; otherwise, false. + */ + public function is_local_url() : bool { + global $CFG; + + $url = $this->out(); + // Does URL start with wwwroot? Otherwise, URL isn't relative to wwwroot. + return ( ($url === $CFG->wwwroot) || (strpos($url, $CFG->wwwroot.'/') === 0) ); + } + + /** + * Returns URL as relative path from $CFG->wwwroot * * Can be used for passing around urls with the wwwroot stripped * @@ -884,10 +897,9 @@ public static function make_legacyfile_url($courseid, $filepath, $forcedownload public function out_as_local_url($escaped = true, array $overrideparams = null) { global $CFG; - $url = $this->out($escaped, $overrideparams); - - // Url should be equal to wwwroot. If not then throw exception. - if (($url === $CFG->wwwroot) || (strpos($url, $CFG->wwwroot.'/') === 0)) { + // URL should be relative to wwwroot. If not then throw exception. + if ($this->is_local_url()) { + $url = $this->out($escaped, $overrideparams); $localurl = substr($url, strlen($CFG->wwwroot)); return !empty($localurl) ? $localurl : ''; } else {