From 8b7d96ca68b37bd2bdaad26bac0473bb35e3043f Mon Sep 17 00:00:00 2001 From: Daniel Ziegenberg Date: Wed, 17 Jun 2020 14:53:44 +0200 Subject: [PATCH] MDL-69078 questions: check for UTF-8 encoding of the import file Signed-off-by: Daniel Ziegenberg --- lang/en/question.php | 1 + question/format.php | 34 +++++++++++++++++++ question/format/examview/format.php | 12 +++++++ question/format/gift/format.php | 12 +++++++ .../gift/tests/behat/import_export.feature | 8 +++++ .../questions_encoding_windows-1252.gift.txt | 18 ++++++++++ question/format/missingword/format.php | 12 +++++++ question/format/multianswer/format.php | 12 +++++++ question/format/upgrade.txt | 5 +++ question/format/webct/format.php | 12 +++++++ question/format/xml/format.php | 12 +++++++ question/import_form.php | 6 ++++ 12 files changed, 144 insertions(+) create mode 100644 question/format/gift/tests/fixtures/questions_encoding_windows-1252.gift.txt diff --git a/lang/en/question.php b/lang/en/question.php index 81227317af6..70785e1b614 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -202,6 +202,7 @@ $string['importparseerror'] = 'Error(s) found parsing the import file. No questi $string['importquestions'] = 'Import questions from file'; $string['importquestions_help'] = 'This function enables questions in a variety of formats to be imported via text file. Note that the file must use UTF-8 encoding.'; $string['importquestions_link'] = 'question/import'; +$string['importwrongfileencoding'] = 'The file you selected is not in UFT-8 character encoding. {$a} files must use UTF-8.'; $string['importwrongfiletype'] = 'The type of the file you selected ({$a->actualtype}) does not match the type expected by this import format ({$a->expectedtype}).'; $string['invalidarg'] = 'No valid arguments supplied or incorrect server configuration'; $string['invalidcategoryidforparent'] = 'Invalid category id for parent!'; diff --git a/question/format.php b/question/format.php index 0177abf03e4..b2885abef03 100644 --- a/question/format.php +++ b/question/format.php @@ -95,6 +95,40 @@ class qformat_default { return ($file->get_mimetype() == $this->mime_type()); } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return ''; + } + + /** + * Check if the given file has the required utf8 encoding. + * + * @param stored_file $file the file to check + * @return string the error message if the file encoding is not UTF-8 + */ + protected function validate_is_utf8_file(stored_file $file): string { + if (!mb_check_encoding($file->get_content(), "UTF-8")) { + return get_string('importwrongfileencoding', 'question', $this->get_name()); + } + return ''; + } + + /** + * Return the localized pluginname string for the question format. + * + * @return string the pluginname string for the question format + */ + protected function get_name(): string { + return get_string('pluginname', get_class($this)); + } + // Accessor methods /** diff --git a/question/format/examview/format.php b/question/format/examview/format.php index e092a877c44..38f84033e75 100644 --- a/question/format/examview/format.php +++ b/question/format/examview/format.php @@ -61,6 +61,18 @@ class qformat_examview extends qformat_based_on_xml { return 'application/xml'; } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + /** * unxmlise reconstructs part of the xml data structure in order * to identify the actual data therein diff --git a/question/format/gift/format.php b/question/format/gift/format.php index 763e8a5cd4f..54752dffe98 100644 --- a/question/format/gift/format.php +++ b/question/format/gift/format.php @@ -73,6 +73,18 @@ class qformat_gift extends qformat_default { return '.txt'; } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + protected function answerweightparser(&$answer) { $answer = substr($answer, 1); // Removes initial %. $endposition = strpos($answer, "%"); diff --git a/question/format/gift/tests/behat/import_export.feature b/question/format/gift/tests/behat/import_export.feature index 48f0227321f..735153795cc 100644 --- a/question/format/gift/tests/behat/import_export.feature +++ b/question/format/gift/tests/behat/import_export.feature @@ -46,3 +46,11 @@ Feature: Test importing questions from GIFT format. And I should see "Match the activity to the description." When I press "Continue" Then I should see "Moodle activities" + + @javascript @_file_upload + Scenario: import some GIFT questions with unsupported encoding + When I navigate to "Question bank > Import" in current page administration + And I set the field "id_format_gift" to "1" + And I upload "question/format/gift/tests/fixtures/questions_encoding_windows-1252.gift.txt" file to "Import" filemanager + And I press "id_submitbutton" + Then I should see "The file you selected is not in UFT-8 character encoding. GIFT format files must use UTF-8." diff --git a/question/format/gift/tests/fixtures/questions_encoding_windows-1252.gift.txt b/question/format/gift/tests/fixtures/questions_encoding_windows-1252.gift.txt new file mode 100644 index 00000000000..d455e42a881 --- /dev/null +++ b/question/format/gift/tests/fixtures/questions_encoding_windows-1252.gift.txt @@ -0,0 +1,18 @@ +// question: 0 name: Switch category to $course$/top/Default for LTTEST +$CATEGORY: $course$/top/Default for LTTEST + + +// question: 19756780 name: asdf +::asdf::[html]

