wrong parse when having a semi-column with a setting variables in output filter 'then' or 'else'

[intersel]

Bug report

Summary

When we add a semi-column (';') in the output filter "then", modx can't parse properly and the output is strange....

This behaviour needs to use twice a same setting value... see example.

Step to reproduce

create snippet "echoinput"

<?php
return $input;

create a resource "test bug" with a void template and the following content:

[[echoinput?&input=`[[++mail_smtp_hosts]] [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`?input=`0`]]`]]

Observed behavior

result:

`localhost [[echoinput:eq=`0`:then=`test1:localhost

Expected behavior

localhost test1:localhost;

If you remove the ';' in the then filter like test1:[[++mail_smtp_hosts]] instead of test1:[[++mail_smtp_hosts]];

then everything is parsing ok....

If you add a blank after the name of the setting variable "[[++mail_smtp_hosts ]]" , like

[[echoinput?&input=`[[++mail_smtp_hosts ]] [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`?input=`0`]]`]]

The parsing is ok too !

Related issue(s)/PR(s)

can not say...

Environment

MODX Revolution 2.8.3-pl, Apache/2.4.38 , 10.3.31-MariaDB, Firefox/chrome/...

[JoshuaLuckers]

@intersel are you sure the syntax is right? Does it also occur if you only use [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`]] ?

[intersel]

@JoshuaLuckers Syntax seems right... can't see what's wrong actually :s

if I only use [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`]] it gives me test2:localhost That's seem correct to me...

[intersel]

It is really a weird bug! as it is really in this kind of steps to reproduce that you have it... as soon as I remove or change something... it's working as expected!

[JoshuaLuckers]

It sure is, maybe it has something to do with using the same snippet (tag) twice. In 3.x I made some adjustments to the parser. Would it be possible for you to test it there as well?

[intersel]

Try to do that in the next days... that's an opportunity to try this new version! stay tune ;)

[JoshuaLuckers]

@intersel did you find an opportunity to test this in MODX 3.0?

[intersel]

@JoshuaLuckers Thanks to remind me this! I'll do my best to do this test asap...

[JoshuaLuckers]

I'm able to reproduce this in 3.x.. I think the parser doesn't like the : and ; in the then and else modifiers.

If I replaced the : and removed the ; like this the output shows up fine:

[[echoinput?&input=`[[++mail_smtp_hosts]] [[echoinput:eq=`0`:then=`test1-[[++mail_smtp_hosts]]`:else=`test2-[[++mail_smtp_hosts]]`?input=`0`]]`]]

[JoshuaLuckers]

This is where it goes wrong: https://github.com/modxcms/revolution/blob/697a0286117a7e30b19c7f78ef30803806daf6c3/core/src/Revolution/Filters/modInputFilter.php#L56

The pattern used in the preg_match_all needs to be fixed.

[JoshuaLuckers]

Related issue: #14089

[intersel]

Doing some investigations on 2.8.3 (sorry for that... as I need to make it works on this one for now), the modInputFilter/modOutputFilter filter functions seem to do correctly the job, the regex sounds ok... the output string results sound ok to me... It's like the ";" pb come from the functions that call the filter function... need to do some more investigations...

But as the regex in 2.8.3 is not the same than in Modx 3.0, the environment is not the same. So perhaps there's a pb in the regex too somewhere...?

[JoshuaLuckers]

@intersel you should be able to back port the Unit Test from PR #16165 and additional test case I mentioned in the comment.

[Ibochkarev]

@JoshuaLuckers actual?

[rthrash]

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

https://community.modx.com/t/output-modifier-issue-in-modx-3-0-1-pl/5376/20

[rthrash]

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

https://community.modx.com/t/modx3-broken-output-filters/5906/1

[halftrainedharry]

The cause of this error is not the output filter but this line in the parser, that tries to parse the snippet properties:

https://github.com/modxcms/revolution/blob/f7bc0133f5f09184418a7ddf6f9b98a3f0decd9e/core/src/Revolution/modParser.php#L302

---

With this content

[[echoinput?&input=`[[++mail_smtp_hosts]] [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]]`]]

the parser does its job (parsing the inner tags first) and finds the following replacement:

• Replace [[++mail_smtp_hosts]] with localhost
• Replace [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]] with localhost;

The code does the first replacement (with str_replace). This results in the following content:

[[echoinput?&input=`localhost [[echoinput:default=`localhost;`?input=``]]`]]

Both occurrences of [[++mail_smtp_hosts]] were replaced!

The code tries to do the second replacement, but that doesn't work anymore as the string [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]] no longer exists in the content.

The parser carries on, but when parsing the snippet properties in the function "parsePropertyString", the inner tag confuses the parser in the function "escSplit":

`localhost [[echoinput:default=`localhost;`?input=``]]`

The character ; doesn't seem to be inside of backticks, if you ignore the inner tag.

---

Why does it work if you add a blank after the name of the setting variable [[++mail_smtp_hosts ]]?

[[echoinput?&input=`[[++mail_smtp_hosts ]] [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]]`]]

The first replacement (replacing [[++mail_smtp_hosts ]] with localhost) now replaces only the first occurrence of the system setting and the second replacement (replacing [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]] with localhost;) is successful.

The parser carries on with the content

[[echoinput?&input=`localhost localhost;`]]

and successfully parses the snippet properties (as the character ; is inside of backticks and is therefore ignored).

---

How to fix it?

The function "escSplit" (class xPDO) could be changed to account for possible nested tags by detecting start ([[) and end (]]) of nested tags and keeping track of the depth similar to #16288.

Or maybe the parser could keep track of the position of tags in the content, so that no unexpected replacements can take place.

[halftrainedharry]

Because this problem is caused by one replacement string being part of another replacement string (the string [[echoinput:default=`[[++mail_smtp_hosts]];`?input=``]] contains the string [[++mail_smtp_hosts]]), it can maybe be solved by sorting the replacement-array before doing the replacements.

Adding a uksort to this function

https://github.com/modxcms/revolution/blob/f7bc0133f5f09184418a7ddf6f9b98a3f0decd9e/core/src/Revolution/modParser.php#L248-L252

like this

public function mergeTagOutput(array $tagMap, & $content) {

	//sorting the tagMap by descending length of the key. A longer key can't be a part of a shorter key.
	uksort($tagMap, function($a, $b) {
		$lenA = strlen($a);
		$lenB = strlen($b);
		if ($lenA == $lenB) return 0;
		return ($a < $b) ? 1 : -1;
	});

	if (!empty ($content) && is_array($tagMap) && !empty ($tagMap)) {
		$content= str_replace(array_keys($tagMap), array_values($tagMap), $content);
	}
}

solves the issue at hand. I don't know if there are any broader implications though.