Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

MDL-34841 error importing questions with long names.

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.
  • Loading branch information...
commit b252a84543fda72636d6aa270aab635dacd9e431 1 parent 698ecaf
@timhunt timhunt authored
View
31 mod/lesson/format.php
@@ -541,6 +541,37 @@ function readquestion($lines) {
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
View
31 question/format.php
@@ -661,6 +661,37 @@ protected function defaultquestion() {
}
/**
+ * 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
* @return object question
View
2  question/format/aiken/format.php
@@ -95,7 +95,7 @@ public function readquestions($lines) {
} 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 = '';
View
10 question/format/blackboard/format.php
@@ -138,13 +138,9 @@ public function process_common($questiondata) {
$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;
View
10 question/format/blackboard_six/formatpool.php
@@ -92,13 +92,9 @@ public function process_common($questiondata) {
$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;
View
7 question/format/blackboard_six/formatqti.php
@@ -510,11 +510,8 @@ public function process_common($quest) {
$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();
View
4 question/format/examview/format.php
@@ -131,7 +131,7 @@ protected function process_matches(&$questions) {
$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 @@ public function readquestion($qrec) {
$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:
View
8 question/format/gift/format.php
@@ -206,7 +206,7 @@ public function readquestion($lines) {
// 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 {
@@ -251,13 +251,9 @@ public function readquestion($lines) {
// 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;
View
5 question/format/learnwise/format.php
@@ -126,10 +126,7 @@ function readquestion($lines) {
$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;
View
2  question/format/missingword/format.php
@@ -89,7 +89,7 @@ function readquestion($lines) {
/// 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
View
13 question/format/multianswer/format.php
@@ -62,18 +62,7 @@ public function readquestions($lines) {
$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;
}
View
13 question/format/webct/format.php
@@ -259,11 +259,8 @@ protected function readquestions($lines) {
// 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 @@ protected function readquestions($lines) {
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;
}
View
6 question/format/xml/format.php
@@ -166,9 +166,9 @@ public function import_headers($question) {
$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 @@ public function import_multianswer($question) {
$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();
Please sign in to comment.
Something went wrong with that request. Please try again.