From 996f7e8228f7547ad006ceb1489d3f841bb00433 Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20Mudr=C3=A1k?= Date: Wed, 24 Feb 2016 16:49:32 +0100 Subject: [PATCH] MDL-50794 workshop: Improve the file type restricting implementation 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 | 9 +- mod/workshop/exsubmission.php | 27 ++-- mod/workshop/form/assessment_form.php | 50 +++----- mod/workshop/lang/en/workshop.php | 15 ++- mod/workshop/lib.php | 16 +++ mod/workshop/locallib.php | 169 ++++++++++++++++++++++---- mod/workshop/mod_form.php | 21 ++-- mod/workshop/submission.php | 43 +++---- mod/workshop/submission_form.php | 50 +++----- mod/workshop/tests/locallib_test.php | 156 +++++++++++++----------- 10 files changed, 324 insertions(+), 232 deletions(-) diff --git a/mod/workshop/db/upgrade.php b/mod/workshop/db/upgrade.php index 4c76794c07e..9a7c2814b43 100644 --- a/mod/workshop/db/upgrade.php +++ b/mod/workshop/db/upgrade.php @@ -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; } diff --git a/mod/workshop/exsubmission.php b/mod/workshop/exsubmission.php index ecb7f98a923..4f794bfda6c 100644 --- a/mod/workshop/exsubmission.php +++ b/mod/workshop/exsubmission.php @@ -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()); diff --git a/mod/workshop/form/assessment_form.php b/mod/workshop/form/assessment_form.php index 809c0ef133d..8432cab505f 100644 --- a/mod/workshop/form/assessment_form.php +++ b/mod/workshop/form/assessment_form.php @@ -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) . '
'; + 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) . '
'; + 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; } - } diff --git a/mod/workshop/lang/en/workshop.php b/mod/workshop/lang/en/workshop.php index a0dd09ec9c0..e382cfc77b9 100644 --- a/mod/workshop/lang/en/workshop.php +++ b/mod/workshop/lang/en/workshop.php @@ -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'; diff --git a/mod/workshop/lib.php b/mod/workshop/lib.php index e553500a797..9dc499b269d 100644 --- a/mod/workshop/lib.php +++ b/mod/workshop/lib.php @@ -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); diff --git a/mod/workshop/locallib.php b/mod/workshop/locallib.php index eb7199b9cfe..8418b3d40c6 100644 --- a/mod/workshop/locallib.php +++ b/mod/workshop/locallib.php @@ -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; } /** diff --git a/mod/workshop/mod_form.php b/mod/workshop/mod_form.php index 6dbbf2da6d0..64b9ac0eb23 100644 --- a/mod/workshop/mod_form.php +++ b/mod/workshop/mod_form.php @@ -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)); + } } } diff --git a/mod/workshop/submission.php b/mod/workshop/submission.php index d67dfd0aa2e..000c63fbc3e 100644 --- a/mod/workshop/submission.php +++ b/mod/workshop/submission.php @@ -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; diff --git a/mod/workshop/submission_form.php b/mod/workshop/submission_form.php index f411f4e82a3..398cd4a4ba7 100644 --- a/mod/workshop/submission_form.php +++ b/mod/workshop/submission_form.php @@ -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) . '
'; + 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) . '
'; + 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; } } diff --git a/mod/workshop/tests/locallib_test.php b/mod/workshop/tests/locallib_test.php index 13b5979b069..ee4c476cf5f 100644 --- a/mod/workshop/tests/locallib_test.php +++ b/mod/workshop/tests/locallib_test.php @@ -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~')); } } -- 2.43.0