MDL-50794 workshop: Improve the file type restricting implementation
authorDavid Mudrák <david@moodle.com>
Wed, 24 Feb 2016 15:49:32 +0000 (16:49 +0100)
committerDavid Mudrák <david@moodle.com>
Thu, 25 Feb 2016 17:03:18 +0000 (18:03 +0100)
This is basically a clean up and what I think improved version of the
original Mahmoud's patch.

The actual checking for allowed file extensions has been re-implemented
and is now covered by unit tests. The list of allowed extensions is now
also assed to the filemanager element's accepted_types option to prevent
picking other files (we still need the in-place validation though). The
form validation is simplified a bit. The custom validation of file size
introduced in the previous patch has been removed as not related to this
issue (also I believe it should not be done at this level).

mod/workshop/db/upgrade.php
mod/workshop/exsubmission.php
mod/workshop/form/assessment_form.php
mod/workshop/lang/en/workshop.php
mod/workshop/lib.php
mod/workshop/locallib.php
mod/workshop/mod_form.php
mod/workshop/submission.php
mod/workshop/submission_form.php
mod/workshop/tests/locallib_test.php

index 4c76794..9a7c281 100644 (file)
@@ -49,27 +49,24 @@ function xmldb_workshop_upgrade($oldversion) {
     $dbman = $DB->get_manager();
 
     if ($oldversion < 2016022200) {
-
-        // Define field submissionfiletypes to be added to workshop.
+        // Add field submissionfiletypes to the table workshop.
         $table = new xmldb_table('workshop');
         $field = new xmldb_field('submissionfiletypes', XMLDB_TYPE_CHAR, '255', null, null, null, null, 'nattachments');
 
-        // Conditionally launch add field submissionfiletypes.
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
         }
 
-        // Define field overallfeedbackfiletypes to be added to workshop.
+        // Add field overallfeedbackfiletypes to the table workshop.
         $field = new xmldb_field('overallfeedbackfiletypes',
                 XMLDB_TYPE_CHAR, '255', null, null, null, null, 'overallfeedbackfiles');
 
-        // Conditionally launch add field overallfeedbackfiletypes.
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
         }
 
-        // Workshop savepoint reached.
         upgrade_mod_savepoint(true, 2016022200, 'workshop');
     }
+
     return true;
 }
