Twig icon indicating copy to clipboard operation
Twig copied to clipboard

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

Open fritzmg opened this issue 9 months ago • 1 comments

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

  • The environment $env
  • The string $string
  • The character set $charset

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:

  • The environment $env
  • The string $string

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 avatar May 12 '24 08:05 fritzmg

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

fritzmg avatar May 12 '24 08:05 fritzmg

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?

Levdbas avatar May 13 '24 10:05 Levdbas

@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().

fabpot avatar May 14 '24 05:05 fabpot

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

fritzmg avatar May 14 '24 07:05 fritzmg

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

xabbuh avatar May 14 '24 07:05 xabbuh

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.

fritzmg avatar May 14 '24 08:05 fritzmg