twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.12k stars 1.24k forks source link

Possible BC break with `EscaperExtension` in `3.10.x` #4087

Closed fritzmg closed 3 months ago

fritzmg commented 3 months ago

In 3.9.x the escaper callback was called with 3 arguments:

https://github.com/twigphp/Twig/blob/a842d75fed59cdbcbd3a3ad7fb9eb768fc350d58/src/Extension/EscaperExtension.php#L408

However, in 3.10.1 (and probably 3.10.0) it is only called with 2 arguments:

https://github.com/twigphp/Twig/blob/3af5ab2e52279e5e23dc192b1a26db3b8cffa4e7/src/Extension/EscaperExtension.php#L135

Resulting in an error like this for example:

ArgumentCountError:
Too few arguments to function Contao\CoreBundle\Twig\Interop\ContaoEscaper::escapeHtml(), 2 passed in vendor\twig\twig\src\Extension\EscaperExtension.php on line 135 and exactly 3 expected

  at vendor\contao\contao\core-bundle\src\Twig\Interop\ContaoEscaper.php:38
  at Contao\CoreBundle\Twig\Interop\ContaoEscaper->escapeHtml(object(Environment), 'Main navigation')
     (vendor\twig\twig\src\Extension\EscaperExtension.php:135)
  at Twig\Extension\EscaperExtension->Twig\Extension\{closure}('Main navigation', 'UTF-8')
     (vendor\twig\twig\src\Runtime\EscaperRuntime.php:311)

for custom escapers when updating from 3.9 to 3.10.

fritzmg commented 3 months ago

Restoring the previous behaviour should be fairly simple though: https://github.com/twigphp/Twig/pull/4088

Levdbas commented 3 months ago

I kind of miss an example on how to use the new EscaperRuntime class instead of EscaperExtension. The documentation I found is not up-to-date.

My code:

$escaper_extension = $twig->getExtension('Twig\Runtime\EscaperRuntime');
$escaper_extension->setEscaper('esc_url', $esc_url);

Throws: Twig\Error\RuntimeError: The "Twig\Runtime\EscaperRuntime" extension is not enabled.

Not sure that EscaperRuntime is even an extension?

fabpot commented 3 months ago

@Levdbas I've updated the docs in #4091. You can also find more information about deprecations and how to update the code in the doc/deprecated.rst doc.

In your code, it should be getRuntime() instead of getExtension().

fritzmg commented 3 months ago

@Levdbas @fabpot this has nothing to do with the original issue though.

xabbuh commented 3 months ago

@fritzmg Doesn't the change in https://github.com/twigphp/Twig/pull/4091/files#diff-6fca80088d7f7eeccac5ad9e3f9c7b689944fad09f04368da524a227d771c951R135 fix your issue?

fritzmg commented 3 months ago

It does, yes. I meant the documentation issue is not relevant to this issue. I only noticed later that the charset fix was also incorporated into the PR for the documentation.