Bug #33551: View helper values break out of a partial scope - TYPO3.Fluid - TYPO3 Forge

CMS-Garden Composer Team Editorial Team Extension Comparison Extension Coordination Team Marketing Team Sandbox Screencast Team Server Team Translation Team (archived projects)BLE Project DEV3 flow3.typo3.org Logging Project ReStructured Text Infrastructure TYPO3 Flow TYPO3 Flow Base Distribution TYPO3.Eel TYPO3.Flow TYPO3.Fluid TYPO3.Kickstart TYPO3.Welcome Packages Applications TYPO3 Neos

Custom queries

1.0 beta 1
Hierarchical Issues
Most wanted Bugfixes
Most wanted Features
My open issues
New & Unassigned
Open Bugs
Recently modified

Bug #33551

View helper values break out of a partial scope

Added by Stephan Schuler over 10 years ago. Updated almost 9 years ago.

Status:

New

Priority:

Must have

Assignee:

Sebastian Kurfuerst

Category:

Core

Target version:

Start date:

2012-01-31

Due date:

% Done:

Estimated time:

Has patch:

Description

If a partial is called at least twice which has a certain view helper inside, the last occurence of the inside value breaks out of the partials scope and will be the result of all usages of this view helper from now on.

This only appears when the first call triggers the caching mechanism but executes the uncached template (AbstractTemplateView::render() $parsedTemplate is instance of "TYPO3\Fluid\Core\Parser\ParsingState").
As soon as the template is a compiled one (AbstractTemplateView::render() $parsedTemplate is instance of "TYPO3\Fluid\Core\Compiler\AbstractCompiledTemplate"), the result is as expected.

Main template:

<html>
    <head>
        <title>Little bug</title>
    </head>
    <body>
        <ol>
            <li><f:escape type="xml">I am pre partial</f:escape></li>
            <f:render partial="Demo" arguments="{value: 'I am the first node within a partial'}" />
            <f:render partial="Demo" arguments="{value: 'I am not the first but the second node within a partial'}" />
            <li><f:escape type="xml">I am post partial</f:escape></li>
        </ol>
    </body>
</html>

Demo-Partial:

<li><f:escape type="xml">{value}</f:escape></li>

The expected result would be:

1. I am pre partial
2. I am the first node within a partial
3. I am not the first but the second node within a partial
4. I am post partial

But the given result is:

1. I am pre partial
2. I am the first node within a partial
3. I am not the first but the second node within a partial
4. I am not the first but the second node within a partial

Related issues

Related to TYPO3.Fluid - Bug #47674: HtmlentitiesViewHelper is scope singleton

Resolved

Alexander Berl

2013-04-28

Actions

Issue # Delay: days Cancel

History
Notes

Updated by Bastian Waidelich about 10 years ago

Category set to Core
Status changed from New to Accepted

Wow, thanks for the detailed description

Updated by Bastian Waidelich about 10 years ago

Status changed from Accepted to Needs Feedback
Assignee set to Bastian Waidelich

I fail to reproduce this with a slightly modified example (as the escape viewhelper is not longer part of the distribution):

.h3 Templates/Index.html

<html>
<head>
    <title>Little bug</title>
</head>
<body>
<ol>
    <li><f:format.htmlspecialchars>I am pre partial</f:format.htmlspecialchars></li>
    <f:render partial="test" arguments="{value: 'I am the first node within a partial'}" />
    <f:render partial="test" arguments="{value: 'I am not the first but the second node within a partial'}" />
    <li><f:format.htmlspecialchars>I am post partial</f:format.htmlspecialchars></li>
</ol>
</body>
</html>

.h3 Partials/test.html

<li><f:format.htmlspecialchars>{value}</f:format.htmlspecialchars></li>

Can you please verify this again?

Updated by Bastian Waidelich about 10 years ago

Bastian Waidelich wrote:

I fail to reproduce this [...]

@Stephan can you please confirm this, otherwise I'll close the issue for now

Updated by Bastian Waidelich over 9 years ago

Status changed from Needs Feedback to Closed

Closed because of missing feedback

Updated by Stephan Schuler over 9 years ago

Status changed from Closed to New

He Bastian.

Sorry for not responding, but forge didn't notify me and then I somehow went over this.

Unfortunatelly the error still persist.

The Fluid code looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<rendertest>
    <line><f:format.htmlentities>first main</f:format.htmlentities></line>
    <f:render partial="Partialtest" arguments="{first: 'first partial, first line', second: 'first partial, second line'}" />
    <f:render partial="Partialtest" arguments="{first: 'second partial, first line', second: 'second partial, second line'}" />
    <line><f:format.htmlentities>last main</f:format.htmlentities></line>
</rendertest>

<?xml version="1.0" encoding="UTF-8"?>
<rendertest>
    <line><f:format.htmlentities>first main</f:format.htmlentities></line>
    <f:render partial="Partialtest" arguments="{first: 'first partial, first line', second: 'first partial, second line'}" />
    <f:render partial="Partialtest" arguments="{first: 'second partial, first line', second: 'second partial, second line'}" />
    <line><f:format.htmlentities>last main</f:format.htmlentities></line>
</rendertest>

Output of the first request (here the template is not cached, gets rendered and then pushed to the caching directory):

<?xml version="1.0" encoding="UTF-8"?>
<rendertest>
    <line>first main</line>
    <line>
    first partial, first line
    first partial, second line
