Repository: twigphp/Twig (php)

Issue:
Sandbox is not disabled if syntax error occurs within {% sandbox %} tag
I added test for that: https://github.com/twigphp/Twig/pull/3348

Pull request:
Fixes #3351
Sandbox mode is not disabled if syntax error occurs inside {% sandbox %} tag

Fixes #3351

Patch:
diff --git a/src/Node/SandboxNode.php b/src/Node/SandboxNode.php
index 2d644c3a..3dcd4d8f 100644
--- a/src/Node/SandboxNode.php
+++ b/src/Node/SandboxNode.php
@@ -34,12 +34,19 @@ class SandboxNode extends Node
             ->write("\$this->sandbox->enableSandbox();\n")
             ->outdent()
             ->write("}\n")
+            ->write("try {\n")
+            ->indent()
             ->subcompile($this->getNode('body'))
+            ->outdent()
+            ->write("} finally {\n")
+            ->indent()
             ->write("if (!\$alreadySandboxed) {\n")
             ->indent()
             ->write("\$this->sandbox->disableSandbox();\n")
             ->outdent()
             ->write("}\n")
+            ->outdent()
+            ->write("}\n")
         ;
     }
 }


Test patch:
diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
index 2fe692a2..8bc6475a 100644
--- a/tests/Extension/SandboxTest.php
+++ b/tests/Extension/SandboxTest.php
@@ -12,6 +12,7 @@ namespace Twig\Tests\Extension;
  */
 
 use Twig\Environment;
+use Twig\Error\SyntaxError;
 use Twig\Extension\SandboxExtension;
 use Twig\Loader\ArrayLoader;
 use Twig\Sandbox\SecurityError;
@@ -44,6 +45,8 @@ class SandboxTest extends \PHPUnit\Framework\TestCase
             '1_child' => "{% extends \"1_layout\" %}\n{% block content %}\n{{ \"a\"|json_encode }}\n{% endblock %}",
             '1_include' => '{{ include("1_basic1", sandboxed=true) }}',
             '1_range_operator' => '{{ (1..2)[0] }}',
+            '1_syntax_error_wrapper' => '{% sandbox %}{% include "1_syntax_error" %}{% endsandbox %}',
+            '1_syntax_error' => '{% syntax error }}'
         ];
     }
 
@@ -75,6 +78,18 @@ class SandboxTest extends \PHPUnit\Framework\TestCase
         }
     }
 
+    public function testIfSandBoxIsDisabledAfterSyntaxError()
+    {
+        $twig = $this->getEnvironment(false, [], self::$templates);
+        try {
+            $twig->load('1_syntax_error_wrapper')->render(self::$params);
+        } catch (SyntaxError $e) {
+            /** @var SandboxExtension $sandbox */
+            $sandbox = $twig->getExtension(SandboxExtension::class);
+            $this->assertFalse($sandbox->isSandboxed());
+        }
+    }
+
     public function testSandboxUnallowedFilter()
     {
         $twig = $this->getEnvironment(true, [], self::$templates);
diff --git a/tests/Node/SandboxTest.php b/tests/Node/SandboxTest.php
index b81781e8..7cbddd75 100644
--- a/tests/Node/SandboxTest.php
+++ b/tests/Node/SandboxTest.php
@@ -37,9 +37,12 @@ class SandboxTest extends NodeTestCase
 if (!\$alreadySandboxed = \$this->sandbox->isSandboxed()) {
     \$this->sandbox->enableSandbox();
 }
-echo "foo";
-if (!\$alreadySandboxed) {
-    \$this->sandbox->disableSandbox();
+try {
+    echo "foo";
+} finally {
+    if (!\$alreadySandboxed) {
+        \$this->sandbox->disableSandbox();
+    }
 }
 EOF
         ];