Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluation of cached boolean fluid expressions with empty strings #382

Open
doerler opened this issue Mar 21, 2018 · 3 comments
Open

Evaluation of cached boolean fluid expressions with empty strings #382

doerler opened this issue Mar 21, 2018 · 3 comments
Assignees

Comments

@doerler
Copy link

doerler commented Mar 21, 2018

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.

@NamelessCoder
Copy link
Contributor

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.

@doerler
Copy link
Author

doerler commented Apr 16, 2018

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 !== '') {

@lolli42
Copy link
Member

lolli42 commented Nov 24, 2023

todo: add test to verify current behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants