From ac40d8b589820929fe4201a3f0640414e2b9dabd Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 7 Mar 2017 11:19:00 +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/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/plan.php | 2 +- competency/classes/template.php | 2 +- competency/classes/user_evidence.php | 2 +- lib/classes/external/exporter.php | 5 +++-- lib/classes/form/persistent.php | 7 ++++++- lib/classes/persistent.php | 7 ++++++- 13 files changed, 25 insertions(+), 14 deletions(-) diff --git a/admin/tool/lp/classes/form/competency.php b/admin/tool/lp/classes/form/competency.php index 0a6e154627a46..a59427cadddd9 100644 --- a/admin/tool/lp/classes/form/competency.php +++ b/admin/tool/lp/classes/form/competency.php @@ -103,7 +103,7 @@ public function definition() { // 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 6a20db29b483f..ee5d8f74e8413 100644 --- a/admin/tool/lp/classes/form/competency_framework.php +++ b/admin/tool/lp/classes/form/competency_framework.php @@ -63,7 +63,7 @@ public function definition() { // 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/plan.php b/admin/tool/lp/classes/form/plan.php index 3ce193d4839d7..7954f8725eacd 100644 --- a/admin/tool/lp/classes/form/plan.php +++ b/admin/tool/lp/classes/form/plan.php @@ -60,7 +60,7 @@ public function definition() { $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 c73b93caa7a60..c2d6bc3da809b 100644 --- a/admin/tool/lp/classes/form/template.php +++ b/admin/tool/lp/classes/form/template.php @@ -60,7 +60,7 @@ public function definition() { // 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 9d9ccf807a822..a5982ded13bd6 100644 --- a/admin/tool/lp/classes/form/user_evidence.php +++ b/admin/tool/lp/classes/form/user_evidence.php @@ -56,7 +56,7 @@ public function definition() { $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('size' => '60'), 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 675e722068717..a38681c7d8f3a 100644 --- a/competency/classes/competency.php +++ b/competency/classes/competency.php @@ -68,7 +68,7 @@ protected static function define_properties() { ), '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 54a770f6de5fd..f47a1abd0ce1d 100644 --- a/competency/classes/competency_framework.php +++ b/competency/classes/competency_framework.php @@ -90,7 +90,7 @@ protected static function define_properties() { 'type' => PARAM_RAW ), 'description' => array( - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, 'default' => '' ), 'descriptionformat' => array( diff --git a/competency/classes/plan.php b/competency/classes/plan.php index 3d1363d9e82b0..92c0d208fe019 100644 --- a/competency/classes/plan.php +++ b/competency/classes/plan.php @@ -71,7 +71,7 @@ protected static function define_properties() { '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 6cc0d3fb00f5d..2cb57134715ae 100644 --- a/competency/classes/template.php +++ b/competency/classes/template.php @@ -53,7 +53,7 @@ protected static function define_properties() { ), '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 ffbc9d440d43b..d70084b7ec106 100644 --- a/competency/classes/user_evidence.php +++ b/competency/classes/user_evidence.php @@ -53,7 +53,7 @@ protected static function define_properties() { 'type' => PARAM_TEXT ), 'description' => array( - 'type' => PARAM_RAW, + 'type' => PARAM_CLEANHTML, 'default' => '', ), 'descriptionformat' => array( diff --git a/lib/classes/external/exporter.php b/lib/classes/external/exporter.php index dcfeb4e496f91..9a26281591802 100644 --- a/lib/classes/external/exporter.php +++ b/lib/classes/external/exporter.php @@ -394,7 +394,8 @@ final protected static function get_context_structure() { */ 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; } @@ -512,7 +513,7 @@ final protected static function get_read_structure_from_properties($properties, // 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/lib/classes/form/persistent.php b/lib/classes/form/persistent.php index 817bed9c231e2..8d995339003f0 100644 --- a/lib/classes/form/persistent.php +++ b/lib/classes/form/persistent.php @@ -223,9 +223,14 @@ protected function get_default_data() { $data = $this->get_persistent()->to_record(); $class = static::$persistentclass; $properties = $class::get_formatted_properties(); + $allproperties = $class::properties_definition(); foreach ($data as $field => $value) { - // Convert formatted properties. + // 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']); + } + if (isset($properties[$field])) { $data->$field = array( 'text' => $data->$field, diff --git a/lib/classes/persistent.php b/lib/classes/persistent.php index fbd5326568586..ac214b0d7c23f 100644 --- a/lib/classes/persistent.php +++ b/lib/classes/persistent.php @@ -312,7 +312,8 @@ final public static function get_formatted_properties() { $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; } @@ -696,6 +697,10 @@ final public function validate() { // 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);