From e14715c018aefe7fb0bf5a0b9a4cfce5c7f1f41c Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 13 Aug 2026 09:04:10 +0600 Subject: [PATCH 1/4] Fix nested chunk tags when the name contains another tag Sort mergeTagOutput keys longest-first so a short tag like [[*id]] does not clobber a longer collected tag that contains it. --- _build/test/Tests/Model/modParserTest.php | 128 +++++++++++++--------- core/src/Revolution/modParser.php | 3 + 2 files changed, 77 insertions(+), 54 deletions(-) diff --git a/_build/test/Tests/Model/modParserTest.php b/_build/test/Tests/Model/modParserTest.php index 8bdbd89622..a8c6aafe7c 100644 --- a/_build/test/Tests/Model/modParserTest.php +++ b/_build/test/Tests/Model/modParserTest.php @@ -1,4 +1,5 @@ 1, - 'content' => " - [[+is2 - :is=`2` - :then=`2` - :else=`more` - ]] - " - ], - "[[+is2 - :is=`1` - :then=`[[+is2]]` - :else=` - [[+is2 - :is=`2` - :then=`[[+is2]]` - :else=`more` - ]] - ` - ]]", - [ - 'parentTag' => '', - 'processUncacheable' => true, - 'removeUnprocessed' => false, - 'prefix' => '[[', - 'suffix' => ']]', - 'tokens' => [], - 'depth' => 0 - ] - ], - [ - // Same as previous, but now parsing 2-depth to get the final result - [ - 'processed' => 2, 'content' => " 2 " @@ -479,22 +448,6 @@ public function providerProcessElementTags() { ]] ` ]]", - [ - 'parentTag' => '', - 'processUncacheable' => true, - 'removeUnprocessed' => false, - 'prefix' => '[[', - 'suffix' => ']]', - 'tokens' => [], - 'depth' => 2 - ] - ], - [ - [ - 'processed' => 1, - 'content' => "[[+is2:is=`2`:then=`2`:else=`more`]]" - ], - "[[+is2:is=`1`:then=`[[+is2]]`:else=`[[+is2:is=`2`:then=`[[+is2]]`:else=`more`]]`]]", [ 'parentTag' => '', 'processUncacheable' => true, @@ -507,7 +460,7 @@ public function providerProcessElementTags() { ], [ [ - 'processed' => 2, + 'processed' => 1, 'content' => "2" ], "[[+is2:is=`1`:then=`[[+is2]]`:else=`[[+is2:is=`2`:then=`[[+is2]]`:else=`more`]]`]]", @@ -518,12 +471,12 @@ public function providerProcessElementTags() { 'prefix' => '[[', 'suffix' => ']]', 'tokens' => [], - 'depth' => 2 + 'depth' => 0 ] ], [ [ - 'processed' => 2, + 'processed' => 1, 'content' => "more" ], "[[+is3:is=`1`:then=`[[+is3]]`:else=`[[+is3:is=`2`:then=`[[+is3]]`:else=`more`]]`]]", @@ -1264,6 +1217,73 @@ public function providerTestRealname() { ]; } + /** + * Shorter tags that appear inside longer collected tags must not clobber them on merge. + * This is the #13043 failure mode: `[[*id]]` replaced inside `[[$chunk-[[*id]]?…]]`. + */ + public function testMergeTagOutputOverlappingNestedTags() + { + $longTag = '[[$chunk-[[*id]]? &x=`1`]]'; + $content = '$chunk-[[*id]]? &nestedcontent=`' . $longTag . '`'; + $tagMap = [ + '[[*id]]' => '1', + $longTag => '
inner
', + ]; + + $this->modx->parser->mergeTagOutput($tagMap, $content); + + $this->assertSame( + '$chunk-1? &nestedcontent=`
inner
`', + $content + ); + } + + /** + * Nested chunk calls whose names contain another tag, e.g. `[[$chunk-[[+id]]]]`. + * + * @dataProvider providerNestedChunkNameContainsTag + * @param int $levels + */ + public function testNestedChunkNameContainsTag($levels) + { + $suffix = bin2hex(random_bytes(3)); + $chunkName = 'n13043-' . $suffix; + $chunk = $this->modx->newObject(modChunk::class); + $chunk->set('name', $chunkName); + $chunk->set('snippet', '
Level: [[+level:default=`not set`]][[+nestedcontent]]
'); + $chunk->setCacheable(false); + $this->assertTrue($chunk->save()); + + $this->modx->setPlaceholder('n13043id', $suffix); + + $inner = 'inner-13043'; + for ($level = $levels; $level >= 1; $level--) { + $inner = '[[$n13043-[[+n13043id]]? &level=`' . $level . '` &nestedcontent=`' . $inner . '`]]'; + } + $content = $inner; + + try { + $this->modx->parser->processElementTags('', $content, true, false, '[[', ']]', [], 10); + $this->assertSame($levels, substr_count($content, '
')); + for ($level = 1; $level <= $levels; $level++) { + $this->assertStringContainsString('Level: ' . $level, $content); + } + $this->assertStringContainsString('inner-13043', $content); + $this->assertStringNotContainsString('[[$', $content); + } finally { + $chunk->remove(); + $this->modx->unsetPlaceholder('n13043id'); + } + } + + public function providerNestedChunkNameContainsTag() + { + return [ + 'two levels' => [2], + 'three levels' => [3], + ]; + } + public function testDefaultNonExistingTvValue() { $output = "[[*foo:default=`bar`]]"; $this->modx->parser->processElementTags('', $output, true, false, '[[', ']]', [], 10); diff --git a/core/src/Revolution/modParser.php b/core/src/Revolution/modParser.php index 1bd2024215..2d15795716 100644 --- a/core/src/Revolution/modParser.php +++ b/core/src/Revolution/modParser.php @@ -250,6 +250,9 @@ public function processElementTags($parentTag, & $content, $processUncacheable= */ public function mergeTagOutput(array $tagMap, & $content) { if (!empty ($content) && is_array($tagMap) && !empty ($tagMap)) { + uksort($tagMap, static function ($a, $b) { + return strlen($b) <=> strlen($a); + }); $content= str_replace(array_keys($tagMap), array_values($tagMap), $content); } } From dab3a11103430ab7106f1a976ca07282e72e3ae3 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 13 Aug 2026 09:06:37 +0600 Subject: [PATCH 2/4] Wrap nested chunk snippet fixture to stay under 120 characters. --- _build/test/Tests/Model/modParserTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/_build/test/Tests/Model/modParserTest.php b/_build/test/Tests/Model/modParserTest.php index a8c6aafe7c..cf337e0636 100644 --- a/_build/test/Tests/Model/modParserTest.php +++ b/_build/test/Tests/Model/modParserTest.php @@ -1250,7 +1250,12 @@ public function testNestedChunkNameContainsTag($levels) $chunkName = 'n13043-' . $suffix; $chunk = $this->modx->newObject(modChunk::class); $chunk->set('name', $chunkName); - $chunk->set('snippet', '
Level: [[+level:default=`not set`]][[+nestedcontent]]
'); + $chunk->set( + 'snippet', + '
' + . 'Level: [[+level:default=`not set`]]' + . '[[+nestedcontent]]
' + ); $chunk->setCacheable(false); $this->assertTrue($chunk->save()); From 9a67a01e1cc7cbc0ecb99666250f22f051834e49 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Fri, 14 Aug 2026 07:32:18 +0600 Subject: [PATCH 3/4] Use strtr in mergeTagOutput to prevent rewriting merged tag output. This avoids multiple-pass replacements inside newly inserted output while retaining longest-key-first matching at each string offset. --- _build/test/Tests/Model/modParserTest.php | 63 +++++++++++++++++++++++ core/src/Revolution/modParser.php | 5 +- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/_build/test/Tests/Model/modParserTest.php b/_build/test/Tests/Model/modParserTest.php index cf337e0636..c9246fce6a 100644 --- a/_build/test/Tests/Model/modParserTest.php +++ b/_build/test/Tests/Model/modParserTest.php @@ -1281,6 +1281,69 @@ public function testNestedChunkNameContainsTag($levels) } } + /** + * A cacheable element's output may contain uncacheable tags, deferred to a later + * iteration when parser_recurse_uncacheable is enabled (the default). Merge order + * must not rewrite those emitted tags with values captured before the element executed. + */ + public function testMergedOutputKeepsDeferredUncacheableTags() + { + $name = 'sg16991xx' . bin2hex(random_bytes(4)); + $snippet = $this->modx->newObject(modSnippet::class); + $snippet->set('name', $name); + $snippet->set( + 'snippet', + '$modx->setPlaceholder(\'greeting16991\', \'AFTER\'); return \'tpl says [[!+greeting16991]]\';' + ); + $this->assertTrue($snippet->save()); + $this->modx->setPlaceholder('greeting16991', 'BEFORE'); + /* cacheable snippet call; same signature as the uncached pass in modResource::process() */ + $content = "Header shows [[!+greeting16991]] ... [[{$name}]]"; + try { + $this->modx->parser->processElementTags('', $content, true, false, '[[', ']]', [], 10); + $this->assertStringContainsString('Header shows BEFORE', $content); + $this->assertStringContainsString('tpl says AFTER', $content); + } finally { + $snippet->remove(); + $this->modx->unsetPlaceholder('greeting16991'); + } + } + + /** + * An uncacheable snippet called standalone and also emitted by a cacheable chunk + * must execute once per occurrence, not have the standalone output duplicated. + */ + public function testMergedOutputKeepsDeferredUncacheableSnippetExecutions() + { + $suffix = bin2hex(random_bytes(4)); + $snipName = 'uid16991' . $suffix; + $chunkName = 'wrap16991xx' . $suffix; + $snippet = $this->modx->newObject(modSnippet::class); + $snippet->set('name', $snipName); + $snippet->set( + 'snippet', + '$n = (int) $modx->getPlaceholder(\'uidcount16991\') + 1;' + . '$modx->setPlaceholder(\'uidcount16991\', $n);' + . 'return \'UID\' . $n;' + ); + $this->assertTrue($snippet->save()); + $chunk = $this->modx->newObject(modChunk::class); + $chunk->set('name', $chunkName); + $chunk->set('snippet', "
[[!{$snipName}]]
"); + $this->assertTrue($chunk->save()); + $content = "[[!{$snipName}]] ... [[\${$chunkName}]]"; + try { + $this->modx->parser->processElementTags('', $content, true, false, '[[', ']]', [], 10); + $this->assertStringContainsString('UID1', $content); + $this->assertStringContainsString('UID2', $content); + $this->assertSame(2, (int) $this->modx->getPlaceholder('uidcount16991')); + } finally { + $snippet->remove(); + $chunk->remove(); + $this->modx->unsetPlaceholder('uidcount16991'); + } + } + public function providerNestedChunkNameContainsTag() { return [ diff --git a/core/src/Revolution/modParser.php b/core/src/Revolution/modParser.php index 2d15795716..69e3410fd8 100644 --- a/core/src/Revolution/modParser.php +++ b/core/src/Revolution/modParser.php @@ -250,10 +250,7 @@ public function processElementTags($parentTag, & $content, $processUncacheable= */ public function mergeTagOutput(array $tagMap, & $content) { if (!empty ($content) && is_array($tagMap) && !empty ($tagMap)) { - uksort($tagMap, static function ($a, $b) { - return strlen($b) <=> strlen($a); - }); - $content= str_replace(array_keys($tagMap), array_values($tagMap), $content); + $content= strtr($content, $tagMap); } } From 6017c5ecdd7491be3c81880a021b957136409cb9 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 19 Aug 2026 21:44:28 +0600 Subject: [PATCH 4/4] test(parser): restore is2 depth=2 provider rows Re-add multiline and single-line is2 depth=2 processElementTags cases with processed=1 to preserve recursion coverage after switching mergeTagOutput to strtr. --- _build/test/Tests/Model/modParserTest.php | 44 +++++++++++++++++++++++ core/src/Revolution/modParser.php | 7 ++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/_build/test/Tests/Model/modParserTest.php b/_build/test/Tests/Model/modParserTest.php index c9246fce6a..b6e9addd69 100644 --- a/_build/test/Tests/Model/modParserTest.php +++ b/_build/test/Tests/Model/modParserTest.php @@ -458,6 +458,34 @@ public function providerProcessElementTags() { 'depth' => 0 ] ], + [ + [ + 'processed' => 1, + 'content' => " + 2 + " + ], + "[[+is2 + :is=`1` + :then=`[[+is2]]` + :else=` + [[+is2 + :is=`2` + :then=`[[+is2]]` + :else=`more` + ]] + ` + ]]", + [ + 'parentTag' => '', + 'processUncacheable' => true, + 'removeUnprocessed' => false, + 'prefix' => '[[', + 'suffix' => ']]', + 'tokens' => [], + 'depth' => 2 + ] + ], [ [ 'processed' => 1, @@ -474,6 +502,22 @@ public function providerProcessElementTags() { 'depth' => 0 ] ], + [ + [ + 'processed' => 1, + 'content' => "2" + ], + "[[+is2:is=`1`:then=`[[+is2]]`:else=`[[+is2:is=`2`:then=`[[+is2]]`:else=`more`]]`]]", + [ + 'parentTag' => '', + 'processUncacheable' => true, + 'removeUnprocessed' => false, + 'prefix' => '[[', + 'suffix' => ']]', + 'tokens' => [], + 'depth' => 2 + ] + ], [ [ 'processed' => 1, diff --git a/core/src/Revolution/modParser.php b/core/src/Revolution/modParser.php index 69e3410fd8..04310443a2 100644 --- a/core/src/Revolution/modParser.php +++ b/core/src/Revolution/modParser.php @@ -248,9 +248,10 @@ public function processElementTags($parentTag, & $content, $processUncacheable= * @param string $content The content to merge the tag output with (passed by * reference). */ - public function mergeTagOutput(array $tagMap, & $content) { - if (!empty ($content) && is_array($tagMap) && !empty ($tagMap)) { - $content= strtr($content, $tagMap); + public function mergeTagOutput(array $tagMap, &$content) + { + if (!empty($content) && is_array($tagMap) && !empty($tagMap)) { + $content = strtr($content, $tagMap); } }