L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-46148 qtype_calculated: fix formula validation.

Browse Source
This commit is contained in:
Tim Hunt
2014-07-07 14:34:19 +01:00
committed by Dan Poltawski
parent 9b60a086cf
commit a2bf5412a7
6 changed files with 246 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/type/calculated/edit_calculated_form.php
+25 -22
View File
@@ -189,36 +189,39 @@ class qtype_calculated_edit_form extends qtype_numerical_edit_form {
return 'calculated';
}
/**
* Validate the equations in the some question content.
* @param array $errors where errors are being accumulated.
* @param string $field the field being validated.
* @param string $text the content of that field.
* @return array the updated $errors array.
*/
protected function validate_text($errors, $field, $text) {
$problems = qtype_calculated_find_formula_errors_in_text($text);
if ($problems) {
$errors[$field] = $problems;
}
return $errors;
}
public function validation($data, $files) {
// verifying for errors in {=...} in question text;
$qtext = "";
$qtextremaining = $data['questiontext']['text'];
$possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']);
foreach ($possibledatasets as $name => $value) {
$qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining);
}
while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) {
$qtextsplits = explode($regs1[0], $qtextremaining, 2);
$qtext = $qtext.$qtextsplits[0];
$qtextremaining = $qtextsplits[1];
if (!empty($regs1[1]) && $formulaerrors =
qtype_calculated_find_formula_errors($regs1[1])) {
if (!isset($errors['questiontext'])) {
$errors['questiontext'] = $formulaerrors.':'.$regs1[1];
} else {
$errors['questiontext'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
}
}
}
$errors = parent::validation($data, $files);
// Verifying for errors in {=...} in question text.
$errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']);
$errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']);
// Check that the answers use datasets.
$answers = $data['answer'];
$mandatorydatasets = array();
foreach ($answers as $key => $answer) {
$problems = qtype_calculated_find_formula_errors($answer);
if ($problems) {
$errors['answer['.$key.']'] = $problems;
}
$mandatorydatasets += $this->qtypeobj->find_dataset_names($answer);
$errors = $this->validate_text($errors, 'feedback[' . $key . ']',
$data['feedback'][$key]['text']);
}
if (empty($mandatorydatasets)) {
foreach ($answers as $key => $answer) {
question/type/calculated/question.php
+4 -100
View File
@@ -259,6 +259,7 @@ class qtype_calculated_dataset_loader {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_calculated_variable_substituter {
/** @var array variable name => value */
protected $values;
@@ -388,108 +389,11 @@ class qtype_calculated_variable_substituter {
* @return string the text with values substituted.
*/
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('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
$vs = $this; // Can't use $this in a PHP closure.
$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);
return $this->substitute_values_pretty($text);
}
/**
* Return an array describing any problems there are with an expression.
* Returns false if the expression is fine.
* @param string $formula an expression.
* @return array|false list of problems, or false if the exression is OK.
*/
public function get_formula_errors($formula) {
// 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 empty space and lowercase it
$formula = strtolower(str_replace(' ', '', $formula));
$safeoperatorchar = '-+/*%>:^\~<?=&|!'; /* */
$operatorornumber = "[$safeoperatorchar.0-9eE]";
while (preg_match("~(^|[$safeoperatorchar,(])([a-z0-9_]*)" .
"\\(($operatorornumber+(,$operatorornumber+((,$operatorornumber+)+)?)?)?\\)~",
$formula, $regs)) {
switch ($regs[2]) {
// Simple parenthesis
case '':
if ((isset($regs[4]) && $regs[4]) || strlen($regs[3]) == 0) {
return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
}
break;
// Zero argument functions
case 'pi':
if ($regs[3]) {
return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
}
break;
// Single argument functions (the most common case)
case 'abs': case 'acos': case 'acosh': case 'asin': case 'asinh':
case 'atan': case 'atanh': case 'bindec': case 'ceil': case 'cos':
case 'cosh': case 'decbin': case 'decoct': case 'deg2rad':
case 'exp': case 'expm1': case 'floor': case 'is_finite':
case 'is_infinite': case 'is_nan': case 'log10': case 'log1p':
case 'octdec': case 'rad2deg': case 'sin': case 'sinh': case 'sqrt':
case 'tan': case 'tanh':
if (!empty($regs[4]) || empty($regs[3])) {
return get_string('functiontakesonearg', 'qtype_calculated', $regs[2]);
}
break;
// Functions that take one or two arguments
case 'log': case 'round':
if (!empty($regs[5]) || empty($regs[3])) {
return get_string('functiontakesoneortwoargs', 'qtype_calculated',
$regs[2]);
}
break;
// Functions that must have two arguments
case 'atan2': case 'fmod': case 'pow':
if (!empty($regs[5]) || empty($regs[4])) {
return get_string('functiontakestwoargs', 'qtype_calculated', $regs[2]);
}
break;
// Functions that take two or more arguments
case 'min': case 'max':
if (empty($regs[4])) {
return get_string('functiontakesatleasttwo', 'qtype_calculated', $regs[2]);
}
break;
default:
return get_string('unsupportedformulafunction', 'qtype_calculated', $regs[2]);
}
// Exchange the function call with '1' and then chack for
// another function call...
if ($regs[1]) {
// The function call is proceeded by an operator
$formula = str_replace($regs[0], $regs[1] . '1', $formula);
} else {
// The function call starts the formula
$formula = preg_replace("~^$regs[2]\\([^)]*\\)~", '1', $formula);
}
}
if (preg_match("~[^$safeoperatorchar.0-9eE]+~", $formula, $regs)) {
return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
} else {
// Formula just might be valid
return false;
}
}
}
}
question/type/calculated/questiontype.php
+79 -3
View File
@@ -37,6 +37,9 @@ require_once($CFG->dirroot . '/question/type/numerical/question.php');
* @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 MAX_DATASET_ITEMS = 100;
public $wizardpagesnumber = 3;
@@ -481,6 +484,36 @@ class qtype_calculated extends question_type {
$mform->display();
}
/**
* Verify that the equations in part of the question are OK.
* We throw an exception here because this should have already been validated
* by the form. This is just a last line of defence to prevent a question
* being stored in the database if it has bad formulas. This saves us from,
* for example, malicious imports.
* @param string $text containing equations.
*/
protected function validate_text($text) {
$error = qtype_calculated_find_formula_errors_in_text($text);
if ($error) {
throw new coding_exception($error);
}
}
/**
* Verify that an answer is OK.
* We throw an exception here because this should have already been validated
* by the form. This is just a last line of defence to prevent a question
* being stored in the database if it has bad formulas. This saves us from,
* for example, malicious imports.
* @param string $text containing equations.
*/
protected function validate_answer($answer) {
$error = qtype_calculated_find_formula_errors($answer);
if ($error) {
throw new coding_exception($error);
}
}
/**
* This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
* so that they can be saved
@@ -498,7 +531,7 @@ class qtype_calculated extends question_type {
// are retrieved
$possibledatasets = $this->find_dataset_names($form->questiontext);
$mandatorydatasets = array();
foreach ($form->answers as $answer) {
foreach ($form->answers as $key => $answer) {
$mandatorydatasets += $this->find_dataset_names($answer);
}
// if there are identical datasetdefs already saved in the original question.
@@ -587,8 +620,15 @@ class qtype_calculated extends question_type {
*/
public function save_question($question, $form) {
global $DB;
if (isset($form->correctfeedback)) {
$this->validate_text($form->correctfeedback['text']);
$this->validate_text($form->partiallycorrectfeedback['text']);
$this->validate_text($form->incorrectfeedback['text']);
}
if ($this->wizardpagesnumber() == 1 || $question->qtype == 'calculatedsimple') {
$question = parent::save_question($question, $form);
$question = parent::save_question($question, $form);
return $question;
}
@@ -607,6 +647,14 @@ class qtype_calculated extends question_type {
case '' :
case 'question': // coming from the first page, creating the second
if (empty($form->id)) { // for a new question $form->id is empty
// Make it impossible to save bad formulas anywhere.
$this->validate_text($form->questiontext['text']);
$this->validate_text($form->generalfeedback['text']);
foreach ($form->answer as $key => $answer) {
$this->validate_answer($answer);
$this->validate_text($form->feedback[$key]['text']);
}
$question = parent::save_question($question, $form);
//prepare the datasets using default $questionfromid
$this->preparedatasets($form);
@@ -1973,6 +2021,11 @@ function qtype_calculated_calculate_answer($formula, $individualdata,
}
/**
* Validate a forumula.
* @param string $formula the formula to validate.
* @return string|boolean false if there are no problems. Otherwise a string error message.
*/
function qtype_calculated_find_formula_errors($formula) {
// Validates the formula submitted from the question edit page.
// Returns false if everything is alright.
@@ -2001,7 +2054,7 @@ function qtype_calculated_find_formula_errors($formula) {
// Zero argument functions
case 'pi':
if ($regs[3]) {
if (array_key_exists(3, $regs)) {
return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
}
break;
@@ -2062,3 +2115,26 @@ function qtype_calculated_find_formula_errors($formula) {
return false;
}
}
/**
* Validate all the forumulas in a bit of text.
* @param string $text the text in which to validate the formulas.
* @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);
$errors = array();
foreach ($matches[1] as $match) {
$error = qtype_calculated_find_formula_errors($match);
if ($error) {
$errors[] = $error;
}
}
if ($errors) {
return implode(' ', $errors);
}
return false;
}
question/type/calculated/tests/formula_validation_test.php
+105
View File
@@ -0,0 +1,105 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Unit tests for formula validation code.
*
* @package qtype_calculated
* @copyright 2014 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/question/type/calculated/questiontype.php');
/**
* Unit tests for formula validation code.
*
* @copyright 2014 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_calculated_formula_validation_testcase extends basic_testcase {
protected function assert_nonempty_string($actual) {
$this->assertInternalType('string', $actual);
$this->assertNotEquals('', $actual);
}
public function test_simple_equations_ok() {
$this->assertFalse(qtype_calculated_find_formula_errors(1));
$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}'));
}
public function test_safe_functions_ok() {
$this->assertFalse(qtype_calculated_find_formula_errors('abs(-1)'));
$this->assertFalse(qtype_calculated_find_formula_errors('tan(pi())'));
$this->assertFalse(qtype_calculated_find_formula_errors('log(10)'));
$this->assertFalse(qtype_calculated_find_formula_errors('log(64, 2)'));
$this->assertFalse(qtype_calculated_find_formula_errors('atan2(1.0, 1.0)'));
$this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0)'));
$this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2.0)'));
$this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2, 3)'));
}
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)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('base64_decode(1)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('unserialize(1)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('cos(tan(1) + abs(cos(eval)) * pi())'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval (CONSTANTREADASSTRING)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors("eval \t ()"));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('"eval"()'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('?><?php()'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('?><?php+1'));
}
public function test_functions_with_wrong_num_args_caught() {
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('abs(-1, 1)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('abs()'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('pi(1)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('log()'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('log(64, 2, 3)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0, 1.0, 2.0)'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors('max(1.0)'));
}
public function test_validation_of_formulas_in_text_ok() {
$this->assertFalse(qtype_calculated_find_formula_errors_in_text(
'<p>Look no equations.</p>'));
$this->assertFalse(qtype_calculated_find_formula_errors_in_text(
'<p>Simple variable: {x}.</p>'));
$this->assertFalse(qtype_calculated_find_formula_errors_in_text(
'<p>This is an equation: {=1+1}, as is this: {={x}+{y}}.</p>' .
'<p>Here is a more complex one: {=sin(2*pi()*{theta})}.</p>'));
}
public function test_validation_of_formulas_in_text_bad_function() {
$this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
'<p>This is an equation: {=eval(1)}.</p>'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
'<p>Good: {=1+1}, bad: {=eval(1)}, good: {={x}+{y}}.</p>'));
$this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
'<p>Bad: {=eval(1)}, bad: {=system(1)}.</p>'));
}
}
question/type/calculatedmulti/edit_calculatedmulti_form.php
+26 -40
View File
@@ -218,30 +218,31 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
return $question;
}
/**
* Validate the equations in the some question content.
* @param array $errors where errors are being accumulated.
* @param string $field the field being validated.
* @param string $text the content of that field.
* @return array the updated $errors array.
*/
protected function validate_text($errors, $field, $text) {
$problems = qtype_calculated_find_formula_errors_in_text($text);
if ($problems) {
$errors[$field] = $problems;
}
return $errors;
}
public function validation($data, $files) {
$errors = parent::validation($data, $files);
//verifying for errors in {=...} in question text;
$qtext = '';
$qtextremaining = $data['questiontext']['text'];
$possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']);
foreach ($possibledatasets as $name => $value) {
$qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining);
}
$errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']);
$errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']);
$errors = $this->validate_text($errors, 'correctfeedback', $data['correctfeedback']['text']);
$errors = $this->validate_text($errors, 'partiallycorrectfeedback', $data['partiallycorrectfeedback']['text']);
$errors = $this->validate_text($errors, 'incorrectfeedback', $data['incorrectfeedback']['text']);
while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) {
$qtextsplits = explode($regs1[0], $qtextremaining, 2);
$qtext = $qtext.$qtextsplits[0];
$qtextremaining = $qtextsplits[1];
if (!empty($regs1[1]) && $formulaerrors =
qtype_calculated_find_formula_errors($regs1[1])) {
if (!isset($errors['questiontext'])) {
$errors['questiontext'] = $formulaerrors.':'.$regs1[1];
} else {
$errors['questiontext'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
}
}
}
$answers = $data['answer'];
$answercount = 0;
$maxgrade = false;
@@ -256,6 +257,7 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
get_string('atleastonewildcard', 'qtype_calculated');
}
}
$totalfraction = 0;
$maxfraction = -1;
foreach ($answers as $key => $answer) {
@@ -268,28 +270,12 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
$errors['fraction['.$key.']'] = get_string('errgradesetanswerblank', 'qtype_multichoice');
}
if ($trimmedanswer != '' || $answercount == 0) {
//verifying for errors in {=...} in answer text;
$qanswer = '';
$qanswerremaining = $trimmedanswer;
$possibledatasets = $this->qtypeobj->find_dataset_names($trimmedanswer);
foreach ($possibledatasets as $name => $value) {
$qanswerremaining = str_replace('{'.$name.'}', '1', $qanswerremaining);
}
while (preg_match('~\{=([^[:space:]}]*)}~', $qanswerremaining, $regs1)) {
$qanswersplits = explode($regs1[0], $qanswerremaining, 2);
$qanswer = $qanswer . $qanswersplits[0];
$qanswerremaining = $qanswersplits[1];
if (!empty($regs1[1]) && $formulaerrors =
qtype_calculated_find_formula_errors($regs1[1])) {
if (!isset($errors['answer['.$key.']'])) {
$errors['answer['.$key.']'] = $formulaerrors.':'.$regs1[1];
} else {
$errors['answer['.$key.']'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
}
}
}
// Verifying for errors in {=...} in answer text.
$errors = $this->validate_text($errors, 'answeroptions[' . $key . ']', $answer);
$errors = $this->validate_text($errors, 'feedback[' . $key . ']',
$data['feedback'][$key]['text']);
}
if ($trimmedanswer != '') {
if ('2' == $data['correctanswerformat'][$key] &&
'0' == $data['correctanswerlength'][$key]) {
question/type/calculatedmulti/questiontype.php
+7
View File
@@ -156,6 +156,13 @@ class qtype_calculatedmulti extends qtype_calculated {
return true;
}
protected function validate_answer($answer) {
$error = qtype_calculated_find_formula_errors_in_text($answer);
if ($error) {
throw new coding_exception($error);
}
}
protected function make_question_instance($questiondata) {
question_bank::load_question_definition_classes($this->name());
if ($questiondata->options->single) {