[3.0.2] Fix parsing modifiers when encountering literal brackets

[Mark-H]

What does it do?

Fix where literal brackets (not intended as an opening/closing tag) go in the parsed command modifiers.

Why is it needed?

John MacDonald reported the parser breaking in the following scenario:

[[++google_analytics:notempty=`
<!-- Global site tag (gtag.js) - Google Analytics -->
<script async src="https://www.googletagmanager.com/gtag/js?id=[[++google_analytics]]"></script>
<script>
  window.dataLayer = window.dataLayer || [];
  function gtag(){dataLayer.push(arguments);}
  gtag('js', new Date());

  gtag('config', '[[++google_analytics]]');
</script>
`]]

Rather than outputting the script, it returned just the value of [[++google_analytics]].

That test case was simplified to [[+tag:notempty=`[]`]] as the literal brackets appeared to be the problem.

Internally, the command ended up as notempty[] with an empty modifier. The notempty[] command doesn't exist so the original value is returned in its place.

By using the $inModifier flag (which gets set in the switch case for ` when depth is 0) the brackets are parsed into the modifiers as they should.

How to test

See tests for examples. Feel free to come up with more complex cases.

Related issue(s)/PR(s)

Reported via Slack.

Introduced with #16288 in 3.0.2.

[Maef]

@Mark-H Can you also add a test for

[[[[!snippetname=`1`:then=`$chunkname`]]]]

because we get this error log

Could not find snippet with name [[!snippetname=`1`:then=`$chunkname`]].

To be honest this is our fix after the 3.0.2 release. We had to simplify it. Before we had this kind of code

[[!snippet:is=`1`:then=` [[++settingname1:notempty=`[[$chunkname1? &id=`[[++settingname1]]`]]`]] [[++settingname2:notempty=`[[$chunkname2? &id=`[[++settingname2]]`]]`]] `]]

[Mark-H]

That tag isn't valid; the inner uncached tag will always be processed later than the outer cached tag, leading to that error. It'll still "work" because the cached tag does get processed later on when the uncached tag is also parsed, but it is wrong.

You'll need to uncache the outer tag too to make it correct:

[[![[!snippetname:eq=`1`:then=`$chunkname`]]]]

[Maef]

Corrected our wrong code.

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/a-colon-in-a-resources-content-is-breaking-a-template/6127/4

[smg6511]

@Mark-H - I've been working on testing this and wanted to be clear on a couple things before submitting a review:

1. Since problem cases may likely continue to pop up, I get the feeling this should just be reviewed for what it fixes at this point in time and not as a fixes it all solution. If you agree, I'm ready to check “approve” and move on.
2. I have a couple examples of something that doesn't work, but that may be better to fix in a supplemental PR:
   a. This new case fails: [[+tag1:notempty=`[[+tag1]][]`]]
   b. This may be me doing it wrong (in terms of the order of caching), but none of the cases work uncached in the following scenario (output is empty on all); all do work if I set the chunk I'm calling in the template to uncached.

Test Resource (where the cases are placed):

        [[!+tag1:notempty=`[]`]]
        [[!+tag1:notempty=`]`]]
        [...]

Test Chunk (body--test-pr-16302)

        <h2>[[*pagetitle]]</h2>
         [[*content]]

Test Template (Test 16302)

        [[$body--test-pr-16302?
            &tag1=`TagOneValue`
            &tag2=`TagTwoValue`
        ]]

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/modx-3-02-input-output-filters/6242/2

[Mark-H]

Hmm, 2a is interesting, not immediately seeing the cause for that. Putting a space between the end of the tag and the random brackets does help: [[+tag1:notempty=`[[+tag1]] []`]]

The other case seems to be due to the uncached placeholders in a cached chunk. The chunk will get parsed, the uncached placeholders left alone, and then when the uncached gets processed those values no longer exist.

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/modx3-broken-output-filter-modifiers/5906/13

[halftrainedharry]

2. a. This new case fails: [[+tag1:notempty=`[[+tag1]][]`]]

I think the error happens on this line in the modParser:

revolution/core/src/Revolution/modParser.php

Line 150 in ad6977d

$pattern = '/\Q'.$prefix.'\E((?:(?:[^'.$subSuffix.$subPrefix.'][\s\S]*?|(?R))*?))\Q'.$suffix.'\E/x';

This seems to be a separate issue and unrelated to this PR.

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/modx-3-0-3-and-seo-suite-3-1-1-rc4/6295/2

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/content-block-conditional-render-nested-is-1-then-not-working/6321/2

[ju123-2-2]

We have this situation

[[+nested:is=`1`:then=`

    <div class="row">
        <div class="col-xs-12 col-sm-12 col-md-6 [[+bgColour1]]">
            ![[+col1]]
        </div>
        <div class="col-xs-12 col-sm-12 col-md-6 [[+bgColour2]]">
            [[+col2]]
        </div>
    </div>
`:else=`
    <div class="container">
        <div class="row">
            <div class="col-xs-12 col-sm-12 col-md-6 [[+bgColour1]]">
            [[+col1]]
            </div>
            <div class="col-xs-12 col-sm-12 col-md-6 [[+bgColour2]]">
                [[+col2]]
            </div>
        </div>
    </div>
`]]

but it just render this parts below as a text
[[+nested:is=1:then=` ...

...:else=...
even after updating
core/src/Revolution/Filters/modInputFilter.php)

[Mark-H]

Try also applying #16316.

[JoshuaLuckers]

And if there are still issues please open a new issue

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/output-modifiers-issue-on-modx-3-0-3/6405/2

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/problems-after-update-to-modx-3/6429/2

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/pthumb-filtering-with-php8/6431/4