index ecb7f98..4f794bf 100644 (file)
@@ -115,25 +115,14 @@ if ($id and $assess and $canassess) {
 if ($edit and $canmanage) {
     require_once(dirname(__FILE__).'/submission_form.php');
 
-    $maxfiles       = $workshop->nattachments;
-    $maxbytes       = $workshop->maxbytes;
-    $contentopts    = array(
-                        'trusttext' => true,
-                        'subdirs'   => false,
-                        'maxfiles'  => $maxfiles,
-                        'filetypes' => $workshop->submissionfiletypes,
-                        'maxbytes'  => $maxbytes,
-                        'context'   => $workshop->context
-                      );
-
-    $attachmentopts = array('subdirs' => true, 'maxfiles' => $maxfiles, 'maxbytes' => $maxbytes);
-    $example        = file_prepare_standard_editor($example, 'content', $contentopts, $workshop->context,
-                                        'mod_workshop', 'submission_content', $example->id);
-    $example        = file_prepare_standard_filemanager($example, 'attachment', $attachmentopts, $workshop->context,
-                                        'mod_workshop', 'submission_attachment', $example->id);
-
-    $mform          = new workshop_submission_form($PAGE->url, array('current' => $example, 'workshop' => $workshop,
-                                                    'contentopts' => $contentopts, 'attachmentopts' => $attachmentopts));
+    $example = file_prepare_standard_editor($example, 'content', $workshop->submission_content_options(),
+        $workshop->context, 'mod_workshop', 'submission_content', $example->id);
+
+    $example = file_prepare_standard_filemanager($example, 'attachment', $workshop->submission_attachment_options(),
+        $workshop->context, 'mod_workshop', 'submission_attachment', $example->id);
+
+    $mform = new workshop_submission_form($PAGE->url, array('current' => $example, 'workshop' => $workshop,
+        'contentopts' => $workshop->submission_content_options(), 'attachmentopts' => $workshop->submission_attachment_options()));
 
     if ($mform->is_cancelled()) {
         redirect($workshop->view_url());
index 809c0ef..8432cab 100644 (file)
@@ -122,50 +122,38 @@ class workshop_assessment_form extends moodleform {
     }
 
     /**
-     * Validate incoming data.
+     * Validate assessment form data.
      *
      * @param array $data
      * @param array $files
      * @return array
      */
     public function validation($data, $files) {
-        $errors = parent::validation($data, $files);
-
-        if (isset($data['feedbackauthorattachment_filemanager'])) {
-            $draftitemid = $data['feedbackauthorattachment_filemanager'];
 
-            // If we have draft files, then make sure they are the correct ones.
-            if ($draftfiles = file_get_drafarea_files($draftitemid)) {
+        $errors = parent::validation($data, $files);
 
-                if (!$validfileextensions = workshop::get_array_of_file_extensions($this->workshop->overallfeedbackfiletypes)) {
-                    return $errors;
-                }
-                $wrongfileextensions = null;
-                $bigfiles = null;
-
-                // Check the size of each file.
-                foreach ($draftfiles->list as $file) {
-                    $a = new stdClass();
-                    $a->maxbytes = $this->workshop->overallfeedbackmaxbytes;
-                    $a->currentbytes = $file->size;
-                    $a->filename = $file->filename;
-                    $a->validfileextensions = implode(',', $validfileextensions);
-
-                    // Check whether the extension of uploaded file is in the list.
-                    $thisextension = substr(strrchr($file->filename, '.'), 1);
-                    if (!in_array($thisextension, $validfileextensions)) {
-                        $wrongfileextensions .= get_string('err_wrongfileextension', 'workshop', $a) . '<br/>';
+        if (isset($data['feedbackauthorattachment_filemanager']) and isset($this->workshop->overallfeedbackfiletypes)) {
+            $whitelist = workshop::normalize_file_extensions($this->workshop->overallfeedbackfiletypes);
+            if ($whitelist) {
+                $draftfiles = file_get_drafarea_files($data['feedbackauthorattachment_filemanager']);
+                if ($draftfiles) {
+                    $wrongfiles = array();
+                    foreach ($draftfiles->list as $file) {
+                        if (!workshop::is_allowed_file_type($file->filename, $whitelist)) {
+                            $wrongfiles[] = $file->filename;
+                        }
                     }
-                    if ($file->size > $this->workshop->overallfeedbackmaxbytes) {
-                        $bigfiles .= get_string('err_maxbytes', 'workshop', $a) . '<br/>';
+                    if ($wrongfiles) {
+                        $a = array(
+                            'whitelist' => workshop::clean_file_extensions($whitelist),
+                            'wrongfiles' => implode(', ', $wrongfiles),
+                        );
+                        $errors['feedbackauthorattachment_filemanager'] = get_string('err_wrongfileextension', 'mod_workshop', $a);
                     }
                 }
-                if ($bigfiles || $wrongfileextensions) {
-                    $errors['feedbackauthorattachment_filemanager'] = $bigfiles . $wrongfileextensions;
-                }
             }
         }
+
         return $errors;
     }
-
 }
index a0dd09e..e382cfc 100644 (file)
@@ -31,9 +31,15 @@ $string['allocationdone'] = 'Allocation done';
 $string['allocationerror'] = 'Allocation error';
 $string['allocationconfigured'] = 'Allocation configured';
 $string['allowedfiletypesforoverallfeedback'] = 'Feedback attachment allowed file types';
-$string['allowedfiletypesforoverallfeedback_help'] = 'Feedback attachment allowed file types can be restricted by entering a comma-separated list of file extensions, for example \'mp4, mp3, jpg, jpeg\'. If the field is left empty, then all file types are allowed.';
+$string['allowedfiletypesforoverallfeedback_help'] = 'Feedback attachment allowed file types can be restricted by entering a comma-separated list of file extensions, for example \'png, jpg, jpeg, gif\'. If the field is left empty, then all file types are allowed.
+
+Additional supported file extensions can be configured in the server administration';
+$string['allowedfiletypesforoverallfeedback_link'] = 'admin/tool/filetypes/index';
 $string['allowedfiletypesforsubmission'] = 'Submission attachment allowed file types';
-$string['allowedfiletypesforsubmission_help'] = 'Submission attachment allowed file types can be restricted by entering a comma-separated list of file extensions, for example \'mp4, mp3, jpg, jpeg\'. If the field is left empty, then all file types are allowed.';
+$string['allowedfiletypesforsubmission_help'] = 'Submission attachment allowed file types can be restricted by entering a comma-separated list of file extensions, for example `png, jpg, jpeg, gif`. If the field is left empty, then all file types are allowed.
+
+Additional supported file extensions can be configured in the server administration';
+$string['allowedfiletypesforsubmission_link'] = 'admin/tool/filetypes/index';
 $string['allsubmissions'] = 'All submissions ({$a})';
 $string['alreadygraded'] = 'Already graded';
 $string['areaconclusion'] = 'Conclusion text';
@@ -104,9 +110,8 @@ $string['editingsubmission'] = 'Editing submission';
 $string['editsubmission'] = 'Edit submission';
 $string['err_multiplesubmissions'] = 'While editing this form, another version of the submission has been saved. Multiple submissions per user are not allowed.';
 $string['err_removegrademappings'] = 'Unable to remove the unused grade mappings';
-$string['err_maxbytes'] = 'The attachment "{$a->filename} ({$a->currentbytes} bytes)" exeeds the maximum allowed file size ({$a->maxbytes} bytes)';
-$string['err_notallowedfiletype'] = 'The file extension "{$a}" is not allowed';
-$string['err_wrongfileextension'] = 'The file "{$a->filename}" cannot be uploaded, only file types "{$a->validfileextensions}" are allowed';
+$string['err_unknownfileextension'] = 'Unknown file extension: {$a}';
+$string['err_wrongfileextension'] = 'Some files ({$a->wrongfiles}) cannot be uploaded. Only file types {$a->whitelist} are allowed.';
 $string['evaluategradeswait'] = 'Please wait until the assessments are evaluated and the grades are calculated';
 $string['evaluation'] = 'Grading evaluation';
 $string['evaluationmethod'] = 'Grading evaluation method';
index e553500..9dc499b 100644 (file)
@@ -81,6 +81,14 @@ function workshop_add_instance(stdclass $workshop) {
     $workshop->phaseswitchassessment = (int)!empty($workshop->phaseswitchassessment);
     $workshop->evaluation            = 'best';
 
+    if (isset($workshop->submissionfiletypes)) {
+        $workshop->submissionfiletypes = workshop::clean_file_extensions($workshop->submissionfiletypes);
+    }
+
+    if (isset($workshop->overallfeedbackfiletypes)) {
+        $workshop->overallfeedbackfiletypes = workshop::clean_file_extensions($workshop->overallfeedbackfiletypes);
+    }
+
     // insert the new record so we get the id
     $workshop->id = $DB->insert_record('workshop', $workshop);
 
@@ -141,6 +149,14 @@ function workshop_update_instance(stdclass $workshop) {
     $workshop->latesubmissions       = (int)!empty($workshop->latesubmissions);
     $workshop->phaseswitchassessment = (int)!empty($workshop->phaseswitchassessment);
 
+    if (isset($workshop->submissionfiletypes)) {
+        $workshop->submissionfiletypes = workshop::clean_file_extensions($workshop->submissionfiletypes);
+    }
+
+    if (isset($workshop->overallfeedbackfiletypes)) {
+        $workshop->overallfeedbackfiletypes = workshop::clean_file_extensions($workshop->overallfeedbackfiletypes);
+    }
+
     // todo - if the grading strategy is being changed, we may want to replace all aggregated peer grades with nulls
 
     $DB->update_record('workshop', $workshop);
index eb7199b..8418b3d 100644 (file)
@@ -418,37 +418,116 @@ class workshop {
     }
 
     /**
-     * Split a list of file extensions to an array
-     * @param string  $listofextensions
-     * @return array of extensions
+     * Converts the argument into an array (list) of file extensions.
+     *
+     * The list can be separated by whitespace, end of lines, commas colons and semicolons.
+     * Empty values are not returned. Values are converted to lowercase.
+     * Duplicates are removed. Glob evaluation is not supported.
+     *
+     * @param string|array $extensions list of file extensions
+     * @return array of strings
      */
-    public static function get_array_of_file_extensions($listofextensions) {
-        return preg_split("/[\s,;:\"']+/", strtolower($listofextensions), null, PREG_SPLIT_NO_EMPTY);
+    public static function normalize_file_extensions($extensions) {
+
+        if ($extensions === '') {
+            return array();
+        }
+
+        if (!is_array($extensions)) {
+            $extensions = preg_split('/[\s,;:"\']+/', $extensions, null, PREG_SPLIT_NO_EMPTY);
+        }
+
+        foreach ($extensions as $i => $extension) {
+            $extension = str_replace('*.', '', $extension);
+            $extension = strtolower($extension);
+            $extension = ltrim($extension, '.');
+            $extension = trim($extension);
+            $extensions[$i] = $extension;
+        }
+
+        foreach ($extensions as $i => $extension) {
+            if (strpos($extension, '*') !== false or strpos($extension, '?') !== false) {
+                unset($extensions[$i]);
+            }
+        }
+
+        $extensions = array_filter($extensions, 'strlen');
+        $extensions = array_keys(array_flip($extensions));
+
+        foreach ($extensions as $i => $extension) {
+            $extensions[$i] = '.'.$extension;
+        }
+
+        return $extensions;
     }
 
     /**
-     * Check allowed file types and return an error when invalid file extensions found in the list
+     * Cleans the user provided list of file extensions.
      *
-     * @param string $extensionlist
+     * @param string $extensions
+     * @return string
      */
-    public static function check_allowed_file_types($extensionlist) {
-        if (!$extensionlist) {
-            return '';
+    public static function clean_file_extensions($extensions) {
+
+        $extensions = self::normalize_file_extensions($extensions);
+
+        foreach ($extensions as $i => $extension) {
+            $extensions[$i] = ltrim($extension, '.');
         }
 
-        if ($extensions = self::get_array_of_file_extensions($extensionlist)) {
-            $coreextensions = array_keys(get_mimetypes_array());
-            foreach ($extensions as $ext) {
-                $ext = ltrim($ext, '.');
-                if (!$ext) {
-                    continue;
-                }
-                // Use strtolower(), because all extensions are in lower case.
-                if (!in_array($ext, $coreextensions)) {
-                    return get_string('err_notallowedfiletype', 'workshop', $ext);
-                }
+        return implode(', ', $extensions);
+    }
+
+    /**
+     * Check given file types and return invalid/unknown ones.
+     *
+     * Empty whitelist is interpretted as "any extension is valid".
+     *
+     * @param string|array $extensions list of file extensions
+     * @param string|array $whitelist list of valid extensions
+     * @return array list of invalid extensions not found in the whitelist
+     */
+    public static function invalid_file_extensions($extensions, $whitelist) {
+
+        $extensions = self::normalize_file_extensions($extensions);
+        $whitelist = self::normalize_file_extensions($whitelist);
+
+        if (empty($extensions) or empty($whitelist)) {
+            return array();
+        }
+
+        // Return those items from $extensions that are not present in $whitelist.
+        return array_keys(array_diff_key(array_flip($extensions), array_flip($whitelist)));
+    }
+
+    /**
+     * Is the file have allowed to be uploaded to the workshop?
+     *
+     * Empty whitelist is interpretted as "any file type is allowed" rather
+     * than "no file can be uploaded".
+     *
+     * @param string $filename the file name
+     * @param string|array $whitelist list of allowed file extensions
+     * @return false
+     */
+    public static function is_allowed_file_type($filename, $whitelist) {
+
+        $whitelist = self::normalize_file_extensions($whitelist);
+
+        if (empty($whitelist)) {
+            return true;
+        }
+
+        $haystack = strrev(trim(strtolower($filename)));
+
+        foreach ($whitelist as $extension) {
+            if (strpos($haystack, strrev($extension)) === 0) {
+                // The file name ends with the extension.
+                return true;
             }
         }
+
+        return false;
     }
 
     ////////////////////////////////////////////////////////////////////////////////
@@ -2401,6 +2480,43 @@ class workshop {
         return false;
     }
 
+    /**
+     * Return the editor options for the submission content field.
+     *
+     * @return array
+     */
+    public function submission_content_options() {
+        return array(
+            'trusttext' => true,
+            'subdirs' => false,
+            'maxfiles' => $this->nattachments,
+            'maxbytes' => $this->maxbytes,
+            'context' => $this->context,
+            'return_types' => FILE_INTERNAL | FILE_EXTERNAL,
+          );
+    }
+
+    /**
+     * Return the filemanager options for the submission attachments field.
+     *
+     * @return array
+     */
+    public function submission_attachment_options() {
+
+        $options = array(
+            'subdirs' => true,
+            'maxfiles' => $this->nattachments,
+            'maxbytes' => $this->maxbytes,
+            'return_types' => FILE_INTERNAL,
+        );
+
+        if ($acceptedtypes = self::normalize_file_extensions($this->submissionfiletypes)) {
+            $options['accepted_types'] = $acceptedtypes;
+        }
+
+        return $options;
+    }
+
     /**
      * Return the editor options for the overall feedback for the author.
      *
@@ -2410,7 +2526,6 @@ class workshop {
         return array(
             'subdirs' => 0,
             'maxbytes' => $this->overallfeedbackmaxbytes,
-            'filetypes' => $this->overallfeedbackfiletypes,
             'maxfiles' => $this->overallfeedbackfiles,
             'changeformat' => 1,
             'context' => $this->context,
@@ -2423,13 +2538,19 @@ class workshop {
      * @return array
      */
     public function overall_feedback_attachment_options() {
-        return array(
+
+        $options = array(
             'subdirs' => 1,
             'maxbytes' => $this->overallfeedbackmaxbytes,
-            'filetypes' => $this->overallfeedbackfiletypes,
             'maxfiles' => $this->overallfeedbackfiles,
             'return_types' => FILE_INTERNAL,
         );
+
+        if ($acceptedtypes = self::normalize_file_extensions($this->overallfeedbackfiletypes)) {
+            $options['accepted_types'] = $acceptedtypes;
+        }
+
+        return $options;
     }
 
     /**
index 6dbbf2d..64b9ac0 100644 (file)
@@ -361,19 +361,14 @@ class mod_workshop_mod_form extends moodleform_mod {
     public function validation($data, $files) {
         $errors = parent::validation($data, $files);
 
-        // Check the input of submissionfiletypes field.
-        if ($data['submissionfiletypes']) {
-            $invalidextension = workshop::check_allowed_file_types($data['submissionfiletypes']);
-            if ($invalidextension) {
-                $errors['submissionfiletypes'] = $invalidextension;
-            }
-        }
-
-        // Check the input of overallfeedbackfiletypes field.
-        if ($data['overallfeedbackfiletypes']) {
-            $invalidextension = workshop::check_allowed_file_types($data['overallfeedbackfiletypes']);
-            if ($invalidextension) {
-                $errors['overallfeedbackfiletypes'] = $invalidextension;
+        // Validate lists of allowed extensions.
+        foreach (array('submissionfiletypes', 'overallfeedbackfiletypes') as $fieldname) {
+            if (isset($data[$fieldname])) {
+                $invalidextensions = workshop::invalid_file_extensions($data[$fieldname], array_keys(core_filetypes::get_types()));
+                if ($invalidextensions) {
+                    $errors[$fieldname] = get_string('err_unknownfileextension', 'mod_workshop',
+                        workshop::clean_file_extensions($invalidextensions));
+                }
             }
         }
 
index d67dfd0..000c63f 100644 (file)
@@ -183,28 +183,14 @@ if ($assess and $submission->id and !$isreviewer and $canallocate and $workshop-
 if ($edit) {
     require_once(dirname(__FILE__).'/submission_form.php');
 
-    $maxfiles       = $workshop->nattachments;
-    $filetypes      = $workshop->submissionfiletypes;
-    $maxbytes       = $workshop->maxbytes;
-    $contentopts    = array(
-                        'trusttext' => true,
-                        'subdirs'   => false,
-                        'maxfiles'  => $maxfiles,
-                        'filetypes' => $filetypes,
-                        'maxbytes'  => $maxbytes,
-                        'context'   => $workshop->context,
-                        'return_types' => FILE_INTERNAL | FILE_EXTERNAL
-                      );
-
-    $attachmentopts = array('subdirs' => true, 'maxfiles' => $maxfiles, 'filetypes' => $filetypes, 'maxbytes' => $maxbytes,
-                                        'return_types' => FILE_INTERNAL);
-    $submission     = file_prepare_standard_editor($submission, 'content', $contentopts, $workshop->context,
-                                        'mod_workshop', 'submission_content', $submission->id);
-    $submission     = file_prepare_standard_filemanager($submission, 'attachment', $attachmentopts, $workshop->context,
-                                        'mod_workshop', 'submission_attachment', $submission->id);
-
-    $mform          = new workshop_submission_form($PAGE->url, array('current' => $submission, 'workshop' => $workshop,
-                                                    'contentopts' => $contentopts, 'attachmentopts' => $attachmentopts));
+    $submission = file_prepare_standard_editor($submission, 'content', $workshop->submission_content_options(),
+        $workshop->context, 'mod_workshop', 'submission_content', $submission->id);
+
+    $submission = file_prepare_standard_filemanager($submission, 'attachment', $workshop->submission_attachment_options(),
+        $workshop->context, 'mod_workshop', 'submission_attachment', $submission->id);
+
+    $mform = new workshop_submission_form($PAGE->url, array('current' => $submission, 'workshop' => $workshop,
+        'contentopts' => $workshop->submission_content_options(), 'attachmentopts' => $workshop->submission_attachment_options()));
 
     if ($mform->is_cancelled()) {
         redirect($workshop->view_url());
@@ -257,11 +243,14 @@ if ($edit) {
             }
         }
         $params['objectid'] = $submission->id;
-        // save and relink embedded images and save attachments
-        $formdata = file_postupdate_standard_editor($formdata, 'content', $contentopts, $workshop->context,
-                                                      'mod_workshop', 'submission_content', $submission->id);
-        $formdata = file_postupdate_standard_filemanager($formdata, 'attachment', $attachmentopts, $workshop->context,
-                                                           'mod_workshop', 'submission_attachment', $submission->id);
+
+        // Save and relink embedded images and save attachments.
+        $formdata = file_postupdate_standard_editor($formdata, 'content', $workshop->submission_content_options(),
+            $workshop->context, 'mod_workshop', 'submission_content', $submission->id);
+
+        $formdata = file_postupdate_standard_filemanager($formdata, 'attachment', $workshop->submission_attachment_options(),
+            $workshop->context, 'mod_workshop', 'submission_attachment', $submission->id);
+
         if (empty($formdata->attachment)) {
             // explicit cast to zero integer
             $formdata->attachment = 0;
index f411f4e..398cd4a 100644 (file)
@@ -37,8 +37,6 @@ class workshop_submission_form extends moodleform {
         $contentopts    = $this->_customdata['contentopts'];
         $attachmentopts = $this->_customdata['attachmentopts'];
 
-        $this->attachmentopts = $attachmentopts;
-
         $mform->addElement('header', 'general', get_string('submission', 'workshop'));
 
         $mform->addElement('text', 'title', get_string('submissiontitle', 'workshop'));
@@ -90,42 +88,28 @@ class workshop_submission_form extends moodleform {
             }
         }
 
-        if (isset ($data['attachment_filemanager'])) {
-            $draftitemid = $data['attachment_filemanager'];
-
-            // If we have draft files, then make sure they are the correct ones.
-            if ($draftfiles = file_get_drafarea_files($draftitemid)) {
-
-                if (!$validfileextensions = workshop::get_array_of_file_extensions($this->attachmentopts['filetypes'])) {
-                    return $errors;
-                }
-                $wrongfileextensions = null;
-                $bigfiles = null;
-
-                // Check the size and type of each file.
-                foreach ($draftfiles->list as $file) {
-                    $a = new stdClass();
-                    $a->maxbytes = $this->attachmentopts['maxbytes'];
-                    $a->currentbytes = $file->size;
-                    $a->filename = $file->filename;
-                    $a->validfileextensions = implode(',', $validfileextensions);
-
-                    // Check whether the extension of uploaded file is in the list.
-                    $thisextension = substr(strrchr($file->filename, '.'), 1);
-                    if (!in_array($thisextension, $validfileextensions)) {
-                        $wrongfileextensions .= get_string('err_wrongfileextension', 'workshop', $a) . '<br/>';
+        if (isset($data['attachment_filemanager']) and isset($this->_customdata['workshop']->submissionfiletypes)) {
+            $whitelist = workshop::normalize_file_extensions($this->_customdata['workshop']->submissionfiletypes);
+            if ($whitelist) {
+                $draftfiles = file_get_drafarea_files($data['attachment_filemanager']);
+                if ($draftfiles) {
+                    $wrongfiles = array();
+                    foreach ($draftfiles->list as $file) {
+                        if (!workshop::is_allowed_file_type($file->filename, $whitelist)) {
+                            $wrongfiles[] = $file->filename;
+                        }
                     }
-
-                    // Check whether the file size exceeds the maximum submission attachment size.
-                    if ($file->size > $this->attachmentopts['maxbytes']) {
-                        $bigfiles .= get_string('err_maxbytes', 'workshop', $a) . '<br/>';
+                    if ($wrongfiles) {
+                        $a = array(
+                            'whitelist' => workshop::clean_file_extensions($whitelist),
+                            'wrongfiles' => implode(', ', $wrongfiles),
+                        );
+                        $errors['attachment_filemanager'] = get_string('err_wrongfileextension', 'mod_workshop', $a);
                     }
                 }
-                if ($bigfiles || $wrongfileextensions) {
-                    $errors['attachment_filemanager'] = $bigfiles . $wrongfileextensions;
-                }
             }
         }
+
         return $errors;
     }
 }
index 13b5979..ee4c476 100644 (file)
@@ -624,90 +624,98 @@ class mod_workshop_internal_api_testcase extends advanced_testcase {
     }
 
     /**
-     * Test converting the string to array.
+     * Test normalizing list of extensions.
      */
-    public function test_get_array_of_file_extensions() {
+    public function test_normalize_file_extensions() {
         $this->resetAfterTest(true);
 
-        $listofextensions = 'doc, jpg, mp3';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('doc', 'jpg', 'mp3');
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'mp4,; docx,; gif';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('mp4', 'docx', 'gif');
-        $this->assertEquals($expected, $actual);
+        $this->assertSame(['.odt'], workshop::normalize_file_extensions('odt'));
+        $this->assertSame(['.odt'], workshop::normalize_file_extensions('.odt'));
+        $this->assertSame(['.odt'], workshop::normalize_file_extensions('.ODT'));
+        $this->assertSame(['.doc', '.jpg', '.mp3'], workshop::normalize_file_extensions('doc, jpg, mp3'));
+        $this->assertSame(['.doc', '.jpg', '.mp3'], workshop::normalize_file_extensions(['.doc', '.jpg', '.mp3']));
+        $this->assertSame(['.doc', '.jpg', '.mp3'], workshop::normalize_file_extensions('doc, *.jpg, mp3'));
+        $this->assertSame(['.doc', '.jpg', '.mp3'], workshop::normalize_file_extensions(['doc ', ' JPG ', '.mp3']));
+        $this->assertSame(['.rtf', '.pdf', '.docx'], workshop::normalize_file_extensions("RTF,.pdf\n...DocX,,,;\rPDF\trtf ...Rtf"));
+        $this->assertSame(['.tgz', '.tar.gz'], workshop::normalize_file_extensions('tgz,TAR.GZ tar.gz .tar.gz tgz TGZ'));
+        $this->assertSame(['.notebook'], workshop::normalize_file_extensions('"Notebook":notebook;NOTEBOOK;,\'NoTeBook\''));
+        $this->assertSame([], workshop::normalize_file_extensions(''));
+        $this->assertSame([], workshop::normalize_file_extensions([]));
+        $this->assertSame(['.0'], workshop::normalize_file_extensions(0));
+        $this->assertSame(['.0'], workshop::normalize_file_extensions('0'));
+        $this->assertSame(['.odt'], workshop::normalize_file_extensions('*.odt'));
+        $this->assertSame([], workshop::normalize_file_extensions('.'));
+        $this->assertSame(['.foo'], workshop::normalize_file_extensions('. foo'));
+        $this->assertSame([], workshop::normalize_file_extensions('*'));
+        $this->assertSame([], workshop::normalize_file_extensions('*~'));
+        $this->assertSame(['.pdf', '.ps'], workshop::normalize_file_extensions('* pdf *.ps foo* *bar .r??'));
+    }
 
-        $listofextensions = 'mp4 docx gif';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('mp4', 'docx', 'gif');
-        $this->assertEquals($expected, $actual);
+    /**
+     * Test cleaning list of extensions.
+     */
+    public function test_clean_file_extensions() {
+        $this->resetAfterTest(true);
 
-        $listofextensions = 'MP4 DOCx Gif';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('mp4', 'docx', 'gif');
-        $this->assertEquals($expected, $actual);
+        $this->assertSame('', workshop::clean_file_extensions(''));
+        $this->assertSame('', workshop::clean_file_extensions(null));
+        $this->assertSame('', workshop::clean_file_extensions(' '));
+        $this->assertSame('0', workshop::clean_file_extensions(0));
+        $this->assertSame('0', workshop::clean_file_extensions('0'));
+        $this->assertSame('doc, rtf, pdf', workshop::clean_file_extensions('*.Doc, RTF, PDF, .rtf'.PHP_EOL.'PDF '));
+        $this->assertSame('doc, rtf, pdf', 'doc, rtf, pdf');
+    }
 
-        $listofextensions = '.doc; .jpg; Mp4 "mp3"';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('.doc', '.jpg', 'mp4', 'mp3');
-        $this->assertEquals($expected, $actual);
+    /**
+     * Test validation of the list of file extensions.
+     */
+    public function test_invalid_file_extensions() {
+        $this->resetAfterTest(true);
 
-        $listofextensions = '.doc,;.jpg; .Mp3, ".Avi"';
-        $actual = workshop::get_array_of_file_extensions($listofextensions);
-        $expected = array('.doc', '.jpg', '.mp3', '.avi');
-        $this->assertEquals($expected, $actual);
+        $this->assertSame([], workshop::invalid_file_extensions('', ''));
+        $this->assertSame([], workshop::invalid_file_extensions('', '.doc'));
+        $this->assertSame([], workshop::invalid_file_extensions('odt', ''));
+        $this->assertSame([], workshop::invalid_file_extensions('odt', '*'));
+        $this->assertSame([], workshop::invalid_file_extensions('odt', 'odt'));
+        $this->assertSame([], workshop::invalid_file_extensions('doc, odt, pdf', ['pdf', 'doc', 'odt']));
+        $this->assertSame([], workshop::invalid_file_extensions(['doc', 'odt', 'PDF'], ['.doc', '.pdf', '.odt']));
+        $this->assertSame([], workshop::invalid_file_extensions('*~ .docx, Odt PDF :doc .pdf', '*.docx *.odt *.pdf *.doc'));
+        $this->assertSame(['.00001-wtf-is-this'], workshop::invalid_file_extensions('docx tgz .00001-wtf-is-this', 'tgz docx'));
+        $this->assertSame(['.foobar', '.wtfisthis'], workshop::invalid_file_extensions(['.pdf', '.foobar', 'wtfisthis'], 'pdf'));
+        $this->assertSame([], workshop::invalid_file_extensions('', ''));
+        $this->assertSame(['.odt'], workshop::invalid_file_extensions(['.PDF', 'PDF', '.ODT'], 'jpg pdf png gif'));
+        $this->assertSame(['.odt'], workshop::invalid_file_extensions(['.PDF', 'PDF', '.ODT'], '.jpg,.pdf,  .png .gif'));
+        $this->assertSame(['.exe', '.bat'], workshop::invalid_file_extensions(['.exe', '.odt', '.bat', ''], 'odt'));
     }
 
     /**
-     * Test the list of allowed file extensions.
+     * Test checking file name against the list of allowed extensions.
      */
-    public function test_check_allowed_file_types() {
-        $this->resetAfterTest(true);
-
-        // Valid file extensions.
-        $listofextensions = '';
-        $expected = '';
-        // The function returns '' when file extensions are valid or the input field is empty.
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'doc, jpg, mp3';
-        $expected = '';
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'doc; ".jpg"; mp4 ...mp3';
-        $expected = '';
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        // Error handling.
-        $listofextensions = 'doc.jpg .mp3 .avi';
-        $expected = get_string('err_notallowedfiletype', 'workshop', 'doc.jpg');
-        // The function returns and error on the form-field: 'The file extension "doc.jpg" is not allowed'.
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'doc, jpg, mp3, unusual';
-        $expected = get_string('err_notallowedfiletype', 'workshop', 'unusual');
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'doc,; unusual1, unusual2';
-        $expected = get_string('err_notallowedfiletype', 'workshop', 'unusual1');
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'unusual1,; unsusual2, doc, jpg';
-        $expected = get_string('err_notallowedfiletype', 'workshop', 'unusual1');
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
-
-        $listofextensions = 'unusual1; unusual2; mp4';
-        $expected = get_string('err_notallowedfiletype', 'workshop', 'unusual1');
-        $actual = workshop::check_allowed_file_types($listofextensions);
-        $this->assertEquals($expected, $actual);
+    public function test_is_allowed_file_type() {
+        $this->resetAfterTest(true);
+
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', ''));
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', ['']));
+        $this->assertFalse(workshop::is_allowed_file_type('README.txt', '0'));
+
+        $this->assertFalse(workshop::is_allowed_file_type('README.txt', 'xt'));
+        $this->assertFalse(workshop::is_allowed_file_type('README.txt', 'old.txt'));
+
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', 'txt'));
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', '.TXT'));
+        $this->assertTrue(workshop::is_allowed_file_type('README.TXT', 'txt'));
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', '.txt .md'));
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', 'HTML TXT DOC RTF'));
+        $this->assertTrue(workshop::is_allowed_file_type('README.txt', ['HTML', '...TXT', 'DOC', 'RTF']));
+
+        $this->assertTrue(workshop::is_allowed_file_type('C:\Moodle\course-data.tar.gz', 'gzip zip 7z tar.gz'));
+        $this->assertFalse(workshop::is_allowed_file_type('C:\Moodle\course-data.tar.gz', 'gzip zip 7z tar'));
+        $this->assertTrue(workshop::is_allowed_file_type('~/course-data.tar.gz', 'gzip zip 7z gz'));
+        $this->assertFalse(workshop::is_allowed_file_type('~/course-data.tar.gz', 'gzip zip 7z'));
+
+        $this->assertFalse(workshop::is_allowed_file_type('Alice on the beach.jpg.exe', 'png gif jpg bmp'));
+        $this->assertFalse(workshop::is_allowed_file_type('xfiles.exe.jpg', 'exe com bat sh'));
+        $this->assertFalse(workshop::is_allowed_file_type('solution.odt~', 'odt, xls'));
+        $this->assertTrue(workshop::is_allowed_file_type('solution.odt~', 'odt, odt~'));
     }
 }