asdf

{} + + +// question: 19756810 name: test daniel +::test daniel::[html]

asdfasdf

{} + + +// question: 19756750 name: asdf +::asdf::[html]

asdf

{ + =

aödf

-> asdf + =

asdf

-> asdf + =

asdf

-> asdf +} diff --git a/question/format/missingword/format.php b/question/format/missingword/format.php index 2ba96a52eef..96f9a20d7cd 100644 --- a/question/format/missingword/format.php +++ b/question/format/missingword/format.php @@ -55,6 +55,18 @@ class qformat_missingword extends qformat_default { return true; } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + public function readquestion($lines) { // Given an array of lines known to define a question in // this format, this function converts it into a question diff --git a/question/format/multianswer/format.php b/question/format/multianswer/format.php index 2ab3109fabf..a2d9c2cec82 100644 --- a/question/format/multianswer/format.php +++ b/question/format/multianswer/format.php @@ -39,6 +39,18 @@ class qformat_multianswer extends qformat_default { return true; } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + public function readquestions($lines) { question_bank::get_qtype('multianswer'); // Ensure the multianswer code is loaded. diff --git a/question/format/upgrade.txt b/question/format/upgrade.txt index a92a7e8a3e0..a47a4a90782 100644 --- a/question/format/upgrade.txt +++ b/question/format/upgrade.txt @@ -1,5 +1,10 @@ This files describes API changes for question import/export format plugins. +=== 3.11.7 === + +* The new validate_file() method in question/format.php can be overwritten + to implement more expensive or detailed file integrity checks. It is called on imported files. + === 3.6 === * Saving question category descriptions (info) is now supported in Moodle XML import/export format. diff --git a/question/format/webct/format.php b/question/format/webct/format.php index 9d1c8155535..7c2490bed8e 100644 --- a/question/format/webct/format.php +++ b/question/format/webct/format.php @@ -189,6 +189,18 @@ class qformat_webct extends qformat_default { return mimeinfo('type', '.zip'); } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + /** * Store an image file in a draft filearea * @param array $text, if itemid element don't exists it will be created diff --git a/question/format/xml/format.php b/question/format/xml/format.php index e32d1ace76a..b1dfc044e53 100644 --- a/question/format/xml/format.php +++ b/question/format/xml/format.php @@ -58,6 +58,18 @@ class qformat_xml extends qformat_default { return 'application/xml'; } + /** + * Validate the given file. + * + * For more expensive or detailed integrity checks. + * + * @param stored_file $file the file to check + * @return string the error message that occurred while validating the given file + */ + public function validate_file(stored_file $file): string { + return $this->validate_is_utf8_file($file); + } + // IMPORT FUNCTIONS START HERE. /** diff --git a/question/import_form.php b/question/import_form.php index f4693e40184..e9806de3e4b 100644 --- a/question/import_form.php +++ b/question/import_form.php @@ -147,6 +147,12 @@ class question_import_form extends moodleform { $a->actualtype = $file->get_mimetype(); $a->expectedtype = $qformat->mime_type(); $errors['newfile'] = get_string('importwrongfiletype', 'question', $a); + return $errors; + } + + $fileerrors = $qformat->validate_file($file); + if ($fileerrors) { + $errors['newfile'] = $fileerrors; } return $errors;