mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-17 21:49:15 +01:00
Code Issues Projects Releases Wiki Activity

MDL-34841 error importing questions with long names.

Browse Source 
The problem was with the non-UTF-8-safe way that a question name
was being constructed from the question text.

I have done a proper fix with methods in the base class to
carefully construct a question name that is reasonable, and
which will fit in the database column. Then I have changed all
importers to use the new methods.

I also remembered not to break the lesson in the process.
This commit is contained in:
Tim Hunt 2012-09-11 15:04:00 +01:00
parent 935c3d5ed2
commit cacb8fa08a
15 changed files with 154 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
mod/lesson/format.php
View File

@ -520,6 +520,37 @@ class qformat_default {
return NULL;
}
/**
* Construct a reasonable default question name, based on the start of the question text.
* @param string $questiontext the question text.
* @param string $default default question name to use if the constructed one comes out blank.
* @return string a reasonable question name.
*/
public function create_default_question_name($questiontext, $default) {
$name = $this->clean_question_name(shorten_text($questiontext, 80));
if ($name) {
return $name;
} else {
return $default;
}
}
/**
* Ensure that a question name does not contain anything nasty, and will fit in the DB field.
* @param string $name the raw question name.
* @return string a safe question name.
*/
public function clean_question_name($name) {
$name = clean_param($name, PARAM_TEXT); // Matches what the question editing form does.
$name = trim($name);
$trimlength = 251;
while (textlib::strlen($name) > 255 && $trimlength > 0) {
$name = shorten_text($name, $trimlength);
$trimlength -= 10;
}
return $name;
}
function defaultquestion() {
// returns an "empty" question
// Somewhere to specify question parameters that are not handled

31
question/format.php
View File

@ -637,6 +637,37 @@ class qformat_default {
return $question;
}
/**
* Construct a reasonable default question name, based on the start of the question text.
* @param string $questiontext the question text.
* @param string $default default question name to use if the constructed one comes out blank.
* @return string a reasonable question name.
*/
public function create_default_question_name($questiontext, $default) {
$name = $this->clean_question_name(shorten_text($questiontext, 80));
if ($name) {
return $name;
} else {
return $default;
}
}
/**
* Ensure that a question name does not contain anything nasty, and will fit in the DB field.
* @param string $name the raw question name.
* @return string a safe question name.
*/
public function clean_question_name($name) {
$name = clean_param($name, PARAM_TEXT); // Matches what the question editing form does.
$name = trim($name);
$trimlength = 251;
while (textlib::strlen($name) > 255 && $trimlength > 0) {
$name = shorten_text($name, $trimlength);
$trimlength -= 10;
}
return $name;
}
/**
* Add a blank combined feedback to a question object.
* @param object question

2
question/format/aiken/format.php
View File

@ -95,7 +95,7 @@ class qformat_aiken extends qformat_default {
} else {
// Must be the first line of a new question, since no recognised prefix.
$question->qtype = 'multichoice';
$question->name = shorten_text(s($nowline), 50);
$question->name = $this->create_default_question_name($nowline, get_string('questionname', 'question'));
$question->questiontext = htmlspecialchars(trim($nowline), ENT_NOQUOTES);
$question->questiontextformat = FORMAT_HTML;
$question->generalfeedback = '';

10
question/format/blackboard/format.php
View File

@ -123,13 +123,9 @@ class qformat_blackboard extends qformat_based_on_xml {
$question->questiontext = $text;
}
// Put name in question object. We must ensure it is not empty and it is less than 250 chars.
$question->name = shorten_text(strip_tags($question->questiontext), 200);
$question->name = substr($question->name, 0, 250);
if (!$question->name) {
$id = $this->getpath($questiondata,
array('@', 'id'), '', true);
$question->name = get_string('defaultname', 'qformat_blackboard' , $id);
}
$id = $this->getpath($questiondata, array('@', 'id'), '', true);
$question->name = $this->create_default_question_name($question->questiontext,
get_string('defaultname', 'qformat_blackboard', $id));
$question->generalfeedback = '';
$question->generalfeedbackformat = FORMAT_HTML;

10
question/format/blackboard_six/formatpool.php
View File

@ -92,13 +92,9 @@ class qformat_blackboard_six_pool extends qformat_blackboard_six_base {
$question->questiontextformat = FORMAT_HTML; // Needed because add_blank_combined_feedback uses it.
// Put name in question object. We must ensure it is not empty and it is less than 250 chars.
$question->name = shorten_text(strip_tags($question->questiontext['text']), 200);
$question->name = substr($question->name, 0, 250);
if (!$question->name) {
$id = $this->getpath($questiondata,
array('@', 'id'), '', true);
$question->name = get_string('defaultname', 'qformat_blackboard_six' , $id);
}
$id = $this->getpath($questiondata, array('@', 'id'), '', true);
$question->name = $this->create_default_question_name($question->questiontext['text'],
get_string('defaultname', 'qformat_blackboard_six' , $id));
$question->generalfeedback = '';
$question->generalfeedbackformat = FORMAT_HTML;

7
question/format/blackboard_six/formatqti.php
View File

@ -510,11 +510,8 @@ class qformat_blackboard_six_qti extends qformat_blackboard_six_base {
$question->questiontext = $this->cleaned_text_field($text);
$question->questiontextformat = FORMAT_HTML; // Needed because add_blank_combined_feedback uses it.
$question->name = shorten_text(strip_tags($question->questiontext['text']), 200);
$question->name = substr($question->name, 0, 250);
if (!$question->name) {
$question->name = get_string('defaultname', 'qformat_blackboard_six' , $quest->id);
}
$question->name = $this->create_default_question_name($question->questiontext['text'],
get_string('defaultname', 'qformat_blackboard_six' , $quest->id));
$question->generalfeedback = '';
$question->generalfeedbackformat = FORMAT_HTML;
$question->generalfeedbackfiles = array();

4
question/format/examview/format.php
View File

@ -131,7 +131,7 @@ class qformat_examview extends qformat_based_on_xml {
$question->questiontext = $htmltext;
$question->questiontextformat = FORMAT_HTML;
$question->questiontextfiles = array();
$question->name = shorten_text( $question->questiontext, 250 );
$question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
$question->qtype = 'match';
$question = $this->add_blank_combined_feedback($question);
$question->subquestions = array();
@ -200,7 +200,7 @@ class qformat_examview extends qformat_based_on_xml {
$question->questiontext = $this->cleaninput($htmltext);
$question->questiontextformat = FORMAT_HTML;
$question->questiontextfiles = array();
$question->name = shorten_text( $question->questiontext, 250 );
$question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
switch ($question->qtype) {
case 'multichoice':

8
question/format/gift/format.php
View File

@ -206,7 +206,7 @@ class qformat_gift extends qformat_default {
// name will be assigned after processing question text below
} else {
$questionname = substr($text, 0, $namefinish);
$question->name = trim($this->escapedchar_post($questionname));
$question->name = $this->clean_question_name($this->escapedchar_post($questionname));
$text = trim(substr($text, $namefinish+2)); // Remove name from text
}
} else {
@ -265,13 +265,9 @@ class qformat_gift extends qformat_default {
// set question name if not already set
if ($question->name === false) {
$question->name = $question->questiontext;
$question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
}
// ensure name is not longer than 250 characters
$question->name = shorten_text($question->name, 200);
$question->name = strip_tags(substr($question->name, 0, 250));
// determine QUESTION TYPE
$question->qtype = NULL;

5
question/format/learnwise/format.php
View File

@ -126,10 +126,7 @@ class qformat_learnwise extends qformat_default {
$question = $this->defaultquestion();
$question->qtype = 'multichoice';
$question->name = substr($questiontext, 0, 30);
if (strlen($questiontext) > 30) {
$question->name .= '...';
}
$question->name = $this->create_default_question_name($questiontext, get_string('questionname', 'question'));
$question->questiontext = $questiontext;
$question->single = ($type == 'multichoice') ? 1 : 0;

2
question/format/missingword/format.php
View File

@ -89,7 +89,7 @@ class qformat_missingword extends qformat_default {
/// Save the new question text
$question->questiontext = substr_replace($text, "_____", $answerstart, $answerlength+1);
$question->name = $question->questiontext;
$question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
/// Parse the answers

13
question/format/multianswer/format.php
View File

@ -62,18 +62,7 @@ class qformat_multianswer extends qformat_default {
$question->penalty = 0.3333333;
if (!empty($question)) {
$name = html_to_text(implode(' ', $lines));
$name = preg_replace('/{[^}]*}/', '', $name);
$name = trim($name);
if ($name) {
$question->name = shorten_text($name, 45);
} else {
// We need some name, so use the current time, since that will be
// reasonably unique.
$question->name = userdate(time());
}
$question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
$questions[] = $question;
}

4
question/format/multianswer/tests/multianswerformat_test.php
View File

@ -47,7 +47,9 @@ class qformat_multianswer_test extends question_testcase {
$qs = $importer->readquestions($lines);
$expectedq = (object) array(
'name' => 'Match the following cities with the ...',
'name' => 'Match the following cities with the correct state:
* San Francisco: {#1}
* ...',
'questiontext' => 'Match the following cities with the correct state:
* San Francisco: {#1}
* Tucson: {#2}

13
question/format/webct/format.php
View File

@ -259,11 +259,8 @@ class qformat_webct extends qformat_default {
// Setup default value of missing fields
if (!isset($question->name)) {
$question->name = $question->questiontext;
}
if (strlen($question->name) > 255) {
$question->name = substr($question->name,0,250)."...";
$warnings[] = get_string("questionnametoolong", "qformat_webct", $nQuestionStartLine);
$question->name = $this->create_default_question_name(
$question->questiontext, get_string('questionname', 'question'));
}
if (!isset($question->defaultmark)) {
$question->defaultmark = 1;
@ -466,11 +463,7 @@ class qformat_webct extends qformat_default {
if (preg_match("~^:TITLE:(.*)~i",$line,$webct_options)) {
$name = trim($webct_options[1]);
if (strlen($name) > 255) {
$name = substr($name,0,250)."...";
$warnings[] = get_string("questionnametoolong", "qformat_webct", $nLineCounter);
}
$question->name = $name;
$question->name = $this->clean_question_name($name);
continue;
}

6
question/format/xml/format.php
View File

@ -166,9 +166,9 @@ class qformat_xml extends qformat_default {
$qo = $this->defaultquestion();
// Question name
$qo->name = $this->getpath($question,
$qo->name = $this->clean_question_name($this->getpath($question,
array('#', 'name', 0, '#', 'text', 0, '#'), '', true,
get_string('xmlimportnoname', 'qformat_xml'));
get_string('xmlimportnoname', 'qformat_xml')));
$qo->questiontext = $this->getpath($question,
array('#', 'questiontext', 0, '#', 'text', 0, '#'), '', true);
$qo->questiontextformat = $this->trans_format($this->getpath(
@ -437,7 +437,7 @@ class qformat_xml extends qformat_default {
$qo->course = $this->course;
$qo->generalfeedback = '';
$qo->name = $this->import_text($question['#']['name'][0]['#']['text']);
$qo->name = $this->clean_question_name($this->import_text($question['#']['name'][0]['#']['text']));
$qo->questiontextformat = $questiontext['format'];
$qo->questiontext = $qo->questiontext['text'];
$qo->questiontextfiles = array();

67
question/tests/importexport_test.php
View File

@ -87,4 +87,71 @@ class qformat_default_test extends advanced_testcase {
$path = '<evil>Nasty <virus //> thing<//evil>';
$this->assertEquals(array('Nasty thing'), $format->split_category_path($path));
}
public function test_clean_question_name() {
$format = new testable_qformat();
$name = 'Nice simple name';
$this->assertEquals($name, $format->clean_question_name($name));
$name = 'Question in <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>';
$this->assertEquals($name, $format->clean_question_name($name));
$name = 'Evil <script type="text/javascrip">alert("You have been hacked!");</script>';
$this->assertEquals('Evil alert("You have been hacked!");', $format->clean_question_name($name));
$name = 'This is a very long question name. It goes on and on and on. ' .
'I wonder if it will ever stop. The quetsion name field in the database is only ' .
'two hundred and fifty five characters wide, so if the import file contains a ' .
'name longer than that, the code had better truncate it!';
$this->assertEquals(shorten_text($name, 251), $format->clean_question_name($name));
// The worst case scenario is a whole lot of single charaters in separate multilang tags.
$name = '<span lang="en" class="multilang">A</span>' .
'<span lang="fr" class="multilang">B</span>' .
'<span lang="fr_ca" class="multilang">C</span>' .
'<span lang="en_us" class="multilang">D</span>' .
'<span lang="de" class="multilang">E</span>' .
'<span lang="cz" class="multilang">F</span>' .
'<span lang="it" class="multilang">G</span>' .
'<span lang="es" class="multilang">H</span>' .
'<span lang="pt" class="multilang">I</span>' .
'<span lang="ch" class="multilang">J</span>';
$this->assertEquals(shorten_text($name, 1), $format->clean_question_name($name));
}
public function test_create_default_question_name() {
$format = new testable_qformat();
$text = 'Nice simple name';
$this->assertEquals($text, $format->create_default_question_name($text, 'Default'));
$this->assertEquals('Default', $format->create_default_question_name('', 'Default'));
$text = 'Question in <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>';
$this->assertEquals($text, $format->create_default_question_name($text, 'Default'));
$text = 'Evil <script type="text/javascrip">alert("You have been hacked!");</script>';
$this->assertEquals('Evil alert("You have been hacked!");',
$format->create_default_question_name($text, 'Default'));
$text = 'This is a very long question text. It goes on and on and on. ' .
'I wonder if it will ever stop. The question name field in the database is only ' .
'two hundred and fifty five characters wide, so if the import file contains a ' .
'name longer than that, the code had better truncate it!';
$this->assertEquals(shorten_text($text, 80), $format->create_default_question_name($text, 'Default'));
// The worst case scenario is a whole lot of single charaters in separate multilang tags.
$text = '<span lang="en" class="multilang">A</span>' .
'<span lang="fr" class="multilang">B</span>' .
'<span lang="fr_ca" class="multilang">C</span>' .
'<span lang="en_us" class="multilang">D</span>' .
'<span lang="de" class="multilang">E</span>' .
'<span lang="cz" class="multilang">F</span>' .
'<span lang="it" class="multilang">G</span>' .
'<span lang="es" class="multilang">H</span>' .
'<span lang="pt" class="multilang">I</span>' .
'<span lang="ch" class="multilang">J</span>';
$this->assertEquals(shorten_text($text, 1), $format->create_default_question_name($text, 'Default'));
}
}