Repository: twigphp/Twig (php)

Issue:
Custom escapers should not be saved to static variable in `twig_escape_filter`
Custom escapers can be added to the Twig `Enviroment` by
```php
$twig->getExtension(EscaperExtension::class)->setEscaper(...);
```

Those escapers are then saved to a static variable in the `twig_escape_filter` function:
```php
static $escapers;

if (null === $escapers) {
    $escapers = $env->getExtension(EscaperExtension::class)->getEscapers();
}
```
From https://github.com/twigphp/Twig/blob/3.x/src/Extension/EscaperExtension.php#L390-L394.

This leads to problems if you have multiple Twig-Environments with different Escapers, because the filter function is used for all environments, and it will therefore use the custom escapers of the first environment that runs into that code path.

I'm not sure what the performance impacts will be if the "caching" in the static variable will be removed completely, maybe "caching" can be moved to the TwigEnvironment as an instance variable there, in that case the escaper config will only be cached per Environment.

As a matter of fact the `Twig_Tests_Extension_EscaperTest` will be broken if the `testUnknownCustomEscaper` case is executed before the `testCustomEscaper` case.

Pull request description:
Fix custom escapers when using multiple Twig environments
Closes #3668

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index ef0ea4c0..79bbd281 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,6 @@
 # 2.14.12 (2022-XX-XX)
 
- * n/a
+ * Fix custom escapers when using multiple Twig environments
 
 # 2.14.11 (2022-02-04)
 
diff --git a/src/Extension/EscaperExtension.php b/src/Extension/EscaperExtension.php
index eebdc3a4..19dfd7d1 100644
--- a/src/Extension/EscaperExtension.php
+++ b/src/Extension/EscaperExtension.php
@@ -392,20 +392,18 @@ function twig_escape_filter(Environment $env, $string, $strategy = 'html', $char
             return rawurlencode($string);
 
         default:
-            static $escapers;
-
-            if (null === $escapers) {
-                // merge the ones set on CoreExtension for BC (to be removed in 3.0)
-                $escapers = array_merge(
-                    $env->getExtension(CoreExtension::class)->getEscapers(false),
-                    $env->getExtension(EscaperExtension::class)->getEscapers()
-                );
+            // check the ones set on CoreExtension for BC (to be removed in 3.0)
+            $legacyEscapers = $env->getExtension(CoreExtension::class)->getEscapers(false);
+            if (array_key_exists($strategy, $legacyEscapers)) {
+                return $legacyEscapers[$strategy]($env, $string, $charset);
             }
 
-            if (isset($escapers[$strategy])) {
+            $escapers = $env->getExtension(EscaperExtension::class)->getEscapers();
+            if (array_key_exists($strategy, $escapers)) {
                 return $escapers[$strategy]($env, $string, $charset);
             }
 
+            $escapers = array_merge($legacyEscapers, $escapers);
             $validStrategies = implode(', ', array_merge(['html', 'js', 'url', 'css', 'html_attr'], array_keys($escapers)));
 
             throw new RuntimeError(sprintf('Invalid escaping strategy "%s" (valid ones: %s).', $strategy, $validStrategies));


Test patch:
diff --git a/tests/Extension/EscaperTest.php b/tests/Extension/EscaperTest.php
index cf62dd1e..9804feaa 100644
--- a/tests/Extension/EscaperTest.php
+++ b/tests/Extension/EscaperTest.php
@@ -355,6 +355,13 @@ class Twig_Tests_Extension_EscaperTest extends TestCase
         }
     }
 
+    public function testUnknownCustomEscaper()
+    {
+        $this->expectException(RuntimeError::class);
+
+        twig_escape_filter(new Environment($this->createMock(LoaderInterface::class)), 'foo', 'bar');
+    }
+
     /**
      * @dataProvider provideCustomEscaperCases
      */
@@ -375,11 +382,15 @@ class Twig_Tests_Extension_EscaperTest extends TestCase
         ];
     }
 
-    public function testUnknownCustomEscaper()
+    public function testCustomEscapersOnMultipleEnvs()
     {
-        $this->expectException(RuntimeError::class);
+        $env1 = new Environment($this->createMock(LoaderInterface::class));
+        $env1->getExtension(EscaperExtension::class)->setEscaper('foo', 'Twig\Tests\foo_escaper_for_test');
+        $env2 = new Environment($this->createMock(LoaderInterface::class));
+        $env2->getExtension(EscaperExtension::class)->setEscaper('foo', 'Twig\Tests\foo_escaper_for_test1');
 
-        twig_escape_filter(new Environment($this->createMock(LoaderInterface::class)), 'foo', 'bar');
+        $this->assertSame('fooUTF-8', twig_escape_filter($env1, 'foo', 'foo'));
+        $this->assertSame('fooUTF-81', twig_escape_filter($env2, 'foo', 'foo'));
     }
 
     /**
@@ -410,6 +421,11 @@ function foo_escaper_for_test(Environment $twig, $string, $charset)
     return $string.$charset;
 }
 
+function foo_escaper_for_test1(Environment $twig, $string, $charset)
+{
+    return $string.$charset.'1';
+}
+
 interface Extension_SafeHtmlInterface
 {
 }