Skip to content

Fix infinite recursion when a plugin on pdoToolsOnFenomInit re-enters getFenom()#396

Open
ShevArtV wants to merge 1 commit into
modx-pro:3x-newfrom
ShevArtV:fix/fenom-init-recursion
Open

Fix infinite recursion when a plugin on pdoToolsOnFenomInit re-enters getFenom()#396
ShevArtV wants to merge 1 commit into
modx-pro:3x-newfrom
ShevArtV:fix/fenom-init-recursion

Conversation

@ShevArtV

Copy link
Copy Markdown

Problem

On MODX 3 (3x-new), a static plugin attached to the pdoToolsOnFenomInit event makes the whole page die with:

Fatal error: Allowed memory size of NNN bytes exhausted ... in core/components/pdotools/src/Parsing/Fenom/Fenom.php

The same happens for any code reachable from a pdoToolsOnFenomInit handler that re-enters CoreTools::getFenom() while Fenom is still being constructed.

How to reproduce

  1. pdotools_fenom_parser = 1 (use Fenom as the parser).
  2. Create a plugin bound to pdoToolsOnFenomInit and mark it Static file (any handler, e.g. one that registers a custom modifier).
  3. Open any page that initializes Fenom → fatal "memory exhausted".

A DB (non‑static) plugin on the same event works fine, which is why this is easy to miss.

Root cause

fenom is registered as a shared service in bootstrap.php:

$modx->services->add('fenom', function ($c) use ($modx) {
    $class = $modx->getOption('pdotools_fenom_class', null, Fenom::class, true);
    return new $class($modx, $c->get('pdotools'));
});

The container memoizes the instance only after the factory closure returns. But the Fenom constructor invokes pdoToolsOnFenomInit before that:

// Fenom::__construct (end)
$this->modx->invokeEvent('pdoToolsOnFenomInit', ['fenom' => $this, ...]);

So if a plugin on that event re-enters getFenom()services->get('fenom'), the instance is not cached yet → the factory runs again → a new Fenom → the event again → … → memory exhausted.

With pdotools_fenom_parser = 1 this is reliably hit by static elements: loading a static plugin resolves its file through a media source, and modElement::getSourceFile() parses the source properties with the parser, which is the pdoParser and calls getFenom() again. Trace of the re-entry:

getFenom → new Fenom (factory) → pdoToolsOnFenomInit
  → load static plugin → modScript::getFileContent → modElement::getSourceFile
    → modFileMediaSource::initialize → getProperties → parseProperties
      → Parser::processElementTags → getFenom()   // factory runs again → recursion

(MODX 2 / the master branch is unaffected: its getSourceFile() only runs the parser when the file path itself contains tags, and never parses media‑source properties.)

Fix

Fire pdoToolsOnFenomInit once, from getFenom(), after the instance is registered in the container — guarded by a flag. A re‑entrant getFenom() then returns the already‑registered instance instead of building a new one. The event is removed from the constructor and exposed as Fenom::invokeInitEvent().

Validation

On a clean MODX 3 + pdoTools stand with pdotools_fenom_parser = 1 and a plugin on pdoToolsOnFenomInit that registers a modifier:

Plugin type before after
DB event fires, modifier works event fires, modifier works (no change)
Static fatal: memory exhausted event fires, modifier works, no crash

getChunk('@INLINE {$x\|myModifier}') returns the modified value in all cases; pdoToolsOnFenomInit is still dispatched exactly once per request.

…sOnFenomInit

CoreTools::getFenom() resolves the shared `fenom` service from the container,
whose factory builds the Fenom instance. The Fenom constructor fired the
pdoToolsOnFenomInit event before the container had memoized the instance, so
any plugin attached to that event which re-enters getFenom() caused the factory
to run again -> a new Fenom -> the event again -> ... until the PHP memory limit
was exhausted.

This is reliably triggered with pdotools_fenom_parser=1: loading a static
element resolves its file through a media source, and modElement::getSourceFile()
parses the source properties with the (pdo) parser, which calls getFenom().
If a static plugin is bound to pdoToolsOnFenomInit, the page dies with a fatal
"Allowed memory size exhausted".

Fire pdoToolsOnFenomInit once from getFenom(), after the instance is stored in
the services container, guarded by a flag. A re-entrant getFenom() then returns
the already-registered instance instead of constructing a new one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant