codefog / contao-haste

Haste is a collection of tools and classes to ease working with Contao
http://codefog.pl/extension/haste.html
MIT License
43 stars 24 forks source link

Infinite loop in StringUtil::recursiveReplaceTokensAndTags (in conjunction with Isotope) #93

Closed binron closed 7 years ago

binron commented 7 years ago

The problem lies in the changed Contao core function "parseSimpleTokens".

The old version didn't care if the value exists or not:

    public static function parseSimpleTokens($strString, $arrData)
    {
        $strReturn = '';

        // Remove any unwanted tags (especially PHP tags)
        $strString = strip_tags($strString, $GLOBALS['TL_CONFIG']['allowedTags']);
        $arrTags = preg_split('/(\{[^\}]+\})/', $strString, -1, PREG_SPLIT_DELIM_CAPTURE|PREG_SPLIT_NO_EMPTY);

        // Replace the tags
        foreach ($arrTags as $strTag)
        {
            if (strncmp($strTag, '{if', 3) === 0)
            {
                $strReturn .= preg_replace('/\{if ([A-Za-z0-9_]+)([=!<>]+)([^;$\(\)\[\]\}]+).*\}/i', '<?php if ($arrData[\'$1\'] $2 $3): ?>', $strTag);
            }
            elseif (strncmp($strTag, '{elseif', 7) === 0)
            {
                $strReturn .= preg_replace('/\{elseif ([A-Za-z0-9_]+)([=!<>]+)([^;$\(\)\[\]\}]+).*\}/i', '<?php elseif ($arrData[\'$1\'] $2 $3): ?>', $strTag);
            }
            elseif (strncmp($strTag, '{else', 5) === 0)
            {
                $strReturn .= '<?php else: ?>';
            }
            elseif (strncmp($strTag, '{endif', 6) === 0)
            {
                $strReturn .= '<?php endif; ?>';
            }
            else
            {
                $strReturn .= $strTag;
            }
        }

        // Replace tokens
        $strReturn = str_replace('?><br />', '?>', $strReturn);
--------------> $strReturn = preg_replace('/##([A-Za-z0-9_]+)##/i', '<?php echo $arrData[\'$1\']; ?>', $strReturn);
        $strReturn = str_replace("]; ?>\n", '] . "\n"; ?>' . "\n", $strReturn); // see #7178

        // Eval the code
        ob_start();
        $blnEval = eval("?>" . $strReturn);
        $strReturn = ob_get_contents();
        ob_end_clean();

        // Throw an exception if there is an eval() error
        if ($blnEval === false)
        {
            throw new \Exception("Error parsing simple tokens ($strReturn)");
        }

        // Return the evaled code
        return $strReturn;
    }

The new one does:

    public static function parseSimpleTokens($strString, $arrData)
    {
        $strReturn = '';

        $replaceTokens = function ($strSubject) use ($arrData)
        {
            // Replace tokens
            return preg_replace_callback(
                '/##([^=!<>\s]+?)##/',
                function (array $matches) use ($arrData)
                {
--------------------------------------> if (!array_key_exists($matches[1], $arrData))
                    {
                        return '##' . $matches[1] . '##';
                    }

                    return $arrData[$matches[1]];
                },
                $strSubject
            );
        };
                ...

In my case the value for ##payment_label## was not set and the new version of parseSimpleTokens does not replace the token if the key doesn't exists, so here is the infinite loop in StringUtil::recursiveReplaceTokensAndTags:

        ...
        // check if the inserttags have returned a simple token or an insert tag to parse
        if ((strpos($strBuffer, '##') !== false || strpos($strBuffer, '{{') !== false) && $strBuffer != $strText) {
            $strBuffer = static::recursiveReplaceTokensAndTags($strBuffer, $arrTokens, $intTextFlags);
        }
        ...

Changing this to the following solved the Problem:

        ...
        // check if the inserttags have returned a simple token or an insert tag to parse
        if ((strpos($strBuffer, '##') !== false || strpos($strBuffer, '{{') !== false) && $strBuffer != str_replace($arrReplacement, $arrOriginal, $strText)) {
            $strBuffer = static::recursiveReplaceTokensAndTags($strBuffer, $arrTokens, $intTextFlags);
        }
        ...
binron commented 7 years ago

https://github.com/codefog/contao-haste/commit/05879ee4fed58575574f03341243cd1378149d36 solves the problem too, but recursiveReplaceTokensAndTags isn't recursive anymore

qzminski commented 7 years ago

@aschempp can you take a look at this?

aschempp commented 7 years ago

@Toflar rewrote the core functionality. As far as I know we're still replacing unknown tokens?

qzminski commented 7 years ago

I have just encountered this problem myself. @aschempp please fix it or bring back my fix:

https://github.com/codefog/contao-haste/commit/05879ee4fed58575574f03341243cd1378149d36

qzminski commented 7 years ago

@binron can you update the haste to 4.14.3 and see if the issue still occurs?