diff --git a/question/type/calculated/db/upgradelib.php b/question/type/calculated/db/upgradelib.php index 22d87107f47..09803be12d7 100644 --- a/question/type/calculated/db/upgradelib.php +++ b/question/type/calculated/db/upgradelib.php @@ -248,11 +248,18 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update * @return float the computed result. */ protected function calculate_raw($expression) { - // This validation trick from http://php.net/manual/en/function.eval.php. - if (!@eval('return true; $result = ' . $expression . ';')) { - return '[Invalid expression ' . $expression . ']'; + try { + // In older PHP versions this this is a way to validate code passed to eval. + // The trick came from http://php.net/manual/en/function.eval.php. + if (@eval('return true; $result = ' . $expression . ';')) { + return eval('return ' . $expression . ';'); + } + } catch (Throwable $e) { + // PHP7 and later now throws ParseException and friends from eval(), + // which is much better. } - return eval('return ' . $expression . ';'); + // In either case of an invalid $expression, we end here. + return '[Invalid expression ' . $expression . ']'; } /** diff --git a/question/type/calculated/question.php b/question/type/calculated/question.php index af9c819e6c6..99cd17120f7 100644 --- a/question/type/calculated/question.php +++ b/question/type/calculated/question.php @@ -433,11 +433,18 @@ class qtype_calculated_variable_substituter { * @return float the computed result. */ protected function calculate_raw($expression) { - // This validation trick from http://php.net/manual/en/function.eval.php . - if (!@eval('return true; $result = ' . $expression . ';')) { - throw new moodle_exception('illegalformulasyntax', 'qtype_calculated', '', $expression); + try { + // In older PHP versions this this is a way to validate code passed to eval. + // The trick came from http://php.net/manual/en/function.eval.php. + if (@eval('return true; $result = ' . $expression . ';')) { + return eval('return ' . $expression . ';'); + } + } catch (Throwable $e) { + // PHP7 and later now throws ParseException and friends from eval(), + // which is much better. } - return eval('return ' . $expression . ';'); + // In either case of an invalid $expression, we end here. + throw new moodle_exception('illegalformulasyntax', 'qtype_calculated', '', $expression); } /** diff --git a/question/type/calculated/tests/variablesubstituter_test.php b/question/type/calculated/tests/variablesubstituter_test.php index f9be863b591..b67030652fe 100644 --- a/question/type/calculated/tests/variablesubstituter_test.php +++ b/question/type/calculated/tests/variablesubstituter_test.php @@ -65,6 +65,17 @@ class qtype_calculated_variable_substituter_test extends advanced_testcase { $vs->calculate('{a}{b}'); // Have to make sure this does not just evaluate to 12. } + public function test_division_by_zero_expression() { + + if (intval(PHP_VERSION) < 7) { + $this->markTestSkipped('Division by zero triggers a PHP warning before PHP 7.'); + } + + $this->setExpectedException('moodle_exception'); + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 0), '.'); + $vs->calculate('{a} / {b}'); + } + public function test_replace_expressions_in_text_simple_var() { $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); $this->assertEquals('1 + 2', $vs->replace_expressions_in_text('{a} + {b}')); diff --git a/question/type/calculatedmulti/db/upgradelib.php b/question/type/calculatedmulti/db/upgradelib.php index 53bf09dd44a..ed4cb25bee1 100644 --- a/question/type/calculatedmulti/db/upgradelib.php +++ b/question/type/calculatedmulti/db/upgradelib.php @@ -272,11 +272,18 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u * @return float the computed result. */ protected function calculate_raw($expression) { - // This validation trick from http://php.net/manual/en/function.eval.php. - if (!@eval('return true; $result = ' . $expression . ';')) { - return '[Invalid expression ' . $expression . ']'; + try { + // In older PHP versions this this is a way to validate code passed to eval. + // The trick came from http://php.net/manual/en/function.eval.php. + if (@eval('return true; $result = ' . $expression . ';')) { + return eval('return ' . $expression . ';'); + } + } catch (Throwable $e) { + // PHP7 and later now throws ParseException and friends from eval(), + // which is much better. } - return eval('return ' . $expression . ';'); + // In either case of an invalid $expression, we end here. + return '[Invalid expression ' . $expression . ']'; } /**