</line>
    <line>
    second partial, first line
    second partial, second line
</line>
    <line>second partial, second line</line>
</rendertest>

Output of all following requests (here the template has a representation in the Fluid caching directory and so gets only executed):

<?xml version="1.0" encoding="UTF-8"?>
<rendertest>
    <line>first main</line>
    <line>
    first partial, first line
    first partial, second line
</line>
    <line>
    second partial, first line
    second partial, second line
</line>
    <line>last main</line>
</rendertest>

So, nothing changed here.

Changing from <f:format.htmlentities>{foo}</f:format.htmletities> to {foo -> f:format.htmlentities} doesn't change anything.

Updated by Sebastian Kurfuerst over 9 years ago

That's very weird behavior. I'd try to reproduce it inside the viewhelpertest packages, I just cannot promise when I'll find the time.

But that looks like a bug worth fixing ;)

Updated by Alexander Berl about 9 years ago

Some findings:

I think the problem comes from how ParsingState, alas TemplateParser initializes singleton view helpers vs. how AbstractCompiledTemplate does. If you look closely, the format.htmlentities ViewHelper is of scope singleton, while the format.htmlspecialchars is not - that would explain why Bastian couldn't reproduce the problem.
On a closer look, AbstractCompiledTemplate calls viewHelper::resetState on singletons, while TemplateParser doesn't.

After a fix it should be reviewed if htmlentities needs to stay singleton, also the htmlspecialchars ViewHelper contains special code for the ::compile function, while none of the other format viewhelpers does. This cannot be correct to my understandings.

Updated by Alexander Berl about 9 years ago

I reproduced the bug with the Viewhelpertests, unfortunately the problem isn't only the missing resetState() as I suspected.
I also tried simply working around the problem by always using the compiled template even on first load in AbstractTemplateView, but that still doesn't solve the problem oddly enough:

    public function render($actionName = NULL) {
        $this->baseRenderingContext->setControllerContext($this->controllerContext);
        $this->templateParser->setConfiguration($this->buildParserConfiguration());

        $templateIdentifier = $this->getTemplateIdentifier($actionName);
        if ($this->templateCompiler->has($templateIdentifier)) {
            $parsedTemplate = $this->templateCompiler->get($templateIdentifier);
        } else {
            $parsedTemplate = $this->templateParser->parse($this->getTemplateSource($actionName));
            if ($parsedTemplate->isCompilable()) {
                $this->templateCompiler->store($templateIdentifier, $parsedTemplate);
                    // Directly use the compiled template
                $parsedTemplate = $this->templateCompiler->get($templateIdentifier);
            }
        }
        ...
    }

I hence suspect that there is something other wrong with the context or alike, which I can't really dig into atm.
I pushed my Viewhelpertests case at https://review.typo3.org/20270, so Bastian and/or Sebastian can work from there.

I will however add a new issue that just will remove the singleton scope from the htmlentities viewhelper, which will at least be a cosmetic fix for this issue.

Updated by Bastian Waidelich almost 9 years ago

Assignee changed from Bastian Waidelich to Sebastian Kurfuerst

Alexander Berl wrote:

I reproduced the bug with the Viewhelpertests, unfortunately the problem isn't only the missing resetState() as I suspected.

Thanks a lot for preparing this so nicely, Alexander!

Sebastian Kurfuerst: I'm assigning this to you only for feedback

I finally came around having a closer look at this again and here's my finding:

If I remove the @Flow\Scope("singleton") from the HtmlentitiesViewHelper your second test case still fails during parsing on the second hit everything works.
If I instead remove lines 41 - 46 of the AbstractCompiledTemplate, everything works as expected..

Here my suggestion:
First of all we should get rid of the singleton scope.. HtmlentitiesViewHelper is the only VH that is a singleton – and I can't exactly remember why, probably for performance reasons (this VH is used a lot because it's added automatically by Fluid for HTML templates).
To save memory and increase performance we should implement the CompilableInterface instead.
This looks like that https://gist.github.com/bwaidelich/e0e34ba95fb2b2306dab and fixes the issue for me without further adjustments – we should do that for other core VHs to IMO

Anyways, I still didn't completely get why the output of the HtmlentitiesViewHelper is "cached" otherwise..

#10

Updated by Bastian Waidelich almost 9 years ago

Bastian Waidelich wrote:

A little addition:

If I remove the @Flow\Scope("singleton") from the HtmlentitiesViewHelper your second test case still fails during parsing on the second hit everything works.

Obviously, because your second test uses a new ViewHelper "Singleton" that is.. of scope singleton. If I remove that too, everything works too

#11

Updated by Alexander Berl almost 9 years ago

Bastian Waidelich wrote:

Bastian Waidelich wrote:

A little addition:

If I remove the @Flow\Scope("singleton") from the HtmlentitiesViewHelper your second test case still fails during parsing on the second hit everything works.

Obviously, because your second test uses a new ViewHelper "Singleton" that is.. of scope singleton. If I remove that too, everything works too

Yes, I added that so there's still a test for the singleton-misbehavior even when the htmlentities viewhelper is not singleton any more :)

To save memory and increase performance we should implement the CompilableInterface instead.

Is there any profiling-suite or setup you use for verifying the performance/memory influence of changes?

Also available in: Atom PDF