Merge branch 'm42_MDL-72461_Moodle_JS_Files_Should_Be_Always_Served_By_JS_Handler' of https://github.com/scara/moodle

lib/outputrequirementslib.php

@@ -710,15 +710,28 @@ class page_requirements_manager {
     }
 
     /**
-     * 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 @@ class page_requirements_manager {
             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);

lib/tests/outputrequirementslib_test.php

@@ -135,4 +135,268 @@ class outputrequirementslib_test extends \advanced_testcase {
         $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 <u>moodle_url</u> 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 <u>\moodle_url</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 <u>\page_requirements_manager::js_fix_url()</u>.
+     *
+     * @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)'
+            ],
+        ];
+    }
 }

lib/tests/weblib_test.php

@@ -985,4 +985,138 @@ EXPECTED;
     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 <u>\moodle_url::out_as_local_url()</u>.
+     *
+     * @return array
+     * @throws moodle_exception On seriously malformed URLs (<u>parse_url</u>).
+     * @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 <u>\moodle_url::out_as_local_url()</u>.
+     *
+     * @return array
+     * @throws moodle_exception On seriously malformed URLs (<u>parse_url</u>).
+     * @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 <u>\moodle_url::is_local_url()</u>.
+     *
+     * @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
+            ],
+        ];
+    }
 }

lib/weblib.php

@@ -872,7 +872,20 @@ class moodle_url {
     }
 
     /**
-     * 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 @@ class moodle_url {
     public function out_as_local_url($escaped = true, array $overrideparams = null) {
         global $CFG;
 
-        $url = $this->out($escaped, $overrideparams);
+        // URL should be relative to wwwroot. If not then throw exception.
-
+        if ($this->is_local_url()) {
-        // Url should be equal to wwwroot. If not then throw exception.
+            $url = $this->out($escaped, $overrideparams);
-        if (($url === $CFG->wwwroot) || (strpos($url, $CFG->wwwroot.'/') === 0)) {
             $localurl = substr($url, strlen($CFG->wwwroot));
             return !empty($localurl) ? $localurl : '';
         } else {