mirror of
https://github.com/twigphp/Twig.git
synced 2026-09-12 10:26:32 +00:00
merged branch Seldaek/inception (PR #610)
Commits -------0d656f5Add comments0255764Updated CHANGELOGb327a48Protect the Parser against recursive parsing issues Discussion ---------- Make the parser Inception-Proof Spent half a day debugging before I realized what happened, but I'll try to keep a long story short: When the cache is empty, and the first template containing an assetic `{% javascripts %}` or similar tag is parsed, it will build up the assetic "assets" or recipes cache, this in turn will tokenize and parse all your templates to find assetic tags and cache that information. At this point the parser is parsing something else in the middle of a parse() call, and since there is a single instance in the environment, it means all the instance vars are messed up and contain incorrect references to the latest TokenStream that was parsed by assetic, etc. This had two effects on my application, both appearing seemingly randomly because it highly depends on the order of things, the state of your cache and probably other factors: - The first thing that happened is that a template was compiled using the wrong template filename, which means I had a `__TwigTemplate_abcd` in the file named `dcba.php`, and it would never find the right class. - The second issue (could not reproduce but I assume it was caused by this as well) is that the parse tree is completely broken and you end up with a parse error because it thinks it's at the end when it's not, or similar problem. The proposed fix basically pushes/pops all the vars into a stack whenever the parser starts/stops, which worked very effectively here and does not introduce much breakage or complexity. --------------------------------------------------------------------------- by stof at 2012-01-24T17:28:06Z @Seldaek are you able to create a reproducible testcase for this (which should be failing before this fix) ? It would avoid further regressions --------------------------------------------------------------------------- by Seldaek at 2012-01-24T17:45:37Z I'll try to improve on this according to feedback tomorrow. I saw this enough for today :) --------------------------------------------------------------------------- by fabpot at 2012-01-24T17:45:49Z I'm writing some unit tests --------------------------------------------------------------------------- by Seldaek at 2012-01-24T17:47:02Z Ok then I'll add @dzuelke's comments real quick.
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
* fixed the empty test and the length filter for Twig_Markup instances
|
||||
* added a date function to ease date comparison
|
||||
* fixed unary operators precedence
|
||||
* added recursive parsing support in the parser
|
||||
|
||||
* 1.5.1 (2012-01-05)
|
||||
|
||||
|
||||
+14
-1
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
class Twig_Parser implements Twig_ParserInterface
|
||||
{
|
||||
protected $stack = array();
|
||||
protected $stream;
|
||||
protected $parent;
|
||||
protected $handlers;
|
||||
@@ -61,6 +62,11 @@ class Twig_Parser implements Twig_ParserInterface
|
||||
*/
|
||||
public function parse(Twig_TokenStream $stream)
|
||||
{
|
||||
// push all variables into the stack to keep the current state of the parser
|
||||
$vars = get_object_vars($this);
|
||||
unset($vars['stack'], $vars['env']);
|
||||
$this->stack[] = $vars;
|
||||
|
||||
$this->tmpVarCount = 0;
|
||||
|
||||
// tag handlers
|
||||
@@ -102,7 +108,14 @@ class Twig_Parser implements Twig_ParserInterface
|
||||
|
||||
$traverser = new Twig_NodeTraverser($this->env, $this->visitors);
|
||||
|
||||
return $traverser->traverse($node);
|
||||
$node = $traverser->traverse($node);
|
||||
|
||||
// restore previous stack so previous parse() call can resume working
|
||||
foreach (array_pop($this->stack) as $key => $val) {
|
||||
$this->$key = $val;
|
||||
}
|
||||
|
||||
return $node;
|
||||
}
|
||||
|
||||
public function subparse($test, $dropNeedle = false)
|
||||
|
||||
Reference in New Issue
Block a user