Fluid icon indicating copy to clipboard operation
Fluid copied to clipboard

Evaluation of cached boolean fluid expressions with empty strings

Open doerler opened this issue 7 years ago • 3 comments
trafficstars

The content of

<f:else if="{settings.templateLayout} == 10">
</f:else>

is rendered if {settings.templateLayout} is an empty string and the Fluid template file is cached.

Cached part:

$arguments60['__elseifClosures'][] = function() use ($renderingContext, $self) {
// Rendering Array
$array209 = array();
$array210 = array (
);
$array209['0'] = $renderingContext->getVariableProvider()->getByPath('settings.templateLayout', $array210);
$array209['1'] = ' == 50';

return \TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\BooleanNode::evaluateStack($renderingContext, $array209);
};

\TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\BooleanNode::evaluateStack calls \TYPO3Fluid\Fluid\Core\Parser\BooleanParser->evaluate(" == 50", $context).

" == 50" is currently evaluated as true.


Example 2:

<f:else if="'{settings.templateLayout}' == 10">
</f:else>

works, because it results in "'' == 50" which is correctly evaluated as false

Cached part:

$arguments60['__elseifClosures'][] = function() use ($renderingContext, $self) {
// Rendering Array
$array206 = array();
$array206['0'] = '\'';
$array207 = array (
);
$array206['1'] = $renderingContext->getVariableProvider()->getByPath('settings.templateLayout', $array207);
$array206['2'] = '\' == 50';

return \TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\BooleanNode::evaluateStack($renderingContext, $array206);
};

Solution

I don't know the solution. $array209['0'] should somehow be wrapped by quotes (if not already done like in example 2) to get a syntactically correct expression. Or better, BooleanParser->evaluate($expression, $context) should recognize expressions like " == 50" as "'' == 50" or " != 50" as "'' != 50". There's an open issue regarding syntax checking -> #238 However, validating the syntax and returning false if it's not correct isn't enough because " != 50" should be evaluated as true despite the wrong syntax.

doerler avatar Mar 21 '18 14:03 doerler

Thanks for the detailed report!

I found this suspect: https://github.com/TYPO3/Fluid/blob/master/src/Core/Parser/SyntaxTree/BooleanNode.php#L109

The call to is_string does not also check if the string is empty and I think that's where all the trouble begins. If I'm right, then removing that is_string check should make it work (at the cost of a few additional lookups for reading the actual text in an expression). Maybe you can try and if it solves it, prepare a pull request?

PS: I think it evaluates to false because it never sees the comparator and just returns the boolean value of the first segment, the empty string. But I haven't confirmed this.

NamelessCoder avatar Mar 21 '18 15:03 NamelessCoder

Removing the is_string would work, but what about adding && $expressionPart !== '' to avoid the costs of additional lookups for strings that aren't empty?

if ($expressionPart instanceof TextNode || is_string($expressionPart) && $expressionPart !== '') {

doerler avatar Apr 16 '18 12:04 doerler

todo: add test to verify current behavior

lolli42 avatar Nov 24 '23 17:11 lolli42