Skip to content

Commit

Permalink
[BUGFIX] Fix math expression error verbosity
Browse files Browse the repository at this point in the history
Prevents seeing the “non-numeric value encountered”
error from PHP when a template attempts to do math
on a variable that is not declared, or does not have the
right type. See code comments for exact behavior.

The intention is to match PHP type coercion as close
as possible - among other things by inferring NULL as
value when a variable is most likely not defined.
  • Loading branch information
NamelessCoder committed Jul 16, 2018
1 parent 9726683 commit dc99f18
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 13 deletions.
50 changes: 47 additions & 3 deletions src/Core/Parser/SyntaxTree/Expression/MathExpressionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,28 @@ public static function evaluateExpression(RenderingContextInterface $renderingCo
// any special precedence on the priority of operators. We simply process
// them in order.
$result = array_shift($matches[0]);
$result = static::getTemplateVariableOrValueItself($result, $renderingContext);
$firstPart = static::getTemplateVariableOrValueItself($result, $renderingContext);
if ($firstPart === $result && !is_numeric($firstPart)) {
// Pitfall: the expression part was not numeric and did not resolve to a variable. We null the
// value - although this means the edge case of a variable's value being the same as its name,
// results in the expression part being treated as zero. Which is different from how PHP would
// coerce types in earlier versions, implying that a non-numeric string just counts as "1".
// Here, it counts as zero with the intention of error prevention on undeclared variables.
// Note that the same happens in the loop below.
$firstPart = null;
}
$result = $firstPart;
$operator = null;
$operators = ['*', '^', '-', '+', '/', '%'];
foreach ($matches[0] as $part) {
if (in_array($part, $operators)) {
$operator = $part;
} else {
$part = static::getTemplateVariableOrValueItself($part, $renderingContext);
$result = self::evaluateOperation($result, $operator, $part);
$newPart = static::getTemplateVariableOrValueItself($part, $renderingContext);
if ($newPart === $part && !is_numeric($part)) {
$newPart = null;
}
$result = self::evaluateOperation($result, $operator, $newPart);
}
}
return $result;
Expand All @@ -69,6 +82,19 @@ public static function evaluateExpression(RenderingContextInterface $renderingCo
*/
protected static function evaluateOperation($left, $operator, $right)
{
// Special case: the "+" operator can be used with two arrays which will combine the two arrays. But it is
// only allowable if both sides are in fact arrays and only for this one operator. Please see PHP documentation
// about "union" on https://secure.php.net/manual/en/language.operators.array.php for specific behavior!
if ($operator === '+' && is_array($left) && is_array($right)) {
return $left + $right;
}

// Guard: if left or right side are not numeric values, infer a value for the expression part based on how
// PHP would coerce types in versions that are not strict typed. We do this to avoid fatal PHP errors about
// encountering non-numeric values.
$left = static::coerceNumericValue($left);
$right = static::coerceNumericValue($right);

if ($operator === '%') {
return $left % $right;
} elseif ($operator === '-') {
Expand All @@ -84,4 +110,22 @@ protected static function evaluateOperation($left, $operator, $right)
}
return 0;
}

protected static function coerceNumericValue($value)
{
if (is_object($value) && method_exists($value, '__toString')) {
// Delegate to another coercion call after casting to string
return static::coerceNumericValue((string) $value);
}
if (is_null($value)) {
return 0;
}
if (is_bool($value)) {
return $value ? 1 : 0;
}
if (is_numeric($value)) {
return $value;
}
return 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\Expression\MathExpressionNode;
use TYPO3Fluid\Fluid\Core\Rendering\RenderingContext;
use TYPO3Fluid\Fluid\Core\Variables\StandardVariableProvider;
use TYPO3Fluid\Fluid\Tests\Unit\ViewHelpers\Fixtures\UserWithToString;
use TYPO3Fluid\Fluid\Tests\UnitTestCase;
use TYPO3Fluid\Fluid\View\TemplateView;

Expand Down Expand Up @@ -38,17 +39,25 @@ public function testEvaluateExpression($expression, array $variables, $expected)
*/
public function getEvaluateExpressionTestValues()
{
$objectWithToStringReturningNumericValue = new UserWithToString('123');
$objectWithToStringReturningNonNumericValue = new UserWithToString('text');
return [
['1 gabbagabbahey 1', [], 0],
['1 + 1', [], 2],
['2 - 1', [], 1],
['2 % 4', [], 2],
['2 * 4', [], 8],
['4 / 2', [], 2],
['4 ^ 2', [], 16],
['a + 1', ['a' => 1], 2],
['1 + b', ['b' => 1], 2],
['a + b', ['a' => 1, 'b' => 1], 2],
'invalid operator returns zero' => ['1 gabbagabbahey 1', [], 0],
'1 + 1 = 2' => ['1 + 1', [], 2],
'2 - 1 = 1' => ['2 - 1', [], 1],
'2 % 4 = 2' => ['2 % 4', [], 2],
'2 * 4 = 8' => ['2 * 4', [], 8],
'4 / 2 = 2' => ['4 / 2', [], 2],
'4 ^ 2 = 16' => ['4 ^ 2', [], 16],
'$a(1) + 1 = 2' => ['a + 1', ['a' => 1], 2],
'1 + $b(1) = 2' => ['1 + b', ['b' => 1], 2],
'$a(1) + $b(1) = 2' => ['a + b', ['a' => 1, 'b' => 1], 2],
'$a(array) + $b(array) = $union' => ['a + b', ['a' => ['foo' => 'foo'], 'b' => ['bar' => 'bar']], ['foo' => 'foo', 'bar' => 'bar']],
'$a(object-numeric) + 2 = 125' => ['a + 2', ['a' => $objectWithToStringReturningNumericValue], 125],
'$a(object-not-numeric) + 2 = 2' => ['a + 2', ['a' => $objectWithToStringReturningNonNumericValue], 2],
'$notdefined(null) + 1 = 1' => ['notdefined + 1', [], 1],
'$a(true) + 1 = 2' => ['a + 1', ['a' => true], 2],
'$a(false) + 1 = 1' => ['a + 1', ['a' => false], 1],
];
}
}

0 comments on commit dc99f18

Please sign in to comment.