From 0a76cd2ca1c5216d67c9bf7c844020c3b59b1fd7 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 7 Mar 2017 12:26:27 +0800 Subject: [PATCH] MDL-57596 forms: CLEANHTML in persistent forms Add special handling for text fields with the CLEANHTML type. This should be used when students and teachers can edit the same field (you can't trust those students). Applies cleaning on submitted data, and on data stored in the DB before it is put back in an editing form. --- admin/tool/lp/classes/form/competency.php | 2 +- admin/tool/lp/classes/form/competency_framework.php | 2 +- admin/tool/lp/classes/form/persistent.php | 6 ++++++ admin/tool/lp/classes/form/plan.php | 2 +- admin/tool/lp/classes/form/template.php | 2 +- admin/tool/lp/classes/form/user_evidence.php | 2 +- competency/classes/competency.php | 2 +- competency/classes/competency_framework.php | 2 +- competency/classes/external/exporter.php | 5 +++-- competency/classes/persistent.php | 7 ++++++- competency/classes/plan.php | 2 +- competency/classes/template.php | 2 +- competency/classes/user_evidence.php | 2 +- 13 files changed, 25 insertions(+), 13 deletions(-) diff --git a/admin/tool/lp/classes/form/competency.php b/admin/tool/lp/classes/form/competency.php index 32136bc5155..6960d979eb2 100644 --- a/admin/tool/lp/classes/form/competency.php +++ b/admin/tool/lp/classes/form/competency.php @@ -99,7 +99,7 @@ class competency extends persistent { // Description. $mform->addElement('editor', 'description', get_string('description', 'tool_lp'), array('rows' => 4)); - $mform->setType('description', PARAM_RAW); + $mform->setType('description', PARAM_CLEANHTML); // ID number. $mform->addElement('text', 'idnumber', get_string('idnumber', 'tool_lp'), 'maxlength="100"'); $mform->setType('idnumber', PARAM_RAW); diff --git a/admin/tool/lp/classes/form/competency_framework.php b/admin/tool/lp/classes/form/competency_framework.php index 6d4b6bcd461..b678cbb8fa6 100644 --- a/admin/tool/lp/classes/form/competency_framework.php +++ b/admin/tool/lp/classes/form/competency_framework.php @@ -62,7 +62,7 @@ class competency_framework extends persistent { // Description. $mform->addElement('editor', 'description', get_string('description', 'tool_lp'), array('rows' => 4)); - $mform->setType('description', PARAM_RAW); + $mform->setType('description', PARAM_CLEANHTML); // ID number. $mform->addElement('text', 'idnumber', get_string('idnumber', 'tool_lp'), 'maxlength="100"'); $mform->setType('idnumber', PARAM_RAW); diff --git a/admin/tool/lp/classes/form/persistent.php b/admin/tool/lp/classes/form/persistent.php index 9b7dc85f4ff..b32d8640695 100644 --- a/admin/tool/lp/classes/form/persistent.php +++ b/admin/tool/lp/classes/form/persistent.php @@ -184,8 +184,14 @@ abstract class persistent extends moodleform { $data = $this->get_persistent()->to_record(); $class = static::$persistentclass; $properties = $class::get_formatted_properties(); + $allproperties = $class::properties_definition(); foreach ($data as $field => $value) { + // Clean data if it is to be displayed in a form. + if (isset($allproperties[$field]['type'])) { + $data->$field = clean_param($data->$field, $allproperties[$field]['type']); + } + // Convert formatted properties. if (isset($properties[$field])) { $data->$field = array( diff --git a/admin/tool/lp/classes/form/plan.php b/admin/tool/lp/classes/form/plan.php index 461f8c2e887..2cbc6330de6 100644 --- a/admin/tool/lp/classes/form/plan.php +++ b/admin/tool/lp/classes/form/plan.php @@ -59,7 +59,7 @@ class plan extends persistent { $mform->addRule('name', get_string('maximumchars', '', 100), 'maxlength', 100, 'client'); // Description. $mform->addElement('editor', 'description', get_string('plandescription', 'tool_lp'), array('rows' => 4)); - $mform->setType('description', PARAM_RAW); + $mform->setType('description', PARAM_CLEANHTML); $mform->addElement('date_time_selector', 'duedate', get_string('duedate', 'tool_lp'), array('optional' => true)); $mform->addHelpButton('duedate', 'duedate', 'tool_lp'); diff --git a/admin/tool/lp/classes/form/template.php b/admin/tool/lp/classes/form/template.php index 8922d249bec..905b0bf30bd 100644 --- a/admin/tool/lp/classes/form/template.php +++ b/admin/tool/lp/classes/form/template.php @@ -58,7 +58,7 @@ class template extends persistent { // Description. $mform->addElement('editor', 'description', get_string('description', 'tool_lp'), array('rows' => 4)); - $mform->setType('description', PARAM_RAW); + $mform->setType('description', PARAM_CLEANHTML); $mform->addElement('selectyesno', 'visible', get_string('visible', 'tool_lp')); $mform->addElement('date_time_selector', diff --git a/admin/tool/lp/classes/form/user_evidence.php b/admin/tool/lp/classes/form/user_evidence.php index 28de24cb2cd..1d368566f60 100644 --- a/admin/tool/lp/classes/form/user_evidence.php +++ b/admin/tool/lp/classes/form/user_evidence.php @@ -54,7 +54,7 @@ class user_evidence extends persistent { $mform->addRule('name', get_string('maximumchars', '', 100), 'maxlength', 100, 'client'); // Description. $mform->addElement('editor', 'description', get_string('userevidencedescription', 'tool_lp'), array('rows' => 10)); - $mform->setType('description', PARAM_RAW); + $mform->setType('description', PARAM_CLEANHTML); $mform->addElement('url', 'url', get_string('userevidenceurl', 'tool_lp'), array(), array('usefilepicker' => false)); $mform->setType('url', PARAM_RAW_TRIMMED); // Can not use PARAM_URL, it silently converts bad URLs to ''. diff --git a/competency/classes/competency.php b/competency/classes/competency.php index 8af4927ff28..65657ec2a7d 100644 --- a/competency/classes/competency.php +++ b/competency/classes/competency.php @@ -68,7 +68,7 @@ class competency extends persistent { ), 'description' => array( 'default' => '', - 'type' => PARAM_RAW + 'type' => PARAM_CLEANHTML ), 'descriptionformat' => array( 'choices' => array(FORMAT_HTML, FORMAT_MOODLE, FORMAT_PLAIN, FORMAT_MARKDOWN), diff --git a/competency/classes/competency_framework.php b/competency/classes/competency_framework.php index b1f323aab7f..704560900fa 100644 --- a/competency/classes/competency_framework.php +++ b/competency/classes/competency_framework.php @@ -90,7 +90,7 @@ class competency_framework extends persistent { 'type' => PARAM_RAW ), 'description' => array( - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, 'default' => '' ), 'descriptionformat' => array( diff --git a/competency/classes/external/exporter.php b/competency/classes/external/exporter.php index d54c6c6cf8b..23fb6c65194 100644 --- a/competency/classes/external/exporter.php +++ b/competency/classes/external/exporter.php @@ -334,7 +334,8 @@ abstract class exporter { */ final protected static function get_format_field($definitions, $property) { $formatproperty = $property . 'format'; - if ($definitions[$property]['type'] == PARAM_RAW && isset($definitions[$formatproperty]) + if (($definitions[$property]['type'] == PARAM_RAW || $definitions[$property]['type'] == PARAM_CLEANHTML) + && isset($definitions[$formatproperty]) && $definitions[$formatproperty]['type'] == PARAM_INT) { return $formatproperty; } @@ -451,7 +452,7 @@ abstract class exporter { // This is a nested array of more properties. $thisvalue = self::get_read_structure_from_properties($type, $proprequired, $propdefault); } else { - if ($definition['type'] == PARAM_TEXT) { + if ($definition['type'] == PARAM_TEXT || $definition['type'] == PARAM_CLEANHTML) { // PARAM_TEXT always becomes PARAM_RAW because filters may be applied. $type = PARAM_RAW; } diff --git a/competency/classes/persistent.php b/competency/classes/persistent.php index fe94961f17f..e1c95b3ed48 100644 --- a/competency/classes/persistent.php +++ b/competency/classes/persistent.php @@ -248,7 +248,8 @@ abstract class persistent { $formatted = array(); foreach ($properties as $property => $definition) { $propertyformat = $property . 'format'; - if ($definition['type'] == PARAM_RAW && array_key_exists($propertyformat, $properties) + if (($definition['type'] == PARAM_RAW || $definition['type'] == PARAM_CLEANHTML) + && array_key_exists($propertyformat, $properties) && $properties[$propertyformat]['type'] == PARAM_INT) { $formatted[$property] = $propertyformat; } @@ -616,6 +617,10 @@ abstract class persistent { // Validate_param() does not like false with PARAM_BOOL, better to convert it to int. $value = 0; } + if ($definition['type'] === PARAM_CLEANHTML) { + // We silently clean for this type. It may introduce changes even to valid data. + $value = clean_param($value, PARAM_CLEANHTML); + } validate_param($value, $definition['type'], $definition['null']); } catch (invalid_parameter_exception $e) { $errors[$property] = static::get_property_error_message($property); diff --git a/competency/classes/plan.php b/competency/classes/plan.php index 91f397d7698..4afd355b467 100644 --- a/competency/classes/plan.php +++ b/competency/classes/plan.php @@ -71,7 +71,7 @@ class plan extends persistent { 'type' => PARAM_TEXT, ), 'description' => array( - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, 'default' => '' ), 'descriptionformat' => array( diff --git a/competency/classes/template.php b/competency/classes/template.php index 93d54b520f3..5d57f4d2086 100644 --- a/competency/classes/template.php +++ b/competency/classes/template.php @@ -53,7 +53,7 @@ class template extends persistent { ), 'description' => array( 'default' => '', - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, ), 'descriptionformat' => array( 'choices' => array(FORMAT_HTML, FORMAT_MOODLE, FORMAT_PLAIN, FORMAT_MARKDOWN), diff --git a/competency/classes/user_evidence.php b/competency/classes/user_evidence.php index 50747d32f28..50737f2ff2a 100644 --- a/competency/classes/user_evidence.php +++ b/competency/classes/user_evidence.php @@ -53,7 +53,7 @@ class user_evidence extends persistent { 'type' => PARAM_TEXT ), 'description' => array( - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, 'default' => '', ), 'descriptionformat' => array( -- 2.43.0