From 8ea68e06e4b96e8f703fb636a20ee60cd0a8eddc Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 6 Apr 2019 14:02:52 +0200 Subject: [PATCH] fixed "include" with "ignore missing" when an error loading occurs in the included template (again) --- src/Environment.php | 18 +------------ src/Extension/CoreExtension.php | 12 +++++++-- src/Node/ModuleNode.php | 21 +++++++-------- test/Twig/Tests/EnvironmentTest.php | 27 ------------------- .../Fixtures/exceptions/undefined_parent.test | 2 ++ .../Tests/Fixtures/expressions/floats.test | 2 +- .../include/include_missing_extends.test | 13 +++++++++ .../tags/include/include_missing_extends.test | 13 +++++++++ test/Twig/Tests/Node/ModuleTest.php | 6 ++--- 9 files changed, 53 insertions(+), 61 deletions(-) create mode 100644 test/Twig/Tests/Fixtures/functions/include/include_missing_extends.test create mode 100644 test/Twig/Tests/Fixtures/tags/include/include_missing_extends.test diff --git a/src/Environment.php b/src/Environment.php index 78863a04f..224ecf856 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -83,7 +83,6 @@ class Environment private $runtimeLoaders = []; private $runtimes = []; private $optionsHash; - private $loading = []; /** * Constructor. @@ -507,22 +506,7 @@ class Environment $this->initRuntime(); } - if (isset($this->loading[$cls])) { - throw new RuntimeError(sprintf('Circular reference detected for Twig template "%s", path: %s.', $name, implode(' -> ', array_merge($this->loading, [$name])))); - } - - $this->loading[$cls] = $name; - - try { - $this->loadedTemplates[$cls] = new $cls($this); - unset($this->loading[$cls]); - } catch (\Exception $e) { - unset($this->loading[$cls]); - - throw $e; - } - - return $this->loadedTemplates[$cls]; + return $this->loadedTemplates[$cls] = new $cls($this); } /** diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php index 73f118e3b..b0ea66942 100644 --- a/src/Extension/CoreExtension.php +++ b/src/Extension/CoreExtension.php @@ -1570,12 +1570,20 @@ function twig_include(Environment $env, $context, $template, $variables = [], $w } try { - return $loaded ? $loaded->render($variables) : ''; - } finally { + $ret = $loaded ? $loaded->render($variables) : ''; + } catch (\Exception $e) { if ($isSandboxed && !$alreadySandboxed) { $sandbox->disableSandbox(); } + + throw $e; } + + if ($isSandboxed && !$alreadySandboxed) { + $sandbox->disableSandbox(); + } + + return $ret; } /** diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index 2eb0d49dc..665f93147 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -198,17 +198,6 @@ class ModuleNode extends Node // parent if (!$this->hasNode('parent')) { $compiler->write("\$this->parent = false;\n\n"); - } elseif (($parent = $this->getNode('parent')) && $parent instanceof ConstantExpression) { - $compiler - ->addDebugInfo($parent) - ->write('$this->parent = $this->loadTemplate(') - ->subcompile($parent) - ->raw(', ') - ->repr($this->source->getName()) - ->raw(', ') - ->repr($parent->getTemplateLine()) - ->raw(");\n") - ; } $countTraits = \count($this->getNode('traits')); @@ -331,8 +320,18 @@ class ModuleNode extends Node if ($this->hasNode('parent')) { $parent = $this->getNode('parent'); + $compiler->addDebugInfo($parent); if ($parent instanceof ConstantExpression) { + $compiler + ->write('$this->parent = $this->loadTemplate(') + ->subcompile($parent) + ->raw(', ') + ->repr($this->source->getName()) + ->raw(', ') + ->repr($parent->getTemplateLine()) + ->raw(");\n") + ; $compiler->write('$this->parent'); } else { $compiler->write('$this->getParent($context)'); diff --git a/test/Twig/Tests/EnvironmentTest.php b/test/Twig/Tests/EnvironmentTest.php index a33b8550a..88fd15cfa 100644 --- a/test/Twig/Tests/EnvironmentTest.php +++ b/test/Twig/Tests/EnvironmentTest.php @@ -498,33 +498,6 @@ EOF $this->assertEquals('foo', $twig->render('func_string_named_args')); } - /** - * @expectedException \Twig\Error\RuntimeError - * @expectedExceptionMessage Circular reference detected for Twig template "base.html.twig", path: base.html.twig -> base.html.twig in "base.html.twig" at line 1 - */ - public function testFailLoadTemplateOnCircularReference() - { - $twig = new Environment(new ArrayLoader([ - 'base.html.twig' => '{% extends "base.html.twig" %}', - ])); - - $twig->load('base.html.twig'); - } - - /** - * @expectedException \Twig\Error\RuntimeError - * @expectedExceptionMessage Circular reference detected for Twig template "base1.html.twig", path: base1.html.twig -> base2.html.twig -> base1.html.twig in "base1.html.twig" at line 1 - */ - public function testFailLoadTemplateOnComplexCircularReference() - { - $twig = new Environment(new ArrayLoader([ - 'base1.html.twig' => '{% extends "base2.html.twig" %}', - 'base2.html.twig' => '{% extends "base1.html.twig" %}', - ])); - - $twig->load('base1.html.twig'); - } - protected function getMockLoader($templateName, $templateContent) { // to be removed in 2.0 diff --git a/test/Twig/Tests/Fixtures/exceptions/undefined_parent.test b/test/Twig/Tests/Fixtures/exceptions/undefined_parent.test index 566ee7219..07f855a3f 100644 --- a/test/Twig/Tests/Fixtures/exceptions/undefined_parent.test +++ b/test/Twig/Tests/Fixtures/exceptions/undefined_parent.test @@ -4,5 +4,7 @@ Exception for an undefined parent {% extends 'foo.html' %} {% set foo = "foo" %} +--DATA-- +return [] --EXCEPTION-- Twig\Error\LoaderError: Template "foo.html" is not defined in "index.twig" at line 2. diff --git a/test/Twig/Tests/Fixtures/expressions/floats.test b/test/Twig/Tests/Fixtures/expressions/floats.test index cf563a032..cdf871cde 100644 --- a/test/Twig/Tests/Fixtures/expressions/floats.test +++ b/test/Twig/Tests/Fixtures/expressions/floats.test @@ -9,7 +9,7 @@ version_compare(phpversion(), '7.0.0', '>=') {{ val2 is same as (0.0) ? 'Yes' : 'No' }} {{ val is same as (val2) ? 'Yes' : 'No' }} --DATA-- -return array('val' => 0.0) +return ['val' => 0.0] --EXPECT-- Yes Yes diff --git a/test/Twig/Tests/Fixtures/functions/include/include_missing_extends.test b/test/Twig/Tests/Fixtures/functions/include/include_missing_extends.test new file mode 100644 index 000000000..810ae8248 --- /dev/null +++ b/test/Twig/Tests/Fixtures/functions/include/include_missing_extends.test @@ -0,0 +1,13 @@ +--TEST-- +"include" function +--TEMPLATE-- +{{ include(['bad.twig', 'good.twig'], ignore_missing = true) }} +NOT DISPLAYED +--TEMPLATE(bad.twig)-- +{% extends 'DOES NOT EXIST' %} +--TEMPLATE(good.twig)-- +NOT DISPLAYED +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\LoaderError: Template "DOES NOT EXIST" is not defined in "bad.twig" at line 2. diff --git a/test/Twig/Tests/Fixtures/tags/include/include_missing_extends.test b/test/Twig/Tests/Fixtures/tags/include/include_missing_extends.test new file mode 100644 index 000000000..d0d1bfe59 --- /dev/null +++ b/test/Twig/Tests/Fixtures/tags/include/include_missing_extends.test @@ -0,0 +1,13 @@ +--TEST-- +"include" tag +--TEMPLATE-- +{% include ['bad.twig', 'good.twig'] ignore missing %} +NOT DISPLAYED +--TEMPLATE(bad.twig)-- +{% extends 'DOES NOT EXIST' %} +--TEMPLATE(good.twig)-- +NOT DISPLAYED +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\LoaderError: Template "DOES NOT EXIST" is not defined in "bad.twig" at line 2. diff --git a/test/Twig/Tests/Node/ModuleTest.php b/test/Twig/Tests/Node/ModuleTest.php index 875bea204..213350a2c 100644 --- a/test/Twig/Tests/Node/ModuleTest.php +++ b/test/Twig/Tests/Node/ModuleTest.php @@ -140,14 +140,13 @@ class __TwigTemplate_%x extends \Twig\Template { parent::__construct(\$env); - // line 1 - \$this->parent = \$this->loadTemplate("layout.twig", "foo.twig", 1); \$this->blocks = [ ]; } protected function doGetParent(array \$context) { + // line 1 return "layout.twig"; } @@ -156,6 +155,7 @@ class __TwigTemplate_%x extends \Twig\Template // line 2 \$context["macro"] = \$this->loadTemplate("foo.twig", "foo.twig", 2); // line 1 + \$this->parent = \$this->loadTemplate("layout.twig", "foo.twig", 1); \$this->parent->display(\$context, array_merge(\$this->blocks, \$blocks)); } @@ -171,7 +171,7 @@ class __TwigTemplate_%x extends \Twig\Template public function getDebugInfo() { - return array ( 37 => 1, 35 => 2, 22 => 1,); + return array ( 36 => 1, 34 => 2, 28 => 1,); } /** @deprecated since 1.27 (to be removed in 2.0). Use getSourceContext() instead */