Revolution: Output filters broken for undefined placeholders in 2.4.3

[yoleg]

When an undefined placeholder has an output filter, the entire tag is removed instead of treating the placeholder as empty. For example, this chunk with an undefined placeholder fake:

Start [[!+fake:notempty=`Not empty`:default=`Default`]] End

Results in:

Start End

Instead of (as expected):

Start Default End

This was caused by pull request #12884. You can see the explanation in the comments to #12884. Assuming no-one fixes it sooner, I will submit a fix today.

Probable duplicate of #12901

[OptimusCrime]

As far as I can see this is a 100% duplicate. Any reason to keep this open?

[yoleg]

Sure, you can close this. Should I change the pull request to point to the other issue too, or it doesn't matter?

[pixelchutes]

I have closed the original since this one already has an associated PR.

[iLexN]

Not sure it is related to this topic or not.

[[*longtitle:notempty=`[[*longtitle]]`:else=`[[*pagetitle]]`]]

this work fine with php5.6 but cause error in php7

Parse error: syntax error, unexpected ')' in /home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php(191) : eval()'d code on line 1

after var_dump($conditional) in line 189;

....

/home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php:189:string '1' (length=1)

/home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php:189:string '0' (length=1)

/home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php:189:string '0' (length=1)

/home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php:189:string '' (length=0)

Parse error: syntax error, unexpected ')' in /home/dev/ihi/httpdocs/core/model/modx/filters/modoutputfilter.class.php(191) : eval()'d code on line 1

find that the empty string cause error in php7, https://3v4l.org/GbMP9

I know now may not support php7 , just for information.

[Jako]

@iLexN notempty is not a conditional modifier. So the else outputfilter query won't work in 5.6 and 7. I think it should show those issues in 5.6 too.

Two valid solutions for your query

[[*longtitle:default=`[[*pagetitle]]`]]
[[[[*longtitle:ne=``:then=`*longtitle`:else=`*pagetitle`]]]]

[iLexN]

@Jako , thanks for your advice.

i will search from the database , see how many template / chunk need to change.

The error is only come from php7 , php5.6 is working fine.

[opengeek]

@yoleg @pixelchutes the code in the original post is working as expected for me, so I am still unable to reproduce the problem being discussed here. Can someone explain the problem in more detail and provide steps to reproduce? I have created a chunk that calls another chunk that contains the tags above and it always returns Start Default End as expected.

[pixelchutes]

What is your parser_recurse_uncacheable setting set to?

[pixelchutes]

@opengeek: With parser_recurse_uncacheable enabled, try calling your Chunk _uncached_, on a cacheable Resource, with the placeholder call inside the chunk _cached_:

[[!$chunk]]

Contents of $chunk

[[+fake:default=`Some default value`]]

Expected Result:

_Output:_

Some default value

Actual Result:

_Output:_

_(empty)_

[opengeek]

That helped—successfully reproducing now. Thanks @pixelchutes.

[yoleg]

I submitted another pull request with a simpler fix - #12925

The only question I have left and why I called the fix "temporary" (I have not had time to look into this) is why the second part of if ($this->_output !== null || $this->modx->parser->startedProcessingUncacheable() is even needed in the first place before $this->filterOutput(); takes place.

After pull request #12884, parsing now switches back and forth from cached to uncached, so it should not matter whether uncached parsing has started globally or not. However, when I removed that second bit and left just if ($this->_output !== null) {..., I had issues.

The only answer I can think of is that parsing or caching during $this->modx->resource->process() (which is called before parser->startedProcessingUncacheable() becomes true) works differently with output filters than any cached parsing that happens during $this->modx->parser->processElementTags('', $this->modx->resource->_output, true, false, '[[', ']]', array(), $maxIterations);. (In core/model/modx/modrequest.class.php)

[bwente]

Thank you! I was going through all my chunks one by one trying to figure out was was went wrong with my upgrade.

parser_recurse_uncacheable

[pixelchutes]

There is a pending patch to fix this regression. Hopefully it will land in 2.4.x (and 2.5.x) soon.

[yoleg]

As a temporary fix, I am just copying the patched version of core/model/modx/modparser.class.php to all of my upgraded sites. (This file is in pull request #12925.)

Disabling parser_recurse_uncacheable also works, but it disables a fix that prevents certain uncached elements from getting cached. Most sites are not affected, but it could for example cause user or request data to be cached and thus be exposed to other users. See bug #12835. (Then again, this caching bug seems to have been present for many versions of MODX, so I bet most affected sites have put in workarounds for it already.)

@bwente

[pixelchutes]

Marking as closed, fixed for 2.4.4 _(pending release)_