From 4d2ad0bff7107b8d320ff86afc68cac79cc2b920 Mon Sep 17 00:00:00 2001 From: meirzamoodle Date: Tue, 10 Sep 2024 14:09:22 +0700 Subject: [PATCH] MDL-62640 core: Adjust code for slashargument removal --- .upgradenotes/MDL-62640-2024082911475525.yml | 9 ++ ...store_structure_parser_processor.class.php | 9 +- ...estore_structure_parser_processor_test.php | 86 +++++---------- config-dist.php | 6 - lib/amd/build/url.min.js | 2 +- lib/amd/build/url.min.js.map | 2 +- lib/amd/src/url.js | 6 +- .../page_requirements_manager.php | 51 +++------ lib/classes/output/theme_config.php | 55 +++------ lib/classes/url.php | 56 +++++----- lib/filelib.php | 28 ++--- lib/javascript-static.js | 13 +-- lib/tests/filelib_test.php | 71 ++++++------ lib/tests/outputcomponents_test.php | 4 +- lib/tests/outputrequirementslib_test.php | 101 ++++------------- lib/tests/url_test.php | 36 +----- lib/tests/weblib_test.php | 104 ++++++------------ 17 files changed, 196 insertions(+), 443 deletions(-) create mode 100644 .upgradenotes/MDL-62640-2024082911475525.yml diff --git a/.upgradenotes/MDL-62640-2024082911475525.yml b/.upgradenotes/MDL-62640-2024082911475525.yml new file mode 100644 index 00000000000..0b859809c00 --- /dev/null +++ b/.upgradenotes/MDL-62640-2024082911475525.yml @@ -0,0 +1,9 @@ +issueNumber: MDL-62640 +notes: + core: + - message: | + The code that is associated with the slashargument option has been deprecated. Here is the detail: + - The parameter $support of the \url::set_slashargument(). + - The parameter $includeslashargument of the \url::get_path(). + - The $CFG->yuislasharguments configuration. + type: deprecated diff --git a/backup/util/helper/restore_structure_parser_processor.class.php b/backup/util/helper/restore_structure_parser_processor.class.php index 67a7a2d9213..2304ca9ae4b 100644 --- a/backup/util/helper/restore_structure_parser_processor.class.php +++ b/backup/util/helper/restore_structure_parser_processor.class.php @@ -63,13 +63,8 @@ class restore_structure_parser_processor extends grouped_parser_processor { if (strpos($cdata, '$@FILEPHP@$') !== false) { // We need to convert $@FILEPHP@$. - if ($CFG->slasharguments) { - $slash = '/'; - $forcedownload = '?forcedownload=1'; - } else { - $slash = '%2F'; - $forcedownload = '&forcedownload=1'; - } + $slash = '/'; + $forcedownload = '?forcedownload=1'; // We have to remove trailing slashes, otherwise file URLs will be restored with an extra slash. $basefileurl = rtrim(moodle_url::make_legacyfile_url($this->courseid, null)->out(true), $slash); diff --git a/backup/util/helper/tests/restore_structure_parser_processor_test.php b/backup/util/helper/tests/restore_structure_parser_processor_test.php index 7f74ec7cef1..1e88aa33bd5 100644 --- a/backup/util/helper/tests/restore_structure_parser_processor_test.php +++ b/backup/util/helper/tests/restore_structure_parser_processor_test.php @@ -23,6 +23,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +namespace core_backup; + defined('MOODLE_INTERNAL') || die(); global $CFG; @@ -36,8 +38,9 @@ require_once($CFG->dirroot . '/backup/util/helper/restore_structure_parser_proce * @package core_backup * @copyright 2017 Dmitrii Metelkin (dmitriim@catalyst-au.net) * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core_backup\restore_structure_parser_processor */ -class restore_structure_parser_processor_test extends advanced_testcase { +final class restore_structure_parser_processor_test extends \advanced_testcase { /** * Initial set up. @@ -54,83 +57,52 @@ class restore_structure_parser_processor_test extends advanced_testcase { * @return array */ public function process_cdata_data_provider() { - return array( - array(null, null, true), - array("$@NULL@$", null, true), - array("$@NULL@$ ", "$@NULL@$ ", true), - array(1, 1, true), - array(" ", " ", true), - array("1", "1", true), - array("$@FILEPHP@$1.jpg", "$@FILEPHP@$1.jpg", true), - array( + return [ + [null, null], + ["$@NULL@$", null], + ["$@NULL@$ ", "$@NULL@$ "], + [1, 1], + [" ", " "], + ["1", "1"], + ["$@FILEPHP@$1.jpg", "$@FILEPHP@$1.jpg"], + [ "http://test.test/$@SLASH@$", "http://test.test/$@SLASH@$", - true - ), - array( + ], + [ "Image", "Image", - true - ), - array( + ], + [ "Image", "Image", - true - ), - array( + ], + [ "Image", "Image", - true - ), - array( - "Image", - "Image", - false - ), - array( - "Image", - "Image", - false - ), - array( - "Image", - "Image", - false - ), - array( + ], + [ "Image", "Image", - true - ), - array( - "Image", - "Image", - false - ), - array( + ], + [ "", "", - true - ), - ); + ], + ]; } /** * Test that restore_structure_parser_processor replaces $@FILEPHP@$ to correct file php links. * * @dataProvider process_cdata_data_provider - * @param string $content Testing content. - * @param string $expected Expected result. - * @param bool $slasharguments A value for $CFG->slasharguments setting. + * @param mixed $content Testing content. + * @param mixed $expected Expected result. */ - public function test_process_cdata($content, $expected, $slasharguments): void { + public function test_process_cdata(mixed $content, mixed $expected): void { global $CFG; - - $CFG->slasharguments = $slasharguments; $CFG->wwwroot = 'http://test.test'; - - $processor = new restore_structure_parser_processor(1, 1); - + $processor = new \restore_structure_parser_processor(1, 1); $this->assertEquals($expected, $processor->process_cdata($content)); } diff --git a/config-dist.php b/config-dist.php index c0fb1163afb..f423c9eaea3 100644 --- a/config-dist.php +++ b/config-dist.php @@ -329,12 +329,6 @@ $CFG->admin = 'admin'; // Please note: It is *not* possible to use X-Sendfile with the per-request directory. // The directory is highly likely to have been deleted by the time the web server sends the file. // -// YUI caching may be sometimes improved by slasharguments: -// $CFG->yuislasharguments = 1; -// Some servers may need a special rewrite rule to work around internal path length limitations: -// RewriteRule (^.*/theme/yui_combo\.php)(/.*) $1?file=$2 -// -// // Following settings may be used to select session driver, uncomment only one of the handlers. // Database session handler: // $CFG->session_handler_class = '\core\session\database'; diff --git a/lib/amd/build/url.min.js b/lib/amd/build/url.min.js index 182a191f41c..2d2e913bf30 100644 --- a/lib/amd/build/url.min.js +++ b/lib/amd/build/url.min.js @@ -6,6 +6,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @since 2.9 */ -define("core/url",["jquery","core/config"],(function($,config){return{fileUrl:function(relativeScript,slashArg){var url=config.wwwroot+relativeScript;return"/"!=slashArg.charAt(0)&&(slashArg="/"+slashArg),config.slasharguments?url+=slashArg:url+="?file="+encodeURIComponent(slashArg),url},relativeUrl:function(relativePath,params,includeSessKey){if(0===relativePath.indexOf("http:")||0===relativePath.indexOf("https:")||relativePath.indexOf("://")>=0)throw new Error("relativeUrl function does not accept absolute urls");"/"!=relativePath.charAt(0)&&(relativePath="/"+relativePath),"admin"!==config.admin&&(relativePath=relativePath.replace(/^\/admin\//,"/"+config.admin+"/")),params=params||{},includeSessKey&&(params.sesskey=config.sesskey);var queryString="";return Object.keys(params).length&&(queryString=$.map(params,(function(value,param){return param+"="+value})).join("&")),""!==queryString?config.wwwroot+relativePath+"?"+queryString:config.wwwroot+relativePath},imageUrl:function(imagename,component){return M.util.image_url(imagename,component)}}})); +define("core/url",["jquery","core/config"],(function($,config){return{fileUrl:function(relativeScript,slashArg){var url=config.wwwroot+relativeScript;return"/"!=slashArg.charAt(0)&&(slashArg="/"+slashArg),url+=slashArg},relativeUrl:function(relativePath,params,includeSessKey){if(0===relativePath.indexOf("http:")||0===relativePath.indexOf("https:")||relativePath.indexOf("://")>=0)throw new Error("relativeUrl function does not accept absolute urls");"/"!=relativePath.charAt(0)&&(relativePath="/"+relativePath),"admin"!==config.admin&&(relativePath=relativePath.replace(/^\/admin\//,"/"+config.admin+"/")),params=params||{},includeSessKey&&(params.sesskey=config.sesskey);var queryString="";return Object.keys(params).length&&(queryString=$.map(params,(function(value,param){return param+"="+value})).join("&")),""!==queryString?config.wwwroot+relativePath+"?"+queryString:config.wwwroot+relativePath},imageUrl:function(imagename,component){return M.util.image_url(imagename,component)}}})); //# sourceMappingURL=url.min.js.map \ No newline at end of file diff --git a/lib/amd/build/url.min.js.map b/lib/amd/build/url.min.js.map index e76a3879fba..74951a8f560 100644 --- a/lib/amd/build/url.min.js.map +++ b/lib/amd/build/url.min.js.map @@ -1 +1 @@ -{"version":3,"file":"url.min.js","sources":["../src/url.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * URL utility functions.\n *\n * @module core/url\n * @copyright 2015 Damyon Wiese \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n * @since 2.9\n */\ndefine(['jquery', 'core/config'], function($, config) {\n\n\n return /** @alias module:core/url */ {\n // Public variables and functions.\n /**\n * Construct a file url\n *\n * @method fileUrl\n * @param {string} relativeScript\n * @param {string} slashArg\n * @return {string}\n */\n fileUrl: function(relativeScript, slashArg) {\n\n var url = config.wwwroot + relativeScript;\n\n // Force a /\n if (slashArg.charAt(0) != '/') {\n slashArg = '/' + slashArg;\n }\n if (config.slasharguments) {\n url += slashArg;\n } else {\n url += '?file=' + encodeURIComponent(slashArg);\n }\n return url;\n },\n\n /**\n * Take a path relative to the moodle basedir and do some fixing (see class moodle_url in php).\n *\n * @method relativeUrl\n * @param {string} relativePath The path relative to the moodle basedir.\n * @param {object} params The query parameters for the URL.\n * @param {bool} includeSessKey Add the session key to the query params.\n * @return {string}\n */\n relativeUrl: function(relativePath, params, includeSessKey) {\n\n if (relativePath.indexOf('http:') === 0 || relativePath.indexOf('https:') === 0 || relativePath.indexOf('://') >= 0) {\n throw new Error('relativeUrl function does not accept absolute urls');\n }\n\n // Fix non-relative paths;\n if (relativePath.charAt(0) != '/') {\n relativePath = '/' + relativePath;\n }\n\n // Fix admin urls.\n if (config.admin !== 'admin') {\n relativePath = relativePath.replace(/^\\/admin\\//, '/' + config.admin + '/');\n }\n\n params = params || {};\n if (includeSessKey) {\n params.sesskey = config.sesskey;\n }\n\n var queryString = '';\n if (Object.keys(params).length) {\n queryString = $.map(params, function(value, param) {\n return param + '=' + value;\n }).join('&');\n }\n\n if (queryString !== '') {\n return config.wwwroot + relativePath + '?' + queryString;\n } else {\n return config.wwwroot + relativePath;\n }\n },\n\n /**\n * Wrapper for image_url function.\n *\n * @method imageUrl\n * @param {string} imagename The image name (e.g. t/edit).\n * @param {string} component The component (e.g. mod_feedback).\n * @return {string}\n */\n imageUrl: function(imagename, component) {\n return M.util.image_url(imagename, component);\n }\n };\n});\n"],"names":["define","$","config","fileUrl","relativeScript","slashArg","url","wwwroot","charAt","slasharguments","encodeURIComponent","relativeUrl","relativePath","params","includeSessKey","indexOf","Error","admin","replace","sesskey","queryString","Object","keys","length","map","value","param","join","imageUrl","imagename","component","M","util","image_url"],"mappings":";;;;;;;;AAuBAA,kBAAO,CAAC,SAAU,gBAAgB,SAASC,EAAGC,cAGL,CAUjCC,QAAS,SAASC,eAAgBC,cAE1BC,IAAMJ,OAAOK,QAAUH,qBAGD,KAAtBC,SAASG,OAAO,KAChBH,SAAW,IAAMA,UAEjBH,OAAOO,eACPH,KAAOD,SAEPC,KAAO,SAAWI,mBAAmBL,UAElCC,KAYXK,YAAa,SAASC,aAAcC,OAAQC,mBAEF,IAAlCF,aAAaG,QAAQ,UAAqD,IAAnCH,aAAaG,QAAQ,WAAmBH,aAAaG,QAAQ,QAAU,QACxG,IAAIC,MAAM,sDAIU,KAA1BJ,aAAaJ,OAAO,KACpBI,aAAe,IAAMA,cAIJ,UAAjBV,OAAOe,QACPL,aAAeA,aAAaM,QAAQ,aAAc,IAAMhB,OAAOe,MAAQ,MAG3EJ,OAASA,QAAU,GACfC,iBACAD,OAAOM,QAAUjB,OAAOiB,aAGxBC,YAAc,UACdC,OAAOC,KAAKT,QAAQU,SACpBH,YAAcnB,EAAEuB,IAAIX,QAAQ,SAASY,MAAOC,cACjCA,MAAQ,IAAMD,SACtBE,KAAK,MAGQ,KAAhBP,YACOlB,OAAOK,QAAUK,aAAe,IAAMQ,YAEtClB,OAAOK,QAAUK,cAYhCgB,SAAU,SAASC,UAAWC,kBACnBC,EAAEC,KAAKC,UAAUJ,UAAWC"} \ No newline at end of file +{"version":3,"file":"url.min.js","sources":["../src/url.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * URL utility functions.\n *\n * @module core/url\n * @copyright 2015 Damyon Wiese \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n * @since 2.9\n */\ndefine(['jquery', 'core/config'], function($, config) {\n\n\n return /** @alias module:core/url */ {\n // Public variables and functions.\n /**\n * Construct a file url\n *\n * @method fileUrl\n * @param {string} relativeScript\n * @param {string} slashArg\n * @return {string}\n */\n fileUrl: function(relativeScript, slashArg) {\n\n var url = config.wwwroot + relativeScript;\n\n // Force a /\n if (slashArg.charAt(0) != '/') {\n slashArg = '/' + slashArg;\n }\n url += slashArg;\n return url;\n },\n\n /**\n * Take a path relative to the moodle basedir and do some fixing (see class moodle_url in php).\n *\n * @method relativeUrl\n * @param {string} relativePath The path relative to the moodle basedir.\n * @param {object} params The query parameters for the URL.\n * @param {bool} includeSessKey Add the session key to the query params.\n * @return {string}\n */\n relativeUrl: function(relativePath, params, includeSessKey) {\n\n if (relativePath.indexOf('http:') === 0 || relativePath.indexOf('https:') === 0 || relativePath.indexOf('://') >= 0) {\n throw new Error('relativeUrl function does not accept absolute urls');\n }\n\n // Fix non-relative paths;\n if (relativePath.charAt(0) != '/') {\n relativePath = '/' + relativePath;\n }\n\n // Fix admin urls.\n if (config.admin !== 'admin') {\n relativePath = relativePath.replace(/^\\/admin\\//, '/' + config.admin + '/');\n }\n\n params = params || {};\n if (includeSessKey) {\n params.sesskey = config.sesskey;\n }\n\n var queryString = '';\n if (Object.keys(params).length) {\n queryString = $.map(params, function(value, param) {\n return param + '=' + value;\n }).join('&');\n }\n\n if (queryString !== '') {\n return config.wwwroot + relativePath + '?' + queryString;\n } else {\n return config.wwwroot + relativePath;\n }\n },\n\n /**\n * Wrapper for image_url function.\n *\n * @method imageUrl\n * @param {string} imagename The image name (e.g. t/edit).\n * @param {string} component The component (e.g. mod_feedback).\n * @return {string}\n */\n imageUrl: function(imagename, component) {\n return M.util.image_url(imagename, component);\n }\n };\n});\n"],"names":["define","$","config","fileUrl","relativeScript","slashArg","url","wwwroot","charAt","relativeUrl","relativePath","params","includeSessKey","indexOf","Error","admin","replace","sesskey","queryString","Object","keys","length","map","value","param","join","imageUrl","imagename","component","M","util","image_url"],"mappings":";;;;;;;;AAuBAA,kBAAO,CAAC,SAAU,gBAAgB,SAASC,EAAGC,cAGL,CAUjCC,QAAS,SAASC,eAAgBC,cAE1BC,IAAMJ,OAAOK,QAAUH,qBAGD,KAAtBC,SAASG,OAAO,KAChBH,SAAW,IAAMA,UAErBC,KAAOD,UAaXI,YAAa,SAASC,aAAcC,OAAQC,mBAEF,IAAlCF,aAAaG,QAAQ,UAAqD,IAAnCH,aAAaG,QAAQ,WAAmBH,aAAaG,QAAQ,QAAU,QACxG,IAAIC,MAAM,sDAIU,KAA1BJ,aAAaF,OAAO,KACpBE,aAAe,IAAMA,cAIJ,UAAjBR,OAAOa,QACPL,aAAeA,aAAaM,QAAQ,aAAc,IAAMd,OAAOa,MAAQ,MAG3EJ,OAASA,QAAU,GACfC,iBACAD,OAAOM,QAAUf,OAAOe,aAGxBC,YAAc,UACdC,OAAOC,KAAKT,QAAQU,SACpBH,YAAcjB,EAAEqB,IAAIX,QAAQ,SAASY,MAAOC,cACjCA,MAAQ,IAAMD,SACtBE,KAAK,MAGQ,KAAhBP,YACOhB,OAAOK,QAAUG,aAAe,IAAMQ,YAEtChB,OAAOK,QAAUG,cAYhCgB,SAAU,SAASC,UAAWC,kBACnBC,EAAEC,KAAKC,UAAUJ,UAAWC"} \ No newline at end of file diff --git a/lib/amd/src/url.js b/lib/amd/src/url.js index 47929eed9a5..a9f6e5886b6 100644 --- a/lib/amd/src/url.js +++ b/lib/amd/src/url.js @@ -42,11 +42,7 @@ define(['jquery', 'core/config'], function($, config) { if (slashArg.charAt(0) != '/') { slashArg = '/' + slashArg; } - if (config.slasharguments) { - url += slashArg; - } else { - url += '?file=' + encodeURIComponent(slashArg); - } + url += slashArg; return url; }, diff --git a/lib/classes/output/requirements/page_requirements_manager.php b/lib/classes/output/requirements/page_requirements_manager.php index e6b86afd28b..4d0c1e7552b 100644 --- a/lib/classes/output/requirements/page_requirements_manager.php +++ b/lib/classes/output/requirements/page_requirements_manager.php @@ -174,15 +174,12 @@ class page_requirements_manager { public function __construct() { global $CFG; - // You may need to set up URL rewrite rule because oversized URLs might not be allowed by web server. - $sep = empty($CFG->yuislasharguments) ? '?' : '/'; - $this->yui3loader = new stdClass(); $this->YUI_config = new yui(); // Set up some loader options. $this->yui3loader->local_base = $CFG->wwwroot . '/lib/yuilib/' . $CFG->yui3version . '/'; - $this->yui3loader->local_comboBase = $CFG->wwwroot . '/theme/yui_combo.php' . $sep; + $this->yui3loader->local_comboBase = $CFG->wwwroot . '/theme/yui_combo.php/'; $this->yui3loader->base = $this->yui3loader->local_base; $this->yui3loader->comboBase = $this->yui3loader->local_comboBase; @@ -211,7 +208,7 @@ class page_requirements_manager { $this->YUI_config->add_group('yui2', [ // Loader configuration for our 2in3. 'base' => $CFG->wwwroot . '/lib/yuilib/2in3/' . $CFG->yui2version . '/build/', - 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php' . $sep, + 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php/', 'combine' => $this->yui3loader->combine, 'ext' => false, 'root' => '2in3/' . $CFG->yui2version . '/build/', @@ -225,9 +222,9 @@ class page_requirements_manager { $configname = $this->YUI_config->set_config_source('lib/yui/config/moodle.js'); $this->YUI_config->add_group('moodle', [ 'name' => 'moodle', - 'base' => $CFG->wwwroot . '/theme/yui_combo.php' . $sep . 'm/' . $jsrev . '/', + 'base' => $CFG->wwwroot . '/theme/yui_combo.php/m/' . $jsrev . '/', 'combine' => $this->yui3loader->combine, - 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php' . $sep, + 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php/', 'ext' => false, 'root' => 'm/' . $jsrev . '/', // Add the rev to the root path so that we can control caching. 'patterns' => [ @@ -242,7 +239,7 @@ class page_requirements_manager { 'name' => 'gallery', 'base' => $CFG->wwwroot . '/lib/yuilib/gallery/', 'combine' => $this->yui3loader->combine, - 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php' . $sep, + 'comboBase' => $CFG->wwwroot . '/theme/yui_combo.php/', 'ext' => false, 'root' => 'gallery/' . $jsrev . '/', 'patterns' => [ @@ -321,7 +318,6 @@ class page_requirements_manager { 'sessiontimeout' => $CFG->sessiontimeout, 'sessiontimeoutwarning' => $CFG->sessiontimeoutwarning, 'themerev' => theme_get_revision(), - 'slasharguments' => (int)(!empty($CFG->slasharguments)), 'theme' => $page->theme->name, 'iconsystemmodule' => $iconsystem->get_amd_name(), 'jsrev' => $this->get_jsrev(), @@ -606,23 +602,9 @@ class page_requirements_manager { debugging("Invalid file '$file' specified in jQuery plugin '$plugin' in component '$component'"); continue; } - if (!empty($CFG->slasharguments)) { - $url = new moodle_url("/theme/jquery.php"); - $url->set_slashargument("/$component/$file"); - } else { - // This is not really good, we need slasharguments for relative links, this means no caching... - $path = realpath("$componentdir/jquery/$file"); - if (strpos($path, $CFG->dirroot) === 0) { - $url = $CFG->wwwroot . preg_replace('/^' . preg_quote($CFG->dirroot, '/') . '/', '', $path); - // Replace all occurences of backslashes characters in url to forward slashes. - $url = str_replace('\\', '/', $url); - $url = new moodle_url($url); - } else { - // Bad luck, fix your server! - debugging("Moodle jQuery integration requires 'slasharguments' setting to be enabled."); - continue; - } - } + $url = new moodle_url("/theme/jquery.php"); + $url->set_slashargument("/$component/$file"); + $this->jqueryplugins[$plugin]->urls[] = $url; } @@ -769,13 +751,9 @@ class page_requirements_manager { } if (substr($url, -3) === '.js') { $jsrev = $this->get_jsrev(); - if (empty($CFG->slasharguments)) { - return new moodle_url('/lib/javascript.php', ['rev' => $jsrev, 'jsfile' => $url]); - } else { - $returnurl = new moodle_url('/lib/javascript.php'); - $returnurl->set_slashargument('/' . $jsrev . $url); - return $returnurl; - } + $returnurl = new moodle_url('/lib/javascript.php'); + $returnurl->set_slashargument('/' . $jsrev . $url); + return $returnurl; } else { return new moodle_url($url); } @@ -1469,11 +1447,8 @@ class page_requirements_manager { $requirejsconfig = file_get_contents($CFG->dirroot . '/lib/requirejs/moodle-config.js'); - // No extension required unless slash args is disabled. - $jsextension = '.js'; - if (!empty($CFG->slasharguments)) { - $jsextension = ''; - } + // No extension required. + $jsextension = ''; $minextension = '.min'; if (!$cachejs) { diff --git a/lib/classes/output/theme_config.php b/lib/classes/output/theme_config.php index ae937078c93..5f47351ef0e 100644 --- a/lib/classes/output/theme_config.php +++ b/lib/classes/output/theme_config.php @@ -682,15 +682,7 @@ class theme_config { } $url = new moodle_url("/theme/styles.php"); - if (!empty($CFG->slasharguments)) { - $url->set_slashargument("/{$this->name}/{$rev}/{$type}", 'noparam', true); - } else { - $url->params([ - 'theme' => $this->name, - 'rev' => $rev, - 'type' => $type, - ]); - } + $url->set_slashargument("/{$this->name}/{$rev}/{$type}", 'noparam'); } else { $url = new moodle_url('/theme/styles_debug.php', [ 'theme' => $this->name, @@ -825,30 +817,17 @@ class theme_config { $rev .= "_{$themesubrevision}"; } - if (!empty($CFG->slasharguments)) { - $slashargs = ''; - if (!$svg) { - // We add a simple /_s to the start of the path. - // The underscore is used to ensure that it isn't a valid theme name. - $slashargs .= '/_s' . $slashargs; - } - $slashargs .= '/' . $this->name . '/' . $rev . '/' . $filename; - if ($separate) { - $slashargs .= '/chunk0'; - } - $url->set_slashargument($slashargs, 'noparam', true); - } else { - $params = ['theme' => $this->name, 'rev' => $rev, 'type' => $filename]; - if (!$svg) { - // We add an SVG param so that we know not to serve SVG images. - // We do this because all modern browsers support SVG and this param will one day be removed. - $params['svg'] = '0'; - } - if ($separate) { - $params['chunk'] = '0'; - } - $url->params($params); + $slashargs = ''; + if (!$svg) { + // We add a simple /_s to the start of the path. + // The underscore is used to ensure that it isn't a valid theme name. + $slashargs .= '/_s' . $slashargs; } + $slashargs .= '/' . $this->name . '/' . $rev . '/' . $filename; + if ($separate) { + $slashargs .= '/chunk0'; + } + $url->set_slashargument($slashargs, 'noparam'); $urls[] = $url; } else { $baseurl = new moodle_url('/theme/styles_debug.php'); @@ -1457,9 +1436,9 @@ class theme_config { return null; } - if (!empty($CFG->slasharguments) && $rev > 0) { + if ($rev > 0) { $url = new moodle_url("/theme/javascript.php"); - $url->set_slashargument('/' . $this->name . '/' . $rev . '/' . $params['type'], 'noparam', true); + $url->set_slashargument('/' . $this->name . '/' . $rev . '/' . $params['type'], 'noparam'); return $url; } else { return new moodle_url('/theme/javascript.php', $params); @@ -1697,14 +1676,14 @@ class theme_config { $params['image'] = $imagename; $url = new moodle_url("/theme/image.php"); - if (!empty($CFG->slasharguments) && $rev > 0) { + if ($rev > 0) { $path = '/' . $params['theme'] . '/' . $params['component'] . '/' . $params['rev'] . '/' . $params['image']; if (!$svg) { // We add a simple /_s to the start of the path. // The underscore is used to ensure that it isn't a valid theme name. $path = '/_s' . $path; } - $url->set_slashargument($path, 'noparam', true); + $url->set_slashargument($path, 'noparam'); } else { if (!$svg) { // We add an SVG param so that we know not to serve SVG images. @@ -1743,9 +1722,9 @@ class theme_config { $params['font'] = $font; $url = new moodle_url("/theme/font.php"); - if (!empty($CFG->slasharguments) && $rev > 0) { + if ($rev > 0) { $path = '/' . $params['theme'] . '/' . $params['component'] . '/' . $params['rev'] . '/' . $params['font']; - $url->set_slashargument($path, 'noparam', true); + $url->set_slashargument($path, 'noparam'); } else { $url->params($params); } diff --git a/lib/classes/url.php b/lib/classes/url.php index d55ee3688db..39dbd627e27 100644 --- a/lib/classes/url.php +++ b/lib/classes/url.php @@ -580,24 +580,21 @@ class url { * * @param string $path usually file path * @param string $parameter name of page parameter if slasharguments not supported - * @param bool $supported usually null, then it depends on $CFG->slasharguments, use true or false for other servers + * @param bool|null $supported This parameter has been deprecated since 4.5 and should not be used anymore. + * @todo Final deprecation on Moodle 6.0. See MDL-82768. */ - public function set_slashargument($path, $parameter = 'file', $supported = null) { - global $CFG; - if (is_null($supported)) { - $supported = !empty($CFG->slasharguments); - } - - if ($supported) { - $parts = explode('/', $path); - $parts = array_map('rawurlencode', $parts); - $path = implode('/', $parts); - $this->slashargument = $path; - unset($this->params[$parameter]); - } else { - $this->slashargument = ''; - $this->params[$parameter] = $path; + public function set_slashargument($path, $parameter = 'file', ?bool $supported = null) { + if ($supported !== null) { + debugging( + 'Deprecated argument passed to ' . __FUNCTION__, + DEBUG_DEVELOPER, + ); } + $parts = explode('/', $path); + $parts = array_map('rawurlencode', $parts); + $path = implode('/', $parts); + $this->slashargument = $path; + unset($this->params[$parameter]); } // Static factory methods. @@ -700,9 +697,7 @@ class url { $urlbase = "$CFG->wwwroot/tokenpluginfile.php"; $userid = $includetoken === true ? $USER->id : $includetoken; $token = get_user_key('core_files', $userid); - if ($CFG->slasharguments) { - $path[] = $token; - } + $path[] = $token; } else { $urlbase = "$CFG->wwwroot/pluginfile.php"; } @@ -716,10 +711,7 @@ class url { $path = "/" . implode('/', $path) . "{$pathname}{$filename}"; - $url = self::make_file_url($urlbase, $path, $forcedownload, $includetoken); - if ($includetoken && empty($CFG->slasharguments)) { - $url->param('token', $token); - } + $url = self::make_file_url($urlbase, $path, $forcedownload); return $url; } @@ -831,16 +823,20 @@ class url { * * By default the path includes slash-arguments (for example, * '/myfile.php/extra/arguments') so it is what you would expect from a - * URL path. If you don't want this behaviour, you can opt to exclude the - * slash arguments. (Be careful: if the $CFG variable slasharguments is - * disabled, these URLs will have a different format and you may need to - * look at the 'file' parameter too.) + * URL path. * - * @param bool $includeslashargument If true, includes slash arguments + * @param bool|null $includeslashargument This parameter has been deprecated since 4.5 and should not be used anymore. + * Final deprecation on Moodle 6.0. See MDL-82768. * @return string Path of URL */ - public function get_path($includeslashargument = true) { - return $this->path . ($includeslashargument ? $this->slashargument : ''); + public function get_path(?bool $includeslashargument = null) { + if ($includeslashargument !== null) { + debugging( + 'The includeslashargument argument has been deprecated. Please remove it from your method calls.', + DEBUG_DEVELOPER, + ); + } + return $this->path . $this->slashargument; } /** diff --git a/lib/filelib.php b/lib/filelib.php index c6e3ccd9718..09bcb166a91 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -73,20 +73,12 @@ function file_encode_url($urlbase, $path, $forcedownload=false, $https=false) { //TODO: deprecate this - if ($CFG->slasharguments) { - $parts = explode('/', $path); - $parts = array_map('rawurlencode', $parts); - $path = implode('/', $parts); - $return = $urlbase.$path; - if ($forcedownload) { - $return .= '?forcedownload=1'; - } - } else { - $path = rawurlencode($path); - $return = $urlbase.'?file='.$path; - if ($forcedownload) { - $return .= '&forcedownload=1'; - } + $parts = explode('/', $path); + $parts = array_map('rawurlencode', $parts); + $path = implode('/', $parts); + $return = $urlbase . $path; + if ($forcedownload) { + $return .= '?forcedownload=1'; } if ($https) { @@ -504,13 +496,7 @@ function file_rewrite_pluginfile_urls($text, $file, $contextid, $component, $fil $finalfile = basename($file); $tokenfile = "token{$finalfile}"; $file = substr($file, 0, strlen($file) - strlen($finalfile)) . $tokenfile; - $baseurl = "{$CFG->wwwroot}/{$file}"; - - if (!$CFG->slasharguments) { - $baseurl .= "?token={$token}&file="; - } else { - $baseurl .= "/{$token}"; - } + $baseurl = "{$CFG->wwwroot}/{$file}/{$token}"; } $baseurl .= "/{$contextid}/{$component}/{$filearea}/"; diff --git a/lib/javascript-static.js b/lib/javascript-static.js index c74d5a4a865..04c835ef6ab 100644 --- a/lib/javascript-static.js +++ b/lib/javascript-static.js @@ -42,17 +42,10 @@ M.util.image_url = function(imagename, component) { } var url = M.cfg.wwwroot + '/theme/image.php'; - if (M.cfg.themerev > 0 && M.cfg.slasharguments == 1) { - if (!M.cfg.svgicons) { - url += '/_s'; - } - url += '/' + M.cfg.theme + '/' + component + '/' + M.cfg.themerev + '/' + imagename; - } else { - url += '?theme=' + M.cfg.theme + '&component=' + component + '&rev=' + M.cfg.themerev + '&image=' + imagename; - if (!M.cfg.svgicons) { - url += '&svg=0'; - } + if (!M.cfg.svgicons) { + url += '/_s'; } + url += '/' + M.cfg.theme + '/' + component + '/' + M.cfg.themerev + '/' + imagename; return url; }; diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php index ff7fdf3256f..1b1d48d8e70 100644 --- a/lib/tests/filelib_test.php +++ b/lib/tests/filelib_test.php @@ -1317,9 +1317,7 @@ EOF; * Test file_rewrite_pluginfile_urls with includetoken. */ public function test_file_rewrite_pluginfile_urls_includetoken(): void { - global $USER, $CFG; - - $CFG->slasharguments = true; + global $USER; $this->resetAfterTest(); @@ -1362,43 +1360,6 @@ EOF; $this->assertEquals($expectedtext, $finaltext); } - /** - * Test file_rewrite_pluginfile_urls with includetoken with slasharguments disabled.. - */ - public function test_file_rewrite_pluginfile_urls_includetoken_no_slashargs(): void { - global $USER, $CFG; - - $CFG->slasharguments = false; - - $this->resetAfterTest(); - - $syscontext = \context_system::instance(); - $originaltext = 'Fake test with an image '; - $options = ['includetoken' => true]; - - // Rewrite the content. This will generate a new token. - $finaltext = file_rewrite_pluginfile_urls( - $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); - - $token = get_user_key('core_files', $USER->id); - $expectedurl = new \moodle_url("/tokenpluginfile.php"); - $expectedurl .= "?token={$token}&file=/{$syscontext->id}/user/private/0/image.png"; - $expectedtext = "Fake test with an image "; - $this->assertEquals($expectedtext, $finaltext); - - // Do it again - the second time will use an existing token. - $finaltext = file_rewrite_pluginfile_urls( - $originaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); - $this->assertEquals($expectedtext, $finaltext); - - // Now undo. - $options['reverse'] = true; - $finaltext = file_rewrite_pluginfile_urls($finaltext, 'pluginfile.php', $syscontext->id, 'user', 'private', 0, $options); - - // Compare the final text is the same that the original. - $this->assertEquals($originaltext, $finaltext); - } - /** * Helpter function to create draft files * @@ -2044,6 +2005,36 @@ EOF; ], ]; } + + /** + * Tests the file_encode_url function to ensure it generates the correct URLs. + * + * @dataProvider file_encode_url_provider + * @covers ::file_encode_url() + * + * @param string $urlbase The base URL to be used in the encoding. + * @param string $path The path to be appended to the base URL. + * @param bool $forcedownload Whether to force the file to download. + * @param bool $https Whether to use HTTPS for the URL. + * @param string $expected The expected URL after encoding. + * @return void + */ + public function test_file_encode_url(string $urlbase, string $path, bool $forcedownload, bool $https, string $expected): void { + $actual = file_encode_url($urlbase, $path, $forcedownload, $https); + $this->assertSame($expected, $actual); + } + + /** + * Provides data sets for test_file_encode_url. + */ + public static function file_encode_url_provider(): array { + return [ + ['http://local.test/', '1/1', false, false, 'http://local.test/1/1'], + ['http://local.test/', '1/number two', true, false, 'http://local.test/1/number%20two?forcedownload=1'], + ['http://local.test/', 'number@one/3', true, true, 'https://local.test/number%40one/3?forcedownload=1'], + ]; + } + } /** diff --git a/lib/tests/outputcomponents_test.php b/lib/tests/outputcomponents_test.php index 94d40347ae7..3965005881b 100644 --- a/lib/tests/outputcomponents_test.php +++ b/lib/tests/outputcomponents_test.php @@ -136,7 +136,6 @@ final class outputcomponents_test extends \advanced_testcase { // Verify new install contains expected defaults. $this->assertSame(theme_config::DEFAULT_THEME, $CFG->theme); - $this->assertEquals(1, $CFG->slasharguments); $this->assertEquals(1, $CFG->themerev); $this->assertEquals(0, $CFG->themedesignermode); $this->assertSame('https://www.example.com/moodle', $CFG->wwwroot); @@ -305,14 +304,13 @@ final class outputcomponents_test extends \advanced_testcase { // Test non-slashargument images. set_config('theme', 'classic'); $CFG->wwwroot = str_replace('https:', 'http:', $CFG->wwwroot); - $CFG->slasharguments = 0; $page = new \moodle_page(); $page->set_url('/user/profile.php'); $page->set_context(\context_system::instance()); $renderer = $page->get_renderer('core'); $up3 = new user_picture($user3); - $this->assertSame($CFG->wwwroot.'/theme/image.php?theme=classic&component=core&rev=1&image=u%2Ff2', $up3->get_url($page, $renderer)->out(false)); + $this->assertSame($CFG->wwwroot.'/theme/image.php/classic/core/1/u/f2', $up3->get_url($page, $renderer)->out(false)); } public function test_empty_menu(): void { diff --git a/lib/tests/outputrequirementslib_test.php b/lib/tests/outputrequirementslib_test.php index 32535a6a17f..d4867bafb4e 100644 --- a/lib/tests/outputrequirementslib_test.php +++ b/lib/tests/outputrequirementslib_test.php @@ -61,16 +61,13 @@ final class outputrequirementslib_test extends \advanced_testcase { /** * Test for the jquery_plugin method. * - * Test to make sure that backslashes are not generated with either slasharguments set to on or off. + * Test to make sure that backslashes are not generated. */ public function test_jquery_plugin(): void { - global $CFG, $PAGE; + global $PAGE; $this->resetAfterTest(); - // With slasharguments on. - $CFG->slasharguments = 1; - $page = new \moodle_page(); $requirements = $page->requires; // Assert successful method call. @@ -82,20 +79,6 @@ final class outputrequirementslib_test extends \advanced_testcase { $requirecode = $requirements->get_top_of_body_code($renderer); // Make sure that the generated code does not contain backslashes. $this->assertFalse(strpos($requirecode, '\\'), "Output contains backslashes: " . $requirecode); - - // With slasharguments off. - $CFG->slasharguments = 0; - - $page = new \moodle_page(); - $requirements = $page->requires; - // Assert successful method call. - $this->assertTrue($requirements->jquery_plugin('jquery')); - $this->assertTrue($requirements->jquery_plugin('ui')); - - // Get the code containing the required jquery plugins. - $requirecode = $requirements->get_top_of_body_code($renderer); - // Make sure that the generated code does not contain backslashes. - $this->assertFalse(strpos($requirecode, '\\'), "Output contains backslashes: " . $requirecode); } /** @@ -134,7 +117,6 @@ final class outputrequirementslib_test extends \advanced_testcase { * 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() @@ -142,18 +124,12 @@ final class outputrequirementslib_test extends \advanced_testcase { * @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): void { - global $CFG; - $defaultslashargs = $CFG->slasharguments; - - $CFG->slasharguments = $cfgslashargs; + public function test_js_fix_url_moodle_url(\moodle_url $moodleurl, string $expected): void { $rc = new \ReflectionClass(\page_requirements_manager::class); $rcm = $rc->getMethod('js_fix_url'); $requires = new \page_requirements_manager(); $actualmoodleurl = $rcm->invokeArgs($requires, [$moodleurl]); $this->assertEquals($expected, $actualmoodleurl->out(false)); - - $CFG->slasharguments = $defaultslashargs; } /** @@ -169,47 +145,30 @@ final class outputrequirementslib_test extends \advanced_testcase { $libdir = rtrim($CFG->libdir, '/'); $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' => [ new \moodle_url('/admin/environment.xml'), - 0, - $wwwroot . $admin . 'environment.xml' + $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' + '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' + 'http://maps.googleapis.com/maps/api/js?key=googlemapkey3&sensor=false', ], - 'H5P JS internal resource (slasharguments on)' => [ + 'H5P JS internal resource' => [ 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' + $wwwroot . '/lib/javascript.php/1/h5p/js/embed.js', ], 'A custom Moodle CSS Handler' => [ new \moodle_url('/mod/data/css.php?d=1234567890'), - 1, - $wwwroot . '/mod/data/css.php?d=1234567890' + $wwwroot . '/mod/data/css.php?d=1234567890', ], - 'A custom Moodle JS Handler (slasharguments on)' => [ + 'A custom Moodle JS Handler' => [ 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' + $wwwroot . '/mod/data/js.php?d=1234567890', ], ]; } @@ -218,25 +177,18 @@ final class outputrequirementslib_test extends \advanced_testcase { * 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): void { - global $CFG; - $defaultslashargs = $CFG->slasharguments; - - $CFG->slasharguments = $cfgslashargs; + public function test_js_fix_url_plain_string(string $url, string $expected): void { $rc = new \ReflectionClass(\page_requirements_manager::class); $rcm = $rc->getMethod('js_fix_url'); $requires = new \page_requirements_manager(); $actualmoodleurl = $rcm->invokeArgs($requires, [$url]); $this->assertEquals($expected, $actualmoodleurl->out(false)); - - $CFG->slasharguments = $defaultslashargs; } /** @@ -250,47 +202,34 @@ final class outputrequirementslib_test extends \advanced_testcase { $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' + $wwwroot . $admin . 'environment.xml', ], 'Data JS' => [ '/mod/data/data.js', - 1, - $wwwroot . '/lib/javascript.php/1/mod/data/data.js' + $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' + $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' + $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' + $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' + $wwwroot . '/mod/data/css.php?d=1234567890', ], - 'A custom Moodle JS Handler (slasharguments on)' => [ + 'A custom Moodle JS Handler' => [ '/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' + $wwwroot . '/mod/data/js.php?d=1234567890', ], ]; } diff --git a/lib/tests/url_test.php b/lib/tests/url_test.php index 5b4cd2a6e31..d96dc1dd946 100644 --- a/lib/tests/url_test.php +++ b/lib/tests/url_test.php @@ -68,7 +68,6 @@ final class url_test extends \advanced_testcase { $url = new url('http://www.example.org/pluginfile.php/slash/arguments'); $this->assertSame('/pluginfile.php/slash/arguments', $url->get_path()); - $this->assertSame('/pluginfile.php', $url->get_path(false)); } public function test_round_trip(): void { @@ -304,16 +303,12 @@ final class url_test extends \advanced_testcase { * Test the make_pluginfile_url function. * * @dataProvider make_pluginfile_url_provider - * @param bool $slashargs * @param array $args Args to be provided to make_pluginfile_url * @param string $expected The expected result */ - public function test_make_pluginfile_url($slashargs, $args, $expected): void { - global $CFG; - + public function test_make_pluginfile_url(array $args, string $expected): void { $this->resetAfterTest(); - $CFG->slasharguments = $slashargs; $url = call_user_func_array([url::class, 'make_pluginfile_url'], $args); $this->assertMatchesRegularExpression($expected, $url->out(true)); } @@ -328,7 +323,6 @@ final class url_test extends \advanced_testcase { $tokenbaseurl = "https://www.example.com/moodle/tokenpluginfile.php"; return [ 'Standard with slashargs' => [ - 'slashargs' => true, 'args' => [ 1, 'mod_forum', @@ -339,20 +333,7 @@ final class url_test extends \advanced_testcase { ], 'expected' => "@{$baseurl}/1/mod_forum/posts/422/my/location/file.png@", ], - 'Standard without slashargs' => [ - 'slashargs' => false, - 'args' => [ - 1, - 'mod_forum', - 'posts', - 422, - '/my/location/', - 'file.png', - ], - 'expected' => "@{$baseurl}\?file=%2F1%2Fmod_forum%2Fposts%2F422%2Fmy%2Flocation%2Ffile.png@", - ], 'Token included with slashargs' => [ - 'slashargs' => true, 'args' => [ 1, 'mod_forum', @@ -365,21 +346,6 @@ final class url_test extends \advanced_testcase { ], 'expected' => "@{$tokenbaseurl}/[^/]*/1/mod_forum/posts/422/my/location/file.png@", ], - 'Token included without slashargs' => [ - 'slashargs' => false, - 'args' => [ - 1, - 'mod_forum', - 'posts', - 422, - '/my/location/', - 'file.png', - false, - true, - ], - 'expected' => - "@{$tokenbaseurl}\?file=%2F1%2Fmod_forum%2Fposts%2F422%2Fmy%2Flocation%2Ffile.png&token=[a-z0-9]*@", - ], ]; } diff --git a/lib/tests/weblib_test.php b/lib/tests/weblib_test.php index 1a5d4f84553..91d946a0cdd 100644 --- a/lib/tests/weblib_test.php +++ b/lib/tests/weblib_test.php @@ -824,125 +824,90 @@ EXPECTED; * Data provider for test_get_file_argument. */ public static function provider_get_file_argument() { - return array( + return [ // Serving SCORM content w/o HTTP GET params. - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html', 'SCRIPT_NAME' => '/pluginfile.php', 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 0, '/3854/mod_scorm/content/1/swf.html'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html', - 'SCRIPT_NAME' => '/pluginfile.php', - 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 1, '/3854/mod_scorm/content/1/swf.html'), + ], + '/3854/mod_scorm/content/1/swf.html', + ], // Serving SCORM content w/ HTTP GET 'file' as first param. - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html?file=video_.swf', 'SCRIPT_NAME' => '/pluginfile.php', 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 0, '/3854/mod_scorm/content/1/swf.html'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html?file=video_.swf', - 'SCRIPT_NAME' => '/pluginfile.php', - 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 1, '/3854/mod_scorm/content/1/swf.html'), + ], + '/3854/mod_scorm/content/1/swf.html', + ], // Serving SCORM content w/ HTTP GET 'file' not as first param. - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html?foo=bar&file=video_.swf', 'SCRIPT_NAME' => '/pluginfile.php', 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 0, '/3854/mod_scorm/content/1/swf.html'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php/3854/mod_scorm/content/1/swf.html?foo=bar&file=video_.swf', - 'SCRIPT_NAME' => '/pluginfile.php', - 'PATH_INFO' => '/3854/mod_scorm/content/1/swf.html', - ), 1, '/3854/mod_scorm/content/1/swf.html'), + ], + '/3854/mod_scorm/content/1/swf.html', + ], // Serving content from a generic activity w/ HTTP GET 'file', still forcing slash arguments. - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php/3854/whatever/content/1/swf.html?file=video_.swf', 'SCRIPT_NAME' => '/pluginfile.php', 'PATH_INFO' => '/3854/whatever/content/1/swf.html', - ), 0, '/3854/whatever/content/1/swf.html'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php/3854/whatever/content/1/swf.html?file=video_.swf', - 'SCRIPT_NAME' => '/pluginfile.php', - 'PATH_INFO' => '/3854/whatever/content/1/swf.html', - ), 1, '/3854/whatever/content/1/swf.html'), + ], + '/3854/whatever/content/1/swf.html', + ], // Serving content from a generic activity w/ HTTP GET 'file', still forcing slash arguments (edge case). - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php/?file=video_.swf', 'SCRIPT_NAME' => '/pluginfile.php', 'PATH_INFO' => '/', - ), 0, 'video_.swf'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php/?file=video_.swf', - 'SCRIPT_NAME' => '/pluginfile.php', - 'PATH_INFO' => '/', - ), 1, 'video_.swf'), + ], + 'video_.swf', + ], // Serving content from a generic activity w/ HTTP GET 'file', w/o forcing slash arguments. - array(array( + [ + [ 'SERVER_SOFTWARE' => 'Apache', 'SERVER_PORT' => '80', 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/pluginfile.php?file=%2F3854%2Fwhatever%2Fcontent%2F1%2Fswf.html%3Ffile%3Dvideo_.swf', 'SCRIPT_NAME' => '/pluginfile.php', - ), 0, '/3854/whatever/content/1/swf.html?file=video_.swf'), - array(array( - 'SERVER_SOFTWARE' => 'Apache', - 'SERVER_PORT' => '80', - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => '/pluginfile.php?file=%2F3854%2Fwhatever%2Fcontent%2F1%2Fswf.html%3Ffile%3Dvideo_.swf', - 'SCRIPT_NAME' => '/pluginfile.php', - ), 1, '/3854/whatever/content/1/swf.html?file=video_.swf'), - ); + ], + '/3854/whatever/content/1/swf.html?file=video_.swf', + ], + ]; } /** * Tests for get_file_argument() function. * * @param array $server mockup for $_SERVER. - * @param string $cfgslasharguments slasharguments setting. - * @param string|false $expected Expected value. + * @param string $expected Expected value. * @dataProvider provider_get_file_argument * @covers ::get_file_argument */ - public function test_get_file_argument($server, $cfgslasharguments, $expected): void { - global $CFG; - - // Overwrite the related settings. - $currentsetting = $CFG->slasharguments; - $CFG->slasharguments = $cfgslasharguments; + public function test_get_file_argument(array $server, string $expected): void { // Mock global $_SERVER. $currentserver = isset($_SERVER) ? $_SERVER : null; $_SERVER = $server; @@ -965,7 +930,6 @@ EXPECTED; $this->assertEquals($expected, get_file_argument()); // Restore the current settings and global values. - $CFG->slasharguments = $currentsetting; if (is_null($currentserver)) { unset($_SERVER); } else {