Permalink
Browse files

MDL-62275 qtype_calc*: improve validation of formulae

Many thanks to Marina Glancy for helping with this.
  • Loading branch information...
timhunt authored and stronk7 committed Apr 30, 2018
1 parent a7a5d10 commit 75aa674b997de5e924c9921c3c8c649098e35552
@@ -313,7 +313,7 @@ protected function definition() {
// Decode equations in question text.
$qtext = $this->qtypeobj->substitute_variables(
$this->question->questiontext, $data);
$textequations = $this->qtypeobj->find_math_equations($qtext);
$textequations = $this->qtypeobj->find_formulas($qtext);
if ($textequations != '' && count($textequations) > 0 ) {
$mform->addElement('static', "divider1[{$j}]", '',
'Formulas {=..} in question text');
@@ -58,10 +58,8 @@ public function __construct($submiturl, $question, $category, $contexts,
if (isset($this->question->id)) {
// Remove prefix #{..}# if exists.
$this->initialname = $question->name;
$regs= array();
if (preg_match('~#\{([^[:space:]]*)#~', $question->name , $regs)) {
$question->name = str_replace($regs[0], '', $question->name);
};
$question->name = question_bank::get_qtype($this->qtype())
->clean_technical_prefix_from_question_name($question->name);
}
}
parent::__construct($submiturl, $question, $category, $contexts, $formeditable);
@@ -28,6 +28,7 @@
require_once($CFG->dirroot . '/question/type/questionbase.php');
require_once($CFG->dirroot . '/question/type/numerical/question.php');
require_once($CFG->dirroot . '/question/type/calculated/questiontype.php');
/**
* Represents a calculated question.
@@ -38,8 +38,20 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_calculated extends question_type {
/** Regular expression that finds the formulas in content. */
const FORMULAS_IN_TEXT_REGEX = '~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)\}~';
/**
* @const string a placeholder is a letter, followed by almost any characters. (This should probably be restricted more.)
*/
const PLACEHOLDER_REGEX_PART = '[[:alpha:]][^>} <`{"\']*';
/**
* @const string REGEXP for a placeholder, wrapped in its {...} delimiters, with capturing brackets around the name.
*/
const PLACEHODLER_REGEX = '~\{(' . self::PLACEHOLDER_REGEX_PART . ')\}~';
/**
* @const string Regular expression that finds the formulas in content, with capturing brackets to get the forumlas.
*/
const FORMULAS_IN_TEXT_REGEX = '~\{=([^{}]*(?:\{' . self::PLACEHOLDER_REGEX_PART . '\}[^{}]*)*)\}~';
const MAX_DATASET_ITEMS = 100;
@@ -486,6 +498,15 @@ protected function validate_question_data($question) {
}
}
/**
* Remove prefix #{..}# if exists.
* @param $name a question name,
* @return string the cleaned up question name.
*/
public function clean_technical_prefix_from_question_name($name) {
return preg_replace('~#\{([^[:space:]]*)#~', '', $name);
}
/**
* This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
* so that they can be saved
@@ -553,11 +574,7 @@ public function addnamecategory(&$question) {
AND a.category != 0
AND b.question = ?
ORDER BY a.name ", array($question->id));
$questionname = $question->name;
$regs= array();
if (preg_match('~#\{([^[:space:]]*)#~', $questionname , $regs)) {
$questionname = str_replace($regs[0], '', $questionname);
};
$questionname = $this->clean_technical_prefix_from_question_name($question->name);
if (!empty($categorydatasetdefs)) {
// There is at least one with the same name.
@@ -1530,15 +1547,28 @@ public function dataset_options_from_database($form, $name, $prefix = '',
: '');
}
/**
* Find the names of all datasets mentioned in a piece of question content like the question text.
* @param $text the text to analyse.
* @return array with dataset name for both key and value.
*/
public function find_dataset_names($text) {
// Returns the possible dataset names found in the text as an array.
// The array has the dataset name for both key and value.
$datasetnames = array();
while (preg_match('~\\{([[:alpha:]][^>} <{"\']*)\\}~', $text, $regs)) {
$datasetnames[$regs[1]] = $regs[1];
$text = str_replace($regs[0], '', $text);
}
return $datasetnames;
preg_match_all(self::PLACEHODLER_REGEX, $text, $matches);
return array_combine($matches[1], $matches[1]);
}
/**
* Find all the formulas in a bit of text.
*
* For example, called with "What is {a} plus {b}? (Hint, it is not {={a}*{b}}.)" this
* returns ['{a}*{b}'].
*
* @param $text text to analyse.
* @return array where they keys an values are the formulas.
*/
public function find_formulas($text) {
preg_match_all(self::FORMULAS_IN_TEXT_REGEX, $text, $matches);
return array_combine($matches[1], $matches[1]);
}
/**
@@ -1756,17 +1786,6 @@ public function print_dataset_definitions_category_shared($question, $datasetdef
return $text;
}
public function find_math_equations($text) {
// Returns the possible dataset names found in the text as an array.
// The array has the dataset name for both key and value.
$equations = array();
while (preg_match('~\{=([^[:space:]}]*)}~', $text, $regs)) {
$equations[] = $regs[1];
$text = str_replace($regs[0], '', $text);
}
return $equations;
}
public function get_virtual_qtype() {
return question_bank::get_qtype('numerical');
}
@@ -1921,13 +1940,17 @@ function qtype_calculated_calculate_answer($formula, $individualdata,
* @return string|boolean false if there are no problems. Otherwise a string error message.
*/
function qtype_calculated_find_formula_errors($formula) {
foreach (['//', '/*', '#', '<?', '?>'] as $commentstart) {
if (strpos($formula, $commentstart) !== false) {
return get_string('illegalformulasyntax', 'qtype_calculated', $commentstart);
}
}
// Validates the formula submitted from the question edit page.
// Returns false if everything is alright
// otherwise it constructs an error message.
// Strip away dataset names.
while (preg_match('~\\{[[:alpha:]][^>} <{"\']*\\}~', $formula, $regs)) {
$formula = str_replace($regs[0], '1', $formula);
}
// Strip away dataset names. Use 1.0 to catch illegal concatenation like {a}{b}.
$formula = preg_replace(qtype_calculated::PLACEHODLER_REGEX, '1.0', $formula);
// Strip away empty space and lowercase it.
$formula = strtolower(str_replace(' ', '', $formula));
@@ -1991,14 +2014,14 @@ function qtype_calculated_find_formula_errors($formula) {
return get_string('unsupportedformulafunction', 'qtype_calculated', $regs[2]);
}
// Exchange the function call with '1' and then check for
// Exchange the function call with '1.0' and then check for
// another function call...
if ($regs[1]) {
// The function call is proceeded by an operator.
$formula = str_replace($regs[0], $regs[1] . '1', $formula);
$formula = str_replace($regs[0], $regs[1] . '1.0', $formula);
} else {
// The function call starts the formula.
$formula = preg_replace("~^{$regs[2]}\\([^)]*\\)~", '1', $formula);
$formula = preg_replace('~^' . preg_quote($regs[2], '~') . '\([^)]*\)~', '1.0', $formula);
}
}
@@ -2016,10 +2039,10 @@ function qtype_calculated_find_formula_errors($formula) {
* @return string|boolean false if there are no problems. Otherwise a string error message.
*/
function qtype_calculated_find_formula_errors_in_text($text) {
preg_match_all(qtype_calculated::FORMULAS_IN_TEXT_REGEX, $text, $matches);
$formulas = question_bank::get_qtype('calculated')->find_formulas($text);
$errors = array();
foreach ($matches[1] as $match) {
foreach ($formulas as $match) {
$error = qtype_calculated_find_formula_errors($match);
if ($error) {
$errors[] = $error;
@@ -46,6 +46,14 @@ public function test_simple_equations_ok() {
$this->assertFalse(qtype_calculated_find_formula_errors('1 + 1'));
$this->assertFalse(qtype_calculated_find_formula_errors('{x} + {y}'));
$this->assertFalse(qtype_calculated_find_formula_errors('{x}*{y}'));
$this->assertFalse(qtype_calculated_find_formula_errors('{x}*({y}+1)'));
}
public function test_simple_equations_errors() {
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('{a{b}}'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('{a{b}}'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('{a}({b})'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('2({b})'));
}
public function test_safe_functions_ok() {
@@ -59,6 +67,15 @@ public function test_safe_functions_ok() {
$this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2, 3)'));
}
public function test_php_comments_blocked() {
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('# No need for this.'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('/* Also blocked. */'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('1 + 1 /* Blocked too. */'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('// As is this.'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('1/*2'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('/*{a*///{x}}'));
}
public function test_dangerous_functions_blocked() {
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval(1)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('system(1)'));
@@ -127,4 +127,37 @@ public function test_get_short_question_name() {
$this->assertEquals("Lorem ipsum dolor...", $this->qtype->get_short_question_name($longquestionname, 20));
$this->assertEquals("Lorem ipsum", $this->qtype->get_short_question_name($shortquestionname, 20));
}
public function test_placehodler_regex() {
preg_match_all(qtype_calculated::PLACEHODLER_REGEX, '= {={a} + {b}}', $matches);
$this->assertEquals([['{a}', '{b}'], ['a', 'b']], $matches);
}
public function test_formulas_in_text_regex() {
preg_match_all(qtype_calculated::FORMULAS_IN_TEXT_REGEX, '= {={a} + {b}}', $matches);
$this->assertEquals([['{={a} + {b}}'], ['{a} + {b}']], $matches);
}
public function test_find_dataset_names() {
$this->assertEquals([], $this->qtype->find_dataset_names('Frog.'));
$this->assertEquals(['a' => 'a', 'b' => 'b'],
$this->qtype->find_dataset_names('= {={a} + {b}}'));
$this->assertEquals(['a' => 'a', 'b' => 'b'],
$this->qtype->find_dataset_names('What is {a} plus {b}? (Hint, it is not {={a}*{b}}.)'));
$this->assertEquals(['a' => 'a', 'b' => 'b', 'c' => 'c'],
$this->qtype->find_dataset_names('
<p>If called with $a = {a} and $b = {b}, what does this PHP function return?</p>
<pre>
/**
* What does this do?
*/
function mystery($a, $b) {
return {c}*$a + $b;
}
</pre>
'));
}
}
@@ -317,7 +317,7 @@ protected function substitute_values_pretty($text) {
*/
public function replace_expressions_in_text($text, $length = null, $format = null) {
$vs = $this; // Can't see to use $this in a PHP closure.
$text = preg_replace_callback('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
$text = preg_replace_callback(qtype_calculated::FORMULAS_IN_TEXT_REGEX,
function ($matches) use ($vs, $format, $length) {
return $vs->format_float($vs->calculate($matches[1]), $length, $format);
}, $text);
@@ -54,10 +54,8 @@ public function __construct($submiturl, $question, $category,
if (isset($this->question->id)) {
// Remove prefix #{..}# if exists.
$this->initialname = $question->name;
$regs= array();
if (preg_match('~#\{([^[:space:]]*)#~', $question->name , $regs)) {
$question->name = str_replace($regs[0], '', $question->name);
};
$question->name = question_bank::get_qtype('calculated')
->clean_technical_prefix_from_question_name($question->name);
}
}
parent::__construct($submiturl, $question, $category, $contexts, $formeditable);
@@ -231,36 +231,27 @@ public function comment_header($question) {
public function comment_on_datasetitems($qtypeobj, $questionid, $questiontext,
$answers, $data, $number) {
global $DB;
$comment = new stdClass();
$comment->stranswers = array();
$comment->outsidelimit = false;
$comment->answers = array();
$answers = fullclone($answers);
$errors = '';
$delimiter = ': ';
foreach ($answers as $key => $answer) {
$anssubstituted = $this->substitute_variables($answer->answer, $data);
// Evaluate the equations i.e {=5+4).
$anstext = '';
$anstextremaining = $anssubstituted;
while (preg_match('~\{=([^[:space:]}]*)}~', $anstextremaining, $regs1)) {
$anstextsplits = explode($regs1[0], $anstextremaining, 2);
$anstext =$anstext.$anstextsplits[0];
$anstextremaining = $anstextsplits[1];
if (empty($regs1[1])) {
$str = '';
$anssubstituted = $this->substitute_variables($answer->answer, $data);
$formulas = $this->find_formulas($anssubstituted);
$replaces = [];
foreach ($formulas as $formula) {
if ($formulaerrors = qtype_calculated_find_formula_errors($formula)) {
$str = $formulaerrors;
} else {
if ($formulaerrors = qtype_calculated_find_formula_errors($regs1[1])) {
$str=$formulaerrors;
} else {
eval('$str = '.$regs1[1].';');
}
eval('$str = ' . $formula . ';');
}
$anstext = $anstext.$str;
$replaces[$formula] = $str;
}
$anstext .= $anstextremaining;
$anstext = str_replace(arary_keys($replaces), arary_values($replaces), $anssubstituted);
$comment->stranswers[$key] = $anssubstituted.'<br/>'.$anstext;
}
return fullclone($comment);

0 comments on commit 75aa674

Please sign in to comment.