From aff24313bedf1920df583afb8cc36fa3929f9107 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sat, 24 Sep 2011 15:07:27 +0200 Subject: [PATCH 1/4] MDL-29401 introduce new frankenstyle PARAM types New PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN + fixing of hopefully all current incorrect parameter types. This should help with diagnosing of incorrectly named 3rd party plugins too. --- admin/auth.php | 2 +- admin/auth_config.php | 2 +- admin/editors.php | 2 +- admin/enrol.php | 2 +- admin/localplugins.php | 2 +- admin/modules.php | 6 +- admin/qbehaviours.php | 10 +-- admin/qtypes.php | 10 +-- admin/report/customlang/locallib.php | 2 +- admin/report/questioninstances/index.php | 2 +- admin/repositoryinstance.php | 2 +- admin/tool/health/index.php | 2 +- backup/backupfilesedit.php | 4 +- backup/restorefile.php | 4 +- comment/comment_ajax.php | 4 +- comment/comment_post.php | 4 +- comment/lib.php | 2 +- course/externallib.php | 12 +-- course/modedit.php | 4 +- course/moodleform_mod.php | 2 +- course/report/log/index.php | 2 +- course/search.php | 2 +- files/externallib.php | 16 ++-- files/filebrowser_ajax.php | 4 +- filter/local_settings_form.php | 2 +- grade/lib.php | 4 +- help.php | 2 +- lib/adminlib.php | 4 +- lib/blocklib.php | 2 +- lib/enrollib.php | 6 +- lib/filestorage/file_storage.php | 24 ++++-- lib/moodlelib.php | 105 +++++++++++++++++++---- lib/portfolio/forms.php | 2 +- lib/setup.php | 2 +- lib/simpletest/testmoodlelib.php | 53 ++++++++++++ lib/upgradelib.php | 30 +++---- mod/assignment/lib.php | 4 +- mod/lesson/locallib.php | 2 +- mod/workshop/form/assessment_form.php | 2 +- mod/workshop/form/edit_form.php | 2 +- pluginfile.php | 4 +- rating/index.php | 4 +- rating/rate.php | 4 +- rating/rate_ajax.php | 4 +- repository/coursefiles/lib.php | 4 +- repository/lib.php | 12 +-- repository/local/lib.php | 4 +- repository/recent/lib.php | 4 +- theme/index.php | 4 +- user/externallib.php | 12 +-- 50 files changed, 264 insertions(+), 142 deletions(-) diff --git a/admin/auth.php b/admin/auth.php index 8345aadfdee..93711ee8324 100644 --- a/admin/auth.php +++ b/admin/auth.php @@ -19,7 +19,7 @@ $returnurl = new moodle_url('/admin/settings.php', array('section'=>'manageauths $PAGE->set_url($returnurl); $action = optional_param('action', '', PARAM_ACTION); -$auth = optional_param('auth', '', PARAM_SAFEDIR); +$auth = optional_param('auth', '', PARAM_PLUGIN); get_enabled_auth_plugins(true); // fix the list of enabled auths if (empty($CFG->auth)) { diff --git a/admin/auth_config.php b/admin/auth_config.php index 71dafdfc57a..c70271124fd 100644 --- a/admin/auth_config.php +++ b/admin/auth_config.php @@ -6,7 +6,7 @@ require_once '../config.php'; require_once $CFG->libdir.'/adminlib.php'; -$auth = required_param('auth', PARAM_SAFEDIR); +$auth = required_param('auth', PARAM_PLUGIN); $PAGE->set_pagetype('admin-auth-' . $auth); admin_externalpage_setup('authsetting'.$auth); diff --git a/admin/editors.php b/admin/editors.php index 476df6cd5f8..fbb7be5aac4 100644 --- a/admin/editors.php +++ b/admin/editors.php @@ -14,7 +14,7 @@ require_capability('moodle/site:config', get_context_instance(CONTEXT_SYSTEM)); $returnurl = "$CFG->wwwroot/$CFG->admin/settings.php?section=manageeditors"; $action = optional_param('action', '', PARAM_ACTION); -$editor = optional_param('editor', '', PARAM_SAFEDIR); +$editor = optional_param('editor', '', PARAM_PLUGIN); // get currently installed and enabled auth plugins $available_editors = editors_get_available(); diff --git a/admin/enrol.php b/admin/enrol.php index 26e430ef8f7..efb4dc36113 100644 --- a/admin/enrol.php +++ b/admin/enrol.php @@ -27,7 +27,7 @@ require_once('../config.php'); require_once($CFG->libdir.'/adminlib.php'); $action = required_param('action', PARAM_ACTION); -$enrol = required_param('enrol', PARAM_SAFEDIR); +$enrol = required_param('enrol', PARAM_PLUGIN); $confirm = optional_param('confirm', 0, PARAM_BOOL); $PAGE->set_url('/admin/enrol.php'); diff --git a/admin/localplugins.php b/admin/localplugins.php index 674138b438c..9146c822aed 100644 --- a/admin/localplugins.php +++ b/admin/localplugins.php @@ -33,7 +33,7 @@ require_once($CFG->libdir.'/tablelib.php'); admin_externalpage_setup('managelocalplugins'); -$delete = optional_param('delete', '', PARAM_SAFEDIR); +$delete = optional_param('delete', '', PARAM_PLUGIN); $confirm = optional_param('confirm', '', PARAM_BOOL); /// If data submitted, then process and store. diff --git a/admin/modules.php b/admin/modules.php index 71e22bdb488..45f7a57bff1 100644 --- a/admin/modules.php +++ b/admin/modules.php @@ -11,9 +11,9 @@ admin_externalpage_setup('managemodules'); - $show = optional_param('show', '', PARAM_SAFEDIR); - $hide = optional_param('hide', '', PARAM_SAFEDIR); - $delete = optional_param('delete', '', PARAM_SAFEDIR); + $show = optional_param('show', '', PARAM_PLUGIN); + $hide = optional_param('hide', '', PARAM_PLUGIN); + $delete = optional_param('delete', '', PARAM_PLUGIN); $confirm = optional_param('confirm', '', PARAM_BOOL); diff --git a/admin/qbehaviours.php b/admin/qbehaviours.php index 88b75fcf37c..db76b95f01d 100644 --- a/admin/qbehaviours.php +++ b/admin/qbehaviours.php @@ -85,7 +85,7 @@ if (!empty($config->disabledbehaviours)) { // Process actions ============================================================ // Disable. -if (($disable = optional_param('disable', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($disable = optional_param('disable', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($behaviours[$disable])) { print_error('unknownbehaviour', 'question', $thispageurl, $disable); } @@ -98,7 +98,7 @@ if (($disable = optional_param('disable', '', PARAM_SAFEDIR)) && confirm_sesskey } // Enable. -if (($enable = optional_param('enable', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($enable = optional_param('enable', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($behaviours[$enable])) { print_error('unknownbehaviour', 'question', $thispageurl, $enable); } @@ -115,7 +115,7 @@ if (($enable = optional_param('enable', '', PARAM_SAFEDIR)) && confirm_sesskey() } // Move up in order. -if (($up = optional_param('up', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($up = optional_param('up', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($behaviours[$up])) { print_error('unknownbehaviour', 'question', $thispageurl, $up); } @@ -127,7 +127,7 @@ if (($up = optional_param('up', '', PARAM_SAFEDIR)) && confirm_sesskey()) { } // Move down in order. -if (($down = optional_param('down', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($down = optional_param('down', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($behaviours[$down])) { print_error('unknownbehaviour', 'question', $thispageurl, $down); } @@ -139,7 +139,7 @@ if (($down = optional_param('down', '', PARAM_SAFEDIR)) && confirm_sesskey()) { } // Delete. -if (($delete = optional_param('delete', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($delete = optional_param('delete', '', PARAM_PLUGIN)) && confirm_sesskey()) { // Check it is OK to delete this question type. if ($delete == 'missing') { print_error('cannotdeletemissingbehaviour', 'question', $thispageurl); diff --git a/admin/qtypes.php b/admin/qtypes.php index a95de209099..05e7c754a9c 100644 --- a/admin/qtypes.php +++ b/admin/qtypes.php @@ -79,7 +79,7 @@ $sortedqtypes = question_bank::sort_qtype_array($sortedqtypes, $config); // Process actions ============================================================ // Disable. -if (($disable = optional_param('disable', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($disable = optional_param('disable', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($qtypes[$disable])) { print_error('unknownquestiontype', 'question', $thispageurl, $disable); } @@ -89,7 +89,7 @@ if (($disable = optional_param('disable', '', PARAM_SAFEDIR)) && confirm_sesskey } // Enable. -if (($enable = optional_param('enable', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($enable = optional_param('enable', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($qtypes[$enable])) { print_error('unknownquestiontype', 'question', $thispageurl, $enable); } @@ -103,7 +103,7 @@ if (($enable = optional_param('enable', '', PARAM_SAFEDIR)) && confirm_sesskey() } // Move up in order. -if (($up = optional_param('up', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($up = optional_param('up', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($qtypes[$up])) { print_error('unknownquestiontype', 'question', $thispageurl, $up); } @@ -114,7 +114,7 @@ if (($up = optional_param('up', '', PARAM_SAFEDIR)) && confirm_sesskey()) { } // Move down in order. -if (($down = optional_param('down', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($down = optional_param('down', '', PARAM_PLUGIN)) && confirm_sesskey()) { if (!isset($qtypes[$down])) { print_error('unknownquestiontype', 'question', $thispageurl, $down); } @@ -125,7 +125,7 @@ if (($down = optional_param('down', '', PARAM_SAFEDIR)) && confirm_sesskey()) { } // Delete. -if (($delete = optional_param('delete', '', PARAM_SAFEDIR)) && confirm_sesskey()) { +if (($delete = optional_param('delete', '', PARAM_PLUGIN)) && confirm_sesskey()) { // Check it is OK to delete this question type. if ($delete == 'missingtype') { print_error('cannotdeletemissingqtype', 'question', $thispageurl); diff --git a/admin/report/customlang/locallib.php b/admin/report/customlang/locallib.php index c3aa6ca0d53..180518f501d 100644 --- a/admin/report/customlang/locallib.php +++ b/admin/report/customlang/locallib.php @@ -262,7 +262,7 @@ class report_customlang_utils { debugging('Unable to dump local strings for non-installed language pack .'.s($lang)); return false; } - if ($component !== clean_param($component, PARAM_SAFEDIR)) { + if ($component !== clean_param($component, PARAM_COMPONENT)) { throw new coding_exception('Incorrect component name'); } if (!$filename = self::get_component_filename($component)) { diff --git a/admin/report/questioninstances/index.php b/admin/report/questioninstances/index.php index 24299d32a5d..2add057b6df 100644 --- a/admin/report/questioninstances/index.php +++ b/admin/report/questioninstances/index.php @@ -12,7 +12,7 @@ require_once($CFG->libdir.'/adminlib.php'); require_once($CFG->libdir.'/questionlib.php'); // Get URL parameters. -$requestedqtype = optional_param('qtype', '', PARAM_SAFEDIR); +$requestedqtype = optional_param('qtype', '', PARAM_PLUGIN); // Print the header & check permissions. admin_externalpage_setup('reportquestioninstances'); diff --git a/admin/repositoryinstance.php b/admin/repositoryinstance.php index 7ba94dfcfdb..e75193d0be4 100644 --- a/admin/repositoryinstance.php +++ b/admin/repositoryinstance.php @@ -10,7 +10,7 @@ $new = optional_param('new', '', PARAM_FORMAT); $hide = optional_param('hide', 0, PARAM_INT); $delete = optional_param('delete', 0, PARAM_INT); $sure = optional_param('sure', '', PARAM_ALPHA); -$type = optional_param('type', '', PARAM_ALPHAEXT); +$type = optional_param('type', '', PARAM_PLUGIN); $context = get_context_instance(CONTEXT_SYSTEM); diff --git a/admin/tool/health/index.php b/admin/tool/health/index.php index d7596ae80c8..bb63e3c9f1f 100644 --- a/admin/tool/health/index.php +++ b/admin/tool/health/index.php @@ -42,7 +42,7 @@ define('SEVERITY_SIGNIFICANT', 'significant'); define('SEVERITY_CRITICAL', 'critical'); - $solution = optional_param('solution', 0, PARAM_SAFEDIR); //in fact it is class name alhanumeric and _ + $solution = optional_param('solution', 0, PARAM_PLUGIN); require_login(); require_capability('moodle/site:config', get_context_instance(CONTEXT_SYSTEM)); diff --git a/backup/backupfilesedit.php b/backup/backupfilesedit.php index 41d0320f14f..0059bbc0f3b 100644 --- a/backup/backupfilesedit.php +++ b/backup/backupfilesedit.php @@ -31,8 +31,8 @@ require_once($CFG->dirroot . '/repository/lib.php'); $contextid = required_param('contextid', PARAM_INT); $currentcontext = required_param('currentcontext', PARAM_INT); // file parameters -$component = optional_param('component', null, PARAM_ALPHAEXT); -$filearea = optional_param('filearea', null, PARAM_ALPHAEXT); +$component = optional_param('component', null, PARAM_COMPONENT); +$filearea = optional_param('filearea', null, PARAM_AREA); $returnurl = optional_param('returnurl', null, PARAM_URL); list($context, $course, $cm) = get_context_info_array($currentcontext); diff --git a/backup/restorefile.php b/backup/restorefile.php index d89143de631..92f8eb0ab3d 100644 --- a/backup/restorefile.php +++ b/backup/restorefile.php @@ -33,8 +33,8 @@ $filecontextid = optional_param('filecontextid', 0, PARAM_INT); $action = optional_param('action', '', PARAM_ALPHA); // file parameters // non js interface may require these parameters -$component = optional_param('component', null, PARAM_ALPHAEXT); -$filearea = optional_param('filearea', null, PARAM_ALPHAEXT); +$component = optional_param('component', null, PARAM_COMPONENT); +$filearea = optional_param('filearea', null, PARAM_AREA); $itemid = optional_param('itemid', null, PARAM_INT); $filepath = optional_param('filepath', null, PARAM_PATH); $filename = optional_param('filename', null, PARAM_FILE); diff --git a/comment/comment_ajax.php b/comment/comment_ajax.php index 50d78056b65..828d5c8a2c3 100644 --- a/comment/comment_ajax.php +++ b/comment/comment_ajax.php @@ -48,12 +48,12 @@ if (!confirm_sesskey()) { } $client_id = required_param('client_id', PARAM_ALPHANUM); -$area = optional_param('area', '', PARAM_ALPHAEXT); +$area = optional_param('area', '', PARAM_AREA); $commentid = optional_param('commentid', -1, PARAM_INT); $content = optional_param('content', '', PARAM_RAW); $itemid = optional_param('itemid', '', PARAM_INT); $page = optional_param('page', 0, PARAM_INT); -$component = optional_param('component', '', PARAM_ALPHAEXT); +$component = optional_param('component', '', PARAM_COMPONENT); // initilising comment object $args = new stdClass; diff --git a/comment/comment_post.php b/comment/comment_post.php index 56a0e28699c..4e852b43900 100644 --- a/comment/comment_post.php +++ b/comment/comment_post.php @@ -31,11 +31,11 @@ require_login($course, true, $cm); require_sesskey(); $action = optional_param('action', '', PARAM_ALPHA); -$area = optional_param('area', '', PARAM_ALPHAEXT); +$area = optional_param('area', '', PARAM_AREA); $content = optional_param('content', '', PARAM_RAW); $itemid = optional_param('itemid', '', PARAM_INT); $returnurl = optional_param('returnurl', '/', PARAM_URL); -$component = optional_param('component', '', PARAM_ALPHAEXT); +$component = optional_param('component', '', PARAM_COMPONENT); // Currently this script can only add comments if ($action !== 'add') { diff --git a/comment/lib.php b/comment/lib.php index 2d90ba72058..99e92f990ed 100644 --- a/comment/lib.php +++ b/comment/lib.php @@ -285,7 +285,7 @@ class comment { self::$comment_itemid = optional_param('comment_itemid', '', PARAM_INT); self::$comment_context = optional_param('comment_context', '', PARAM_INT); self::$comment_page = optional_param('comment_page', '', PARAM_INT); - self::$comment_area = optional_param('comment_area', '', PARAM_ALPHAEXT); + self::$comment_area = optional_param('comment_area', '', PARAM_AREA); $page->requires->string_for_js('addcomment', 'moodle'); $page->requires->string_for_js('deletecomment', 'moodle'); diff --git a/course/externallib.php b/course/externallib.php index 67037cc01d4..7d9e6d62933 100644 --- a/course/externallib.php +++ b/course/externallib.php @@ -146,7 +146,7 @@ class moodle_course_external extends external_api { 'summary' => new external_value(PARAM_RAW, 'summary'), 'summaryformat' => new external_value(PARAM_INT, 'the summary text Moodle format'), - 'format' => new external_value(PARAM_ALPHANUMEXT, + 'format' => new external_value(PARAM_PLUGIN, 'course format: weeks, topics, social, site,..'), 'showgrades' => new external_value(PARAM_INT, '1 if grades are shown, otherwise 0', VALUE_OPTIONAL), @@ -185,9 +185,9 @@ class moodle_course_external extends external_api { VALUE_OPTIONAL), 'completionnotify' => new external_value(PARAM_INT, '1: yes 0: no', VALUE_OPTIONAL), - 'lang' => new external_value(PARAM_ALPHANUMEXT, + 'lang' => new external_value(PARAM_SAFEDIR, 'forced course language', VALUE_OPTIONAL), - 'forcetheme' => new external_value(PARAM_ALPHANUMEXT, + 'forcetheme' => new external_value(PARAM_PLUGIN, 'name of the force theme', VALUE_OPTIONAL), ), 'course' ) @@ -212,7 +212,7 @@ class moodle_course_external extends external_api { 'summary' => new external_value(PARAM_RAW, 'summary', VALUE_OPTIONAL), 'summaryformat' => new external_value(PARAM_INT, 'the summary text Moodle format', VALUE_DEFAULT, FORMAT_MOODLE), - 'format' => new external_value(PARAM_ALPHANUMEXT, + 'format' => new external_value(PARAM_PLUGIN, 'course format: weeks, topics, social, site,..', VALUE_DEFAULT, $courseconfig->format), 'showgrades' => new external_value(PARAM_INT, @@ -252,9 +252,9 @@ class moodle_course_external extends external_api { VALUE_OPTIONAL), 'completionnotify' => new external_value(PARAM_INT, '1: yes 0: no', VALUE_OPTIONAL), - 'lang' => new external_value(PARAM_ALPHANUMEXT, + 'lang' => new external_value(PARAM_SAFEDIR, 'forced course language', VALUE_OPTIONAL), - 'forcetheme' => new external_value(PARAM_ALPHANUMEXT, + 'forcetheme' => new external_value(PARAM_PLUGIN, 'name of the force theme', VALUE_OPTIONAL), ) ), 'courses to create' diff --git a/course/modedit.php b/course/modedit.php index 0650de3e11b..a68ddcca529 100644 --- a/course/modedit.php +++ b/course/modedit.php @@ -248,7 +248,7 @@ if ($mform->is_cancelled()) { } $fromform->course = $course->id; - $fromform->modulename = clean_param($fromform->modulename, PARAM_SAFEDIR); // For safety + $fromform->modulename = clean_param($fromform->modulename, PARAM_PLUGIN); // For safety $addinstancefunction = $fromform->modulename."_add_instance"; $updateinstancefunction = $fromform->modulename."_update_instance"; @@ -342,7 +342,7 @@ if ($mform->is_cancelled()) { set_coursemodule_idnumber($fromform->coursemodule, $fromform->cmidnumber); } - // Now that module is fully updated, also update completion data if + // Now that module is fully updated, also update completion data if // required (this will wipe all user completion data and recalculate it) if ($completion->is_enabled() && !empty($fromform->completionunlocked)) { $completion->reset_all_state($cm); diff --git a/course/moodleform_mod.php b/course/moodleform_mod.php index 20988b433f7..ce5666bbba7 100644 --- a/course/moodleform_mod.php +++ b/course/moodleform_mod.php @@ -623,7 +623,7 @@ abstract class moodleform_mod extends moodleform { $mform->setType('module', PARAM_INT); $mform->addElement('hidden', 'modulename', ''); - $mform->setType('modulename', PARAM_SAFEDIR); + $mform->setType('modulename', PARAM_PLUGIN); $mform->addElement('hidden', 'instance', 0); $mform->setType('instance', PARAM_INT); diff --git a/course/report/log/index.php b/course/report/log/index.php index df325b88beb..015d17d2db4 100644 --- a/course/report/log/index.php +++ b/course/report/log/index.php @@ -23,7 +23,7 @@ $group = optional_param('group', 0, PARAM_INT); // Group to display $user = optional_param('user', 0, PARAM_INT); // User to display $date = optional_param('date', 0, PARAM_FILE); // Date to display - number or some string - $modname = optional_param('modname', '', PARAM_SAFEDIR); // course_module->id + $modname = optional_param('modname', '', PARAM_PLUGIN); // course_module->id $modid = optional_param('modid', 0, PARAM_FILE); // number or 'site_errors' $modaction = optional_param('modaction', '', PARAM_PATH); // an action as recorded in the logs $page = optional_param('page', '0', PARAM_INT); // which page to show diff --git a/course/search.php b/course/search.php index faf3258563e..4433d0375bf 100644 --- a/course/search.php +++ b/course/search.php @@ -13,7 +13,7 @@ $hide = optional_param('hide', 0, PARAM_INT); $show = optional_param('show', 0, PARAM_INT); $blocklist = optional_param('blocklist', 0, PARAM_INT); - $modulelist= optional_param('modulelist', '', PARAM_ALPHAEXT); + $modulelist= optional_param('modulelist', '', PARAM_PLUGIN); $PAGE->set_url('/course/search.php', compact('search', 'page', 'perpage', 'blocklist', 'modulelist', 'edit')); $PAGE->set_context(get_context_instance(CONTEXT_SYSTEM)); diff --git a/files/externallib.php b/files/externallib.php index 85b90987777..d71cb72c33c 100644 --- a/files/externallib.php +++ b/files/externallib.php @@ -142,8 +142,8 @@ class moodle_file_external extends external_api { new external_single_structure( array( 'contextid' => new external_value(PARAM_INT, ''), - 'component' => new external_value(PARAM_ALPHAEXT, ''), - 'filearea' => new external_value(PARAM_ALPHAEXT, ''), + 'component' => new external_value(PARAM_COMPONENT, ''), + 'filearea' => new external_value(PARAM_AREA, ''), 'itemid' => new external_value(PARAM_INT, ''), 'filepath' => new external_value(PARAM_TEXT, ''), 'filename' => new external_value(PARAM_TEXT, ''), @@ -154,8 +154,8 @@ class moodle_file_external extends external_api { new external_single_structure( array( 'contextid' => new external_value(PARAM_INT, ''), - 'component' => new external_value(PARAM_ALPHAEXT, ''), - 'filearea' => new external_value(PARAM_ALPHAEXT, ''), + 'component' => new external_value(PARAM_COMPONENT, ''), + 'filearea' => new external_value(PARAM_AREA, ''), 'itemid' => new external_value(PARAM_INT, ''), 'filepath' => new external_value(PARAM_TEXT, ''), 'filename' => new external_value(PARAM_FILE, ''), @@ -176,8 +176,8 @@ class moodle_file_external extends external_api { return new external_function_parameters( array( 'contextid' => new external_value(PARAM_INT, 'context id'), - 'component' => new external_value(PARAM_ALPHAEXT, 'component'), - 'filearea' => new external_value(PARAM_ALPHAEXT, 'file area'), + 'component' => new external_value(PARAM_COMPONENT, 'component'), + 'filearea' => new external_value(PARAM_AREA, 'file area'), 'itemid' => new external_value(PARAM_INT, 'associated id'), 'filepath' => new external_value(PARAM_PATH, 'file path'), 'filename' => new external_value(PARAM_FILE, 'file name'), @@ -286,8 +286,8 @@ class moodle_file_external extends external_api { return new external_single_structure( array( 'contextid' => new external_value(PARAM_INT, ''), - 'component' => new external_value(PARAM_ALPHAEXT, ''), - 'filearea' => new external_value(PARAM_ALPHAEXT, ''), + 'component' => new external_value(PARAM_COMPONENT, ''), + 'filearea' => new external_value(PARAM_AREA, ''), 'itemid' => new external_value(PARAM_INT, ''), 'filepath' => new external_value(PARAM_TEXT, ''), 'filename' => new external_value(PARAM_FILE, ''), diff --git a/files/filebrowser_ajax.php b/files/filebrowser_ajax.php index 96099863274..2d651e95765 100644 --- a/files/filebrowser_ajax.php +++ b/files/filebrowser_ajax.php @@ -46,8 +46,8 @@ switch ($action) { // used by course file tree viewer case 'getfiletree': $contextid = required_param('contextid', PARAM_INT); - $component = required_param('component', PARAM_ALPHAEXT); - $filearea = required_param('filearea', PARAM_ALPHAEXT); + $component = required_param('component', PARAM_COMPONENT); + $filearea = required_param('filearea', PARAM_AREA); $itemid = required_param('itemid', PARAM_INT); $filepath = required_param('filepath', PARAM_PATH); diff --git a/filter/local_settings_form.php b/filter/local_settings_form.php index f1d59bcb282..cc616cc9a0e 100644 --- a/filter/local_settings_form.php +++ b/filter/local_settings_form.php @@ -53,7 +53,7 @@ abstract class filter_local_settings_form extends moodleform { $mform->setDefault('contextid', $this->context->id); $mform->addElement('hidden', 'filter'); - $mform->setType('filter', PARAM_ALPHAEXT); + $mform->setType('filter', PARAM_PLUGIN); $mform->setDefault('filter', $this->filter); $this->add_action_buttons(); diff --git a/grade/lib.php b/grade/lib.php index 392eed0d356..b558e0f3c7d 100644 --- a/grade/lib.php +++ b/grade/lib.php @@ -706,7 +706,7 @@ class grade_plugin_return { public function grade_plugin_return($params = null) { if (empty($params)) { $this->type = optional_param('gpr_type', null, PARAM_SAFEDIR); - $this->plugin = optional_param('gpr_plugin', null, PARAM_SAFEDIR); + $this->plugin = optional_param('gpr_plugin', null, PARAM_PLUGIN); $this->courseid = optional_param('gpr_courseid', null, PARAM_INT); $this->userid = optional_param('gpr_userid', null, PARAM_INT); $this->page = optional_param('gpr_page', null, PARAM_INT); @@ -838,7 +838,7 @@ class grade_plugin_return { if (!empty($this->plugin)) { $mform->addElement('hidden', 'gpr_plugin', $this->plugin); - $mform->setType('gpr_plugin', PARAM_SAFEDIR); + $mform->setType('gpr_plugin', PARAM_PLUGIN); } if (!empty($this->courseid)) { diff --git a/help.php b/help.php index 2659812376c..197c98427d3 100644 --- a/help.php +++ b/help.php @@ -31,7 +31,7 @@ define('NO_MOODLE_COOKIES', true); require_once(dirname(__FILE__) . '/config.php'); $identifier = required_param('identifier', PARAM_STRINGID); -$component = required_param('component', PARAM_SAFEDIR); +$component = required_param('component', PARAM_COMPONENT); $lang = required_param('lang', PARAM_LANG); // TODO: maybe split into separate scripts $ajax = optional_param('ajax', 0, PARAM_BOOL); diff --git a/lib/adminlib.php b/lib/adminlib.php index 3802199980a..d4adbe679c1 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -3506,9 +3506,9 @@ class admin_setting_emoticons extends admin_setting { $emoticon = new stdClass(); $emoticon->text = clean_param(trim($form['text'.$i]), PARAM_NOTAGS); $emoticon->imagename = clean_param(trim($form['imagename'.$i]), PARAM_PATH); - $emoticon->imagecomponent = clean_param(trim($form['imagecomponent'.$i]), PARAM_SAFEDIR); + $emoticon->imagecomponent = clean_param(trim($form['imagecomponent'.$i]), PARAM_COMPONENT); $emoticon->altidentifier = clean_param(trim($form['altidentifier'.$i]), PARAM_STRINGID); - $emoticon->altcomponent = clean_param(trim($form['altcomponent'.$i]), PARAM_SAFEDIR); + $emoticon->altcomponent = clean_param(trim($form['altcomponent'.$i]), PARAM_COMPONENT); if (strpos($emoticon->text, ':/') !== false or strpos($emoticon->text, '//') !== false) { // prevent from breaking http://url.addresses by accident diff --git a/lib/blocklib.php b/lib/blocklib.php index 5586b3cb2a8..d91fa512b5c 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -1078,7 +1078,7 @@ class block_manager { * @return boolean true if anything was done. False if not. */ public function process_url_add() { - $blocktype = optional_param('bui_addblock', null, PARAM_SAFEDIR); + $blocktype = optional_param('bui_addblock', null, PARAM_PLUGIN); if (!$blocktype) { return false; } diff --git a/lib/enrollib.php b/lib/enrollib.php index a2003ab5446..9ff51bebe93 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -117,8 +117,10 @@ function enrol_get_plugins($enabled) { function enrol_get_plugin($name) { global $CFG; - if ($name !== clean_param($name, PARAM_SAFEDIR)) { - // ignore malformed plugin names completely + $name = clean_param($name, PARAM_PLUGIN); + + if (empty($name)) { + // ignore malformed or missing plugin names completely return null; } diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index 4c97d64f2f2..06718b5f675 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -519,11 +519,13 @@ class file_storage { throw new file_exception('storedfileproblem', 'Invalid contextid'); } - if ($component === '' or $component !== clean_param($component, PARAM_ALPHAEXT)) { + $component = clean_param($component, PARAM_COMPONENT); + if (empty($component)) { throw new file_exception('storedfileproblem', 'Invalid component'); } - if ($filearea === '' or $filearea !== clean_param($filearea, PARAM_ALPHAEXT)) { + $filearea = clean_param($filearea, PARAM_AREA); + if (empty($filearea)) { throw new file_exception('storedfileproblem', 'Invalid filearea'); } @@ -620,13 +622,15 @@ class file_storage { } if ($key == 'component') { - if ($value === '' or $value !== clean_param($value, PARAM_ALPHAEXT)) { + $value = clean_param($value, PARAM_COMPONENT); + if (empty($value)) { throw new file_exception('storedfileproblem', 'Invalid component'); } } if ($key == 'filearea') { - if ($value === '' or $value !== clean_param($value, PARAM_ALPHAEXT)) { + $value = clean_param($value, PARAM_AREA); + if (empty($value)) { throw new file_exception('storedfileproblem', 'Invalid filearea'); } } @@ -755,11 +759,13 @@ class file_storage { throw new file_exception('storedfileproblem', 'Invalid contextid'); } - if ($file_record->component === '' or $file_record->component !== clean_param($file_record->component, PARAM_ALPHAEXT)) { + $file_record->component = clean_param($file_record->component, PARAM_COMPONENT); + if (empty($file_record->component)) { throw new file_exception('storedfileproblem', 'Invalid component'); } - if ($file_record->filearea === '' or $file_record->filearea !== clean_param($file_record->filearea, PARAM_ALPHAEXT)) { + $file_record->filearea = clean_param($file_record->filearea, PARAM_AREA); + if (empty($file_record->filearea)) { throw new file_exception('storedfileproblem', 'Invalid filearea'); } @@ -848,11 +854,13 @@ class file_storage { throw new file_exception('storedfileproblem', 'Invalid contextid'); } - if ($file_record->component === '' or $file_record->component !== clean_param($file_record->component, PARAM_ALPHAEXT)) { + $file_record->component = clean_param($file_record->component, PARAM_COMPONENT); + if (empty($file_record->component)) { throw new file_exception('storedfileproblem', 'Invalid component'); } - if ($file_record->filearea === '' or $file_record->filearea !== clean_param($file_record->filearea, PARAM_ALPHAEXT)) { + $file_record->filearea = clean_param($file_record->filearea, PARAM_ALPHAEXT); + if (empty($file_record->filearea)) { throw new file_exception('storedfileproblem', 'Invalid filearea'); } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index dd689289e7a..e2cde0b755d 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -284,6 +284,29 @@ define('PARAM_TIMEZONE', 'timezone'); */ define('PARAM_CLEANFILE', 'file'); +/** + * PARAM_COMPONENT is used for full component names (aka frankenstyle) such as 'mod_forum', 'core_rating', 'auth_ldap'. + * Short legacy subsystem names and module names are accepted too ex: 'forum', 'rating', 'user'. + * Only lowercase ascii letters, numbers and underscores are allowed, it has to start with a letter. + * NOTE: numbers and underscores are strongly discouraged in plugin names! + */ +define('PARAM_COMPONENT', 'component'); + +/** + * PARAM_AREA is a name of area used when addressing files, comments, ratings, etc. + * It is usually used together with context id and component. + * Only lowercase ascii letters, numbers and underscores are allowed, it has to start with a letter. + */ +define('PARAM_AREA', 'area'); + +/** + * PARAM_PLUGIN is used for plugin names such as 'forum', 'glossary', 'ldap', 'radius', 'paypal', 'completionstatus'. + * Only lowercase ascii letters, numbers and underscores are allowed, it has to start with a letter. + * NOTE: numbers and underscores are strongly discouraged in plugin names! Underscores are forbidden in module names. + */ +define('PARAM_PLUGIN', 'plugin'); + + /// Web Services /// /** @@ -821,6 +844,34 @@ function clean_param($param, $type) { // easy, just strip all tags, if we ever want to fix orphaned '&' we have to do that in format_string() return strip_tags($param); + case PARAM_COMPONENT: + // we do not want any guessing here, either the name is correct or not + // please note only normalised component names are accepted + if (!preg_match('/^[a-z]+(_[a-z][a-z0-9_]*)?[a-z0-9]$/', $param)) { + return ''; + } + if (strpos($param, '__') !== false) { + return ''; + } + if (strpos($param, 'mod_') === 0) { + // module names must not contain underscores because we need to differentiate them from invalid plugin types + if (substr_count($param, '_') != 1) { + return ''; + } + } + return $param; + + case PARAM_PLUGIN: + case PARAM_AREA: + // we do not want any guessing here, either the name is correct or not + if (!preg_match('/^[a-z][a-z0-9_]*[a-z0-9]$/', $param)) { + return ''; + } + if (strpos($param, '__') !== false) { + return ''; + } + return $param; + case PARAM_SAFEDIR: // Remove everything not a-zA-Z0-9_- return preg_replace('/[^a-zA-Z0-9_-]/i', '', $param); @@ -988,8 +1039,10 @@ function clean_param($param, $type) { } case PARAM_AUTH: - $param = clean_param($param, PARAM_SAFEDIR); - if (exists_auth_plugin($param)) { + $param = clean_param($param, PARAM_PLUGIN); + if (empty($param)) { + return ''; + } else if (exists_auth_plugin($param)) { return $param; } else { return ''; @@ -1004,8 +1057,10 @@ function clean_param($param, $type) { } case PARAM_THEME: - $param = clean_param($param, PARAM_SAFEDIR); - if (file_exists("$CFG->dirroot/theme/$param/config.php")) { + $param = clean_param($param, PARAM_PLUGIN); + if (empty($param)) { + return ''; + } else if (file_exists("$CFG->dirroot/theme/$param/config.php")) { return $param; } else if (!empty($CFG->themedir) and file_exists("$CFG->themedir/$param/config.php")) { return $param; @@ -7381,7 +7436,8 @@ function get_plugin_list($plugintype) { if (in_array($pluginname, $ignored)) { continue; } - if ($pluginname !== clean_param($pluginname, PARAM_SAFEDIR)) { + $pluginname = clean_param($pluginname, PARAM_PLUGIN); + if (empty($pluginname)) { // better ignore plugins with problematic names here continue; } @@ -7489,23 +7545,30 @@ function get_list_of_plugins($directory='mod', $exclude='', $basedir='') { * * @param string $type Plugin type e.g. 'mod' * @param string $name Plugin name - * @param string $feature Feature code (FEATURE_xx constant) + * @param string $feature Feature name * @param string $action Feature's action * @param string $options parameters of callback function, should be an array * @param mixed $default default value if callback function hasn't been defined * @return mixed */ function plugin_callback($type, $name, $feature, $action, $options = null, $default=null) { - global $CFG; + $component = clean_param($type . '_' . $name, PARAM_COMPONENT); + if (empty($component)) { + throw coding_exception('Invalid component used in plugin_callback():' . $type . '_' . $name); + } - $name = clean_param($name, PARAM_SAFEDIR); $function = $name.'_'.$feature.'_'.$action; - $file = get_component_directory($type . '_' . $name) . '/lib.php'; + + $dir = get_component_directory($component); + if (empty($dir)) { + throw coding_exception('Invalid component used in plugin_callback():' . $type . '_' . $name); + } // Load library and look for function - if (file_exists($file)) { - require_once($file); + if (file_exists($dir.'/lib.php')) { + require_once($dir.'/lib.php'); } + if (function_exists($function)) { // Function exists, so just return function result $ret = call_user_func_array($function, (array)$options); @@ -7531,24 +7594,30 @@ function plugin_callback($type, $name, $feature, $action, $options = null, $defa function plugin_supports($type, $name, $feature, $default = NULL) { global $CFG; - $name = clean_param($name, PARAM_SAFEDIR); //bit of extra security + if ($type === 'mod' and $name === 'NEWMODULE') { + //somebody forgot to rename the module template + return false; + } + + $component = clean_param($type . '_' . $name, PARAM_COMPONENT); + if (empty($component)) { + throw coding_exception('Invalid component used in plugin_supports():' . $type . '_' . $name); + } $function = null; if ($type === 'mod') { // we need this special case because we support subplugins in modules, // otherwise it would end up in infinite loop - if ($name === 'NEWMODULE') { - //somebody forgot to rename the module template - return false; - } if (file_exists("$CFG->dirroot/mod/$name/lib.php")) { include_once("$CFG->dirroot/mod/$name/lib.php"); - $function = $type.'_'.$name.'_supports'; + $function = $component.'_supports'; if (!function_exists($function)) { // legacy non-frankenstyle function name $function = $name.'_supports'; } + } else { + // invalid module } } else { @@ -7558,7 +7627,7 @@ function plugin_supports($type, $name, $feature, $default = NULL) { } if (file_exists("$path/lib.php")) { include_once("$path/lib.php"); - $function = $type.'_'.$name.'_supports'; + $function = $component.'_supports'; } } diff --git a/lib/portfolio/forms.php b/lib/portfolio/forms.php index c0ecaae1d3e..db7880c0825 100644 --- a/lib/portfolio/forms.php +++ b/lib/portfolio/forms.php @@ -151,7 +151,7 @@ final class portfolio_admin_form extends moodleform { $mform->addElement('hidden', 'visible', $this->visible); $mform->setType('visible', PARAM_INT); $mform->addElement('hidden', 'plugin', $this->plugin); - $mform->setType('plugin', PARAM_SAFEDIR); + $mform->setType('plugin', PARAM_PLUGIN); if (!$this->instance) { $insane = portfolio_instance_sanity_check($this->instance); diff --git a/lib/setup.php b/lib/setup.php index d2d3bfcffb9..316b50632e0 100644 --- a/lib/setup.php +++ b/lib/setup.php @@ -712,7 +712,7 @@ if (!empty($CFG->profilingenabled)) { // Process theme change in the URL. if (!empty($CFG->allowthemechangeonurl) and !empty($_GET['theme'])) { // we have to use _GET directly because we do not want this to interfere with _POST - $urlthemename = optional_param('theme', '', PARAM_SAFEDIR); + $urlthemename = optional_param('theme', '', PARAM_PLUGIN); try { $themeconfig = theme_config::load($urlthemename); // Makes sure the theme can be loaded without errors. diff --git a/lib/simpletest/testmoodlelib.php b/lib/simpletest/testmoodlelib.php index 34c547ac02c..d08515b947d 100644 --- a/lib/simpletest/testmoodlelib.php +++ b/lib/simpletest/testmoodlelib.php @@ -693,6 +693,59 @@ class moodlelib_test extends UnitTestCase { ',9789,42897'); } + function test_clean_param_component() { + // please note the cleaning of component names is very strict, no guessing here + $this->assertIdentical(clean_param('mod_forum', PARAM_COMPONENT), 'mod_forum'); + $this->assertIdentical(clean_param('block_online_users', PARAM_COMPONENT), 'block_online_users'); + $this->assertIdentical(clean_param('mod_something2', PARAM_COMPONENT), 'mod_something2'); + $this->assertIdentical(clean_param('forum', PARAM_COMPONENT), 'forum'); + $this->assertIdentical(clean_param('user', PARAM_COMPONENT), 'user'); + $this->assertIdentical(clean_param('rating', PARAM_COMPONENT), 'rating'); + $this->assertIdentical(clean_param('mod_2something', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('2mod_something', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('mod_something_xx', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('auth_something__xx', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('mod_Something', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('mod_somethíng', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('auth_xx-yy', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('_auth_xx', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('a2uth_xx', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('auth_xx_', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('auth_xx.old', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('_user', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('2rating', PARAM_COMPONENT), ''); + $this->assertIdentical(clean_param('user_', PARAM_COMPONENT), ''); + } + + function test_clean_param_plugin() { + // please note the cleaning of plugin names is very strict, no guessing here + $this->assertIdentical(clean_param('forum', PARAM_PLUGIN), 'forum'); + $this->assertIdentical(clean_param('forum2', PARAM_PLUGIN), 'forum2'); + $this->assertIdentical(clean_param('online_users', PARAM_PLUGIN), 'online_users'); + $this->assertIdentical(clean_param('online__users', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('forum ', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('forum.old', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('xx-yy', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('2xx', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('Xx', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('_xx', PARAM_PLUGIN), ''); + $this->assertIdentical(clean_param('xx_', PARAM_PLUGIN), ''); + } + + function test_clean_param_area() { + // please note the cleaning of area names is very strict, no guessing here + $this->assertIdentical(clean_param('something', PARAM_AREA), 'something'); + $this->assertIdentical(clean_param('something2', PARAM_AREA), 'something2'); + $this->assertIdentical(clean_param('some_thing', PARAM_AREA), 'some_thing'); + $this->assertIdentical(clean_param('_something', PARAM_AREA), ''); + $this->assertIdentical(clean_param('something_', PARAM_AREA), ''); + $this->assertIdentical(clean_param('2something', PARAM_AREA), ''); + $this->assertIdentical(clean_param('Something', PARAM_AREA), ''); + $this->assertIdentical(clean_param('some-thing', PARAM_AREA), ''); + $this->assertIdentical(clean_param('somethííng', PARAM_AREA), ''); + $this->assertIdentical(clean_param('something.x', PARAM_AREA), ''); + } + function test_clean_param_text() { $this->assertEqual(PARAM_TEXT, PARAM_MULTILANG); //standard diff --git a/lib/upgradelib.php b/lib/upgradelib.php index 06693c45d3d..8ea6abd93ba 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -276,14 +276,11 @@ function upgrade_plugins($type, $startcallback, $endcallback, $verbose) { $plugs = get_plugin_list($type); foreach ($plugs as $plug=>$fullplug) { - $component = $type.'_'.$plug; // standardised plugin name + $component = clean_param($type.'_'.$plug, PARAM_COMPONENT); // standardised plugin name // check plugin dir is valid name - $cplug = strtolower($plug); - $cplug = clean_param($cplug, PARAM_SAFEDIR); - $cplug = str_replace('-', '', $cplug); - if ($plug !== $cplug) { - throw new plugin_defective_exception($component, 'Invalid plugin directory name.'); + if (empty($component)) { + throw new plugin_defective_exception($type.'_'.$plug, 'Invalid plugin directory name.'); } if (!is_readable($fullplug.'/version.php')) { @@ -430,15 +427,11 @@ function upgrade_plugins_modules($startcallback, $endcallback, $verbose) { continue; } - $component = 'mod_'.$mod; + $component = clean_param('mod_'.$mod, PARAM_COMPONENT); // check module dir is valid name - $cmod = strtolower($mod); - $cmod = clean_param($cmod, PARAM_SAFEDIR); - $cmod = str_replace('-', '', $cmod); - $cmod = str_replace('_', '', $cmod); // modules MUST not have '_' in name and never will, sorry - if ($mod !== $cmod) { - throw new plugin_defective_exception($component, 'Invalid plugin directory name.'); + if (empty($component)) { + throw new plugin_defective_exception('mod_'.$mod, 'Invalid plugin directory name.'); } if (!is_readable($fullmod.'/version.php')) { @@ -593,18 +586,15 @@ function upgrade_plugins_blocks($startcallback, $endcallback, $verbose) { $first_install = ($DB->count_records('block_instances') == 0); } - if ($blockname == 'NEWBLOCK') { // Someone has unzipped the template, ignore it + if ($blockname === 'NEWBLOCK') { // Someone has unzipped the template, ignore it continue; } - $component = 'block_'.$blockname; + $component = clean_param('block_'.$blockname, PARAM_COMPONENT); // check block dir is valid name - $cblockname = strtolower($blockname); - $cblockname = clean_param($cblockname, PARAM_SAFEDIR); - $cblockname = str_replace('-', '', $cblockname); - if ($blockname !== $cblockname) { - throw new plugin_defective_exception($component, 'Invalid plugin directory name.'); + if (empty($component)) { + throw new plugin_defective_exception('block_'.$blockname, 'Invalid plugin directory name.'); } if (!is_readable($fullblock.'/version.php')) { diff --git a/mod/assignment/lib.php b/mod/assignment/lib.php index e1c3d73f3d8..d247b31561c 100644 --- a/mod/assignment/lib.php +++ b/mod/assignment/lib.php @@ -2473,7 +2473,7 @@ function assignment_delete_instance($id){ function assignment_update_instance($assignment){ global $CFG; - $assignment->assignmenttype = clean_param($assignment->assignmenttype, PARAM_SAFEDIR); + $assignment->assignmenttype = clean_param($assignment->assignmenttype, PARAM_PLUGIN); require_once("$CFG->dirroot/mod/assignment/type/$assignment->assignmenttype/assignment.class.php"); $assignmentclass = "assignment_$assignment->assignmenttype"; @@ -2490,7 +2490,7 @@ function assignment_update_instance($assignment){ function assignment_add_instance($assignment) { global $CFG; - $assignment->assignmenttype = clean_param($assignment->assignmenttype, PARAM_SAFEDIR); + $assignment->assignmenttype = clean_param($assignment->assignmenttype, PARAM_PLUGIN); require_once("$CFG->dirroot/mod/assignment/type/$assignment->assignmenttype/assignment.class.php"); $assignmentclass = "assignment_$assignment->assignmenttype"; diff --git a/mod/lesson/locallib.php b/mod/lesson/locallib.php index f26f1be3b0f..40023e6d003 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -723,7 +723,7 @@ abstract class lesson_add_page_form_base extends moodleform { if ($this->standard === true) { $mform->addElement('hidden', 'qtype'); - $mform->setType('qtype', PARAM_SAFEDIR); + $mform->setType('qtype', PARAM_PLUGIN); $mform->addElement('text', 'title', get_string('pagetitle', 'lesson'), array('size'=>70)); $mform->setType('title', PARAM_TEXT); diff --git a/mod/workshop/form/assessment_form.php b/mod/workshop/form/assessment_form.php index 6ec50d0036d..f6e5e032caf 100644 --- a/mod/workshop/form/assessment_form.php +++ b/mod/workshop/form/assessment_form.php @@ -63,7 +63,7 @@ class workshop_assessment_form extends moodleform { // add the data common for all subplugins $mform->addElement('hidden', 'strategy', $this->workshop->strategy); - $mform->setType('strategy', PARAM_SAFEDIR); + $mform->setType('strategy', PARAM_PLUGIN); if (!empty($this->options['editableweight']) and !$mform->isFrozen()) { $mform->addElement('header', 'assessmentsettings', get_string('assessmentweight', 'workshop')); diff --git a/mod/workshop/form/edit_form.php b/mod/workshop/form/edit_form.php index 12ecaa75955..e70056d7ea0 100644 --- a/mod/workshop/form/edit_form.php +++ b/mod/workshop/form/edit_form.php @@ -63,7 +63,7 @@ class workshop_edit_strategy_form extends moodleform { $mform->setType('workshopid', PARAM_INT); $mform->addElement('hidden', 'strategy', $this->workshop->strategy); // strategy name - $mform->setType('strategy', PARAM_SAFEDIR); + $mform->setType('strategy', PARAM_PLUGIN); $this->definition_inner($mform); diff --git a/pluginfile.php b/pluginfile.php index 635273f03fd..7636365d2b5 100644 --- a/pluginfile.php +++ b/pluginfile.php @@ -49,8 +49,8 @@ if (count($args) < 3) { // always at least context, component and filearea } $contextid = (int)array_shift($args); -$component = clean_param(array_shift($args), PARAM_SAFEDIR); -$filearea = clean_param(array_shift($args), PARAM_SAFEDIR); +$component = clean_param(array_shift($args), PARAM_COMPONENT); +$filearea = clean_param(array_shift($args), PARAM_AREA); list($context, $course, $cm) = get_context_info_array($contextid); diff --git a/rating/index.php b/rating/index.php index 38c2f5c5df9..2667947fb2d 100644 --- a/rating/index.php +++ b/rating/index.php @@ -28,8 +28,8 @@ require_once("../config.php"); require_once("lib.php"); $contextid = required_param('contextid', PARAM_INT); -$component = required_param('component', PARAM_ALPHAEXT); -$ratingarea = optional_param('ratingarea', null, PARAM_ALPHANUMEXT); +$component = required_param('component', PARAM_COMPONENT); +$ratingarea = optional_param('ratingarea', null, PARAM_AREA); $itemid = required_param('itemid', PARAM_INT); $scaleid = required_param('scaleid', PARAM_INT); $sort = optional_param('sort', '', PARAM_ALPHA); diff --git a/rating/rate.php b/rating/rate.php index ad6f66cc3b7..ab05b473306 100644 --- a/rating/rate.php +++ b/rating/rate.php @@ -30,8 +30,8 @@ require_once('../config.php'); require_once($CFG->dirroot.'/rating/lib.php'); $contextid = required_param('contextid', PARAM_INT); -$component = required_param('component', PARAM_ALPHAEXT); -$ratingarea = required_param('ratingarea', PARAM_ALPHANUMEXT); +$component = required_param('component', PARAM_COMPONENT); +$ratingarea = required_param('ratingarea', PARAM_AREA); $itemid = required_param('itemid', PARAM_INT); $scaleid = required_param('scaleid', PARAM_INT); $userrating = required_param('rating', PARAM_INT); diff --git a/rating/rate_ajax.php b/rating/rate_ajax.php index c3e5bf89287..98a81a82346 100644 --- a/rating/rate_ajax.php +++ b/rating/rate_ajax.php @@ -32,8 +32,8 @@ require_once('../config.php'); require_once($CFG->dirroot.'/rating/lib.php'); $contextid = required_param('contextid', PARAM_INT); -$component = required_param('component', PARAM_ALPHAEXT); -$ratingarea = required_param('ratingarea', PARAM_ALPHANUMEXT); +$component = required_param('component', PARAM_COMPONENT); +$ratingarea = required_param('ratingarea', PARAM_AREA); $itemid = required_param('itemid', PARAM_INT); $scaleid = required_param('scaleid', PARAM_INT); $userrating = required_param('rating', PARAM_INT); diff --git a/repository/coursefiles/lib.php b/repository/coursefiles/lib.php index 093e8060e11..8b0fa15773e 100644 --- a/repository/coursefiles/lib.php +++ b/repository/coursefiles/lib.php @@ -134,8 +134,8 @@ class repository_coursefiles extends repository { $fileitemid = clean_param($params['itemid'], PARAM_INT); $filename = clean_param($params['filename'], PARAM_FILE); $filepath = clean_param($params['filepath'], PARAM_PATH);; - $filearea = clean_param($params['filearea'], PARAM_ALPHAEXT); - $component = clean_param($params['component'], PARAM_ALPHAEXT); + $filearea = clean_param($params['filearea'], PARAM_AREA); + $component = clean_param($params['component'], PARAM_COMPONENT); $context = get_context_instance_by_id($contextid); $file_info = $browser->get_file_info($context, $component, $filearea, $fileitemid, $filepath, $filename); diff --git a/repository/lib.php b/repository/lib.php index 4418218e785..fbdcd7dea40 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -640,8 +640,8 @@ abstract class repository { $fileitemid = clean_param($params['itemid'], PARAM_INT); $filename = clean_param($params['filename'], PARAM_FILE); $filepath = clean_param($params['filepath'], PARAM_PATH);; - $filearea = clean_param($params['filearea'], PARAM_ALPHAEXT); - $component = clean_param($params['component'], PARAM_ALPHAEXT); + $filearea = clean_param($params['filearea'], PARAM_AREA); + $component = clean_param($params['component'], PARAM_COMPONENT); $context = get_context_instance_by_id($contextid); // the file needs to copied to draft area @@ -1369,8 +1369,8 @@ abstract class repository { $fileitemid = clean_param($params['itemid'], PARAM_INT); $filename = clean_param($params['filename'], PARAM_FILE); $filepath = clean_param($params['filepath'], PARAM_PATH); - $filearea = clean_param($params['filearea'], PARAM_SAFEDIR); - $component = clean_param($params['component'], PARAM_ALPHAEXT); + $filearea = clean_param($params['filearea'], PARAM_AREA); + $component = clean_param($params['component'], PARAM_COMPONENT); $context = get_context_instance_by_id($contextid); $file_info = $browser->get_file_info($context, $component, $filearea, $fileitemid, $filepath, $filename); if (!empty($file_info)) { @@ -1887,7 +1887,7 @@ final class repository_instance_form extends moodleform { $mform->addElement('hidden', 'new', $this->plugin); $mform->setType('new', PARAM_FORMAT); $mform->addElement('hidden', 'plugin', $this->plugin); - $mform->setType('plugin', PARAM_SAFEDIR); + $mform->setType('plugin', PARAM_PLUGIN); $mform->addElement('hidden', 'typeid', $this->typeid); $mform->setType('typeid', PARAM_INT); $mform->addElement('hidden', 'contextid', $this->contextid); @@ -2001,7 +2001,7 @@ final class repository_type_form extends moodleform { $mform->addElement('hidden', 'action', $this->action); $mform->setType('action', PARAM_TEXT); $mform->addElement('hidden', 'repos', $this->plugin); - $mform->setType('repos', PARAM_SAFEDIR); + $mform->setType('repos', PARAM_PLUGIN); // let the plugin add its specific fields $classname = 'repository_' . $this->plugin; diff --git a/repository/local/lib.php b/repository/local/lib.php index 4f364752005..14a1948ee07 100644 --- a/repository/local/lib.php +++ b/repository/local/lib.php @@ -52,8 +52,8 @@ class repository_local extends repository { if (!empty($encodedpath)) { $params = unserialize(base64_decode($encodedpath)); if (is_array($params)) { - $component = is_null($params['component']) ? NULL : clean_param($params['component'], PARAM_ALPHAEXT); - $filearea = is_null($params['filearea']) ? NULL : clean_param($params['filearea'], PARAM_ALPHAEXT); + $component = is_null($params['component']) ? NULL : clean_param($params['component'], PARAM_COMPONENT); + $filearea = is_null($params['filearea']) ? NULL : clean_param($params['filearea'], PARAM_AREA); $itemid = is_null($params['itemid']) ? NULL : clean_param($params['itemid'], PARAM_INT); $filepath = is_null($params['filepath']) ? NULL : clean_param($params['filepath'], PARAM_PATH);; $filename = is_null($params['filename']) ? NULL : clean_param($params['filename'], PARAM_FILE); diff --git a/repository/recent/lib.php b/repository/recent/lib.php index 758faceccc9..da4e61f0de9 100644 --- a/repository/recent/lib.php +++ b/repository/recent/lib.php @@ -170,8 +170,8 @@ class repository_recent extends repository { $fileitemid = clean_param($params['itemid'], PARAM_INT); $filename = clean_param($params['filename'], PARAM_FILE); $filepath = clean_param($params['filepath'], PARAM_PATH);; - $filearea = clean_param($params['filearea'], PARAM_ALPHAEXT); - $component = clean_param($params['component'], PARAM_ALPHAEXT); + $filearea = clean_param($params['filearea'], PARAM_AREA); + $component = clean_param($params['component'], PARAM_COMPONENT); // XXX: // When user try to pick a file from other filearea, normally file api will use file browse to diff --git a/theme/index.php b/theme/index.php index 2a40a92b9a3..60cd1e666ba 100644 --- a/theme/index.php +++ b/theme/index.php @@ -22,7 +22,7 @@ require_once(dirname(__FILE__) . '/../config.php'); require_once($CFG->libdir . '/adminlib.php'); -$choose = optional_param('choose', '', PARAM_SAFEDIR); +$choose = optional_param('choose', '', PARAM_PLUGIN); $reset = optional_param('reset', 0, PARAM_BOOL); $device = optional_param('device', '', PARAM_TEXT); @@ -43,7 +43,7 @@ if ($reset and confirm_sesskey()) { theme_reset_all_caches(); } else if ($choose && $device && confirm_sesskey()) { - + // Load the theme to make sure it is valid. $theme = theme_config::load($choose); // Get the config argument for the chosen device. diff --git a/user/externallib.php b/user/externallib.php index ac8cd74d9ae..039ea3fbb94 100644 --- a/user/externallib.php +++ b/user/externallib.php @@ -45,10 +45,10 @@ class moodle_user_external extends external_api { 'firstname' => new external_value(PARAM_NOTAGS, 'The first name(s) of the user'), 'lastname' => new external_value(PARAM_NOTAGS, 'The family name of the user'), 'email' => new external_value(PARAM_EMAIL, 'A valid and unique email address'), - 'auth' => new external_value(PARAM_SAFEDIR, 'Auth plugins include manual, ldap, imap, etc', VALUE_DEFAULT, 'manual', NULL_NOT_ALLOWED), + 'auth' => new external_value(PARAM_PLUGIN, 'Auth plugins include manual, ldap, imap, etc', VALUE_DEFAULT, 'manual', NULL_NOT_ALLOWED), 'idnumber' => new external_value(PARAM_RAW, 'An arbitrary ID code number perhaps from the institution', VALUE_DEFAULT, ''), 'lang' => new external_value(PARAM_SAFEDIR, 'Language code such as "en", must exist on server', VALUE_DEFAULT, $CFG->lang, NULL_NOT_ALLOWED), - 'theme' => new external_value(PARAM_SAFEDIR, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), + 'theme' => new external_value(PARAM_PLUGIN, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), 'timezone' => new external_value(PARAM_TIMEZONE, 'Timezone code such as Australia/Perth, or 99 for default', VALUE_OPTIONAL), 'mailformat' => new external_value(PARAM_INTEGER, 'Mail format code is 0 for plain text, 1 for HTML etc', VALUE_OPTIONAL), 'description' => new external_value(PARAM_TEXT, 'User profile description, no HTML', VALUE_OPTIONAL), @@ -252,10 +252,10 @@ class moodle_user_external extends external_api { 'firstname' => new external_value(PARAM_NOTAGS, 'The first name(s) of the user', VALUE_OPTIONAL, '',NULL_NOT_ALLOWED), 'lastname' => new external_value(PARAM_NOTAGS, 'The family name of the user', VALUE_OPTIONAL), 'email' => new external_value(PARAM_EMAIL, 'A valid and unique email address', VALUE_OPTIONAL, '',NULL_NOT_ALLOWED), - 'auth' => new external_value(PARAM_SAFEDIR, 'Auth plugins include manual, ldap, imap, etc', VALUE_OPTIONAL, '', NULL_NOT_ALLOWED), + 'auth' => new external_value(PARAM_PLUGIN, 'Auth plugins include manual, ldap, imap, etc', VALUE_OPTIONAL, '', NULL_NOT_ALLOWED), 'idnumber' => new external_value(PARAM_RAW, 'An arbitrary ID code number perhaps from the institution', VALUE_OPTIONAL), 'lang' => new external_value(PARAM_SAFEDIR, 'Language code such as "en", must exist on server', VALUE_OPTIONAL, '', NULL_NOT_ALLOWED), - 'theme' => new external_value(PARAM_SAFEDIR, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), + 'theme' => new external_value(PARAM_PLUGIN, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), 'timezone' => new external_value(PARAM_TIMEZONE, 'Timezone code such as Australia/Perth, or 99 for default', VALUE_OPTIONAL), 'mailformat' => new external_value(PARAM_INTEGER, 'Mail format code is 0 for plain text, 1 for HTML etc', VALUE_OPTIONAL), 'description' => new external_value(PARAM_TEXT, 'User profile description, no HTML', VALUE_OPTIONAL), @@ -423,11 +423,11 @@ class moodle_user_external extends external_api { 'interests' => new external_value(PARAM_TEXT, 'user interests (separated by commas)', VALUE_OPTIONAL), 'firstaccess' => new external_value(PARAM_INT, 'first access to the site (0 if never)', VALUE_OPTIONAL), 'lastaccess' => new external_value(PARAM_INT, 'last access to the site (0 if never)', VALUE_OPTIONAL), - 'auth' => new external_value(PARAM_SAFEDIR, 'Auth plugins include manual, ldap, imap, etc', VALUE_OPTIONAL), + 'auth' => new external_value(PARAM_PLUGIN, 'Auth plugins include manual, ldap, imap, etc', VALUE_OPTIONAL), 'confirmed' => new external_value(PARAM_NUMBER, 'Active user: 1 if confirmed, 0 otherwise', VALUE_OPTIONAL), 'idnumber' => new external_value(PARAM_RAW, 'An arbitrary ID code number perhaps from the institution', VALUE_OPTIONAL), 'lang' => new external_value(PARAM_SAFEDIR, 'Language code such as "en", must exist on server', VALUE_OPTIONAL), - 'theme' => new external_value(PARAM_SAFEDIR, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), + 'theme' => new external_value(PARAM_PLUGIN, 'Theme name such as "standard", must exist on server', VALUE_OPTIONAL), 'timezone' => new external_value(PARAM_TIMEZONE, 'Timezone code such as Australia/Perth, or 99 for default', VALUE_OPTIONAL), 'mailformat' => new external_value(PARAM_INTEGER, 'Mail format code is 0 for plain text, 1 for HTML etc', VALUE_OPTIONAL), 'description' => new external_value(PARAM_RAW, 'User profile description', VALUE_OPTIONAL), From 65e9eac0d12dfe60c1b9b2d10ffe5f03dfd8e8c9 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Mon, 26 Sep 2011 02:16:31 +0200 Subject: [PATCH 2/4] MDL-29401 fix forgotten area Thanks Eloy! --- lib/filestorage/file_storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index 06718b5f675..180bb0b32b7 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -859,7 +859,7 @@ class file_storage { throw new file_exception('storedfileproblem', 'Invalid component'); } - $file_record->filearea = clean_param($file_record->filearea, PARAM_ALPHAEXT); + $file_record->filearea = clean_param($file_record->filearea, PARAM_AREA); if (empty($file_record->filearea)) { throw new file_exception('storedfileproblem', 'Invalid filearea'); } From 5a2bd4866589e8deaa375177220db7d4fd0d07b7 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Mon, 26 Sep 2011 23:21:39 +0200 Subject: [PATCH 3/4] MDL-29401 fix previous regression The problem was introduced when / was removed from PARAM_ALPHAEXT. --- filter/local_settings_form.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filter/local_settings_form.php b/filter/local_settings_form.php index cc616cc9a0e..1721ffa74bc 100644 --- a/filter/local_settings_form.php +++ b/filter/local_settings_form.php @@ -53,7 +53,7 @@ abstract class filter_local_settings_form extends moodleform { $mform->setDefault('contextid', $this->context->id); $mform->addElement('hidden', 'filter'); - $mform->setType('filter', PARAM_PLUGIN); + $mform->setType('filter', PARAM_SAFEPATH); $mform->setDefault('filter', $this->filter); $this->add_action_buttons(); From a5bb3a70aad48a3bf2808dbe5b0c8c568f7109db Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Mon, 26 Sep 2011 23:26:09 +0200 Subject: [PATCH 4/4] MDL-29401 few more tests for Eloy --- lib/simpletest/testmoodlelib.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/simpletest/testmoodlelib.php b/lib/simpletest/testmoodlelib.php index d08515b947d..fed0767a061 100644 --- a/lib/simpletest/testmoodlelib.php +++ b/lib/simpletest/testmoodlelib.php @@ -697,6 +697,7 @@ class moodlelib_test extends UnitTestCase { // please note the cleaning of component names is very strict, no guessing here $this->assertIdentical(clean_param('mod_forum', PARAM_COMPONENT), 'mod_forum'); $this->assertIdentical(clean_param('block_online_users', PARAM_COMPONENT), 'block_online_users'); + $this->assertIdentical(clean_param('block_blond_online_users', PARAM_COMPONENT), 'block_blond_online_users'); $this->assertIdentical(clean_param('mod_something2', PARAM_COMPONENT), 'mod_something2'); $this->assertIdentical(clean_param('forum', PARAM_COMPONENT), 'forum'); $this->assertIdentical(clean_param('user', PARAM_COMPONENT), 'user'); @@ -722,6 +723,7 @@ class moodlelib_test extends UnitTestCase { $this->assertIdentical(clean_param('forum', PARAM_PLUGIN), 'forum'); $this->assertIdentical(clean_param('forum2', PARAM_PLUGIN), 'forum2'); $this->assertIdentical(clean_param('online_users', PARAM_PLUGIN), 'online_users'); + $this->assertIdentical(clean_param('blond_online_users', PARAM_PLUGIN), 'blond_online_users'); $this->assertIdentical(clean_param('online__users', PARAM_PLUGIN), ''); $this->assertIdentical(clean_param('forum ', PARAM_PLUGIN), ''); $this->assertIdentical(clean_param('forum.old', PARAM_PLUGIN), ''); @@ -737,6 +739,7 @@ class moodlelib_test extends UnitTestCase { $this->assertIdentical(clean_param('something', PARAM_AREA), 'something'); $this->assertIdentical(clean_param('something2', PARAM_AREA), 'something2'); $this->assertIdentical(clean_param('some_thing', PARAM_AREA), 'some_thing'); + $this->assertIdentical(clean_param('some_thing_xx', PARAM_AREA), 'some_thing_xx'); $this->assertIdentical(clean_param('_something', PARAM_AREA), ''); $this->assertIdentical(clean_param('something_', PARAM_AREA), ''); $this->assertIdentical(clean_param('2something', PARAM_AREA), '');