-
-
Notifications
You must be signed in to change notification settings - Fork 527
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
At sign (@) inside chunk parameter value nested into tag output filter breaks tag parsing #16318
Comments
I can't reproduce the issue with the provided example content. |
I can not reproduce this either (on a local install of 3.0.3-dev). If there is in fact a reproducible bug, you're going to need to provide detail on your actual code, not just the pseudo code shown above. |
Thanks guys for your testing. The code above is not a pseudocode, but actual code, the simplest version that demostrates problem. Snippet |
OK, I can reproduce it now (MODX 3.0.2, PHP 7.4) but only if the snippet "xxx" doesn't exist. |
Oh, at least we are moving forward. I can confirm that when snippet exist, it works correctly. I still considers this to be core MODx bug. Probably it wouldn't impose problem on most sites, but have some implications in more complicated scenarios, like ContentBlocks. |
Think I have a fix. Will report back in a bit. |
See potential fix at #16322 - please test and report back. That PR is surfacing a potentially different problem related to property sets that I haven't figured out yet though. |
Just curious — is it necessary to support the ability to call a non-existent snippet? Or put differently, what's the use-case for being able to do so? |
The use case is not breaking the parsing if a snippet doesn't exist. |
Fair enough ;-) Is there any/sufficient error reporting when a non-existent snippet is called? I know this would be a different issue if the answer were "no." |
Agree with Mark, parsing should be done right even if snippet doesn't exist. On very complex sites we dynamically build chunk and snippet names, e.g. from context settings. So, in some context we call a different snippet than in others. In this case sometimes no snippet should be called and of course should not break page. |
Not disagreeing, just wanted to understand the motivations. I too have set up dynamic tags like that of your example, but have never done so where the resulting tag calls a snippet that might not exist. I've more so used that strategy with chunks that pull up different variations of a block of code depending on a certain context/condition. |
The problem may've also been reproducible with an uncached valid snippet in the modifier of a cached placeholder. |
On my test caching of snippet and/or placeholder doesn't change the result. |
Output filters are parsed using a completely different approach than the rest of the parser. It was bolted onto the parser after the original design to incorporate features from a popular extra into the core. |
Btw, we have found another, similar problem, this time related only to MODx 3.x.
This works fine in 2.7 version. |
The pull request #16302 probably fixes this problem. |
I have sucessfully tested this PR, works well in stand-alone test as well in ContentBlocks implementation |
Yes, this PR seems to fix it |
Consider this content:
Supposed output should be:
Instead it is:
This works fine as expected:
At sign (@) inside parameter value breaks parsing, but only when nested inside output filter parameter value. Workaround here is to put inner tag inside separate chunk.
With some more complicated scenarios this even causes Server error with
End of script output before headers: index.php
in error log.Clean MODx 3.0.2 install, the same with older 2.7.3
The text was updated successfully, but these errors were encountered: