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.
rboyatt · Mar 10, 2017 · ac40d8b · ac40d8b
1 parent 6e65554
commit ac40d8b
Show file tree

Hide file tree

Showing 13 changed files with 25 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);