Fixed that scoped variables would overwrite parent scope. (#954)

* Fixed that scoped variables would overwrite parent scope.
Fixes #952

* Moved variable stack maintenance to methods and private properties in Data class.

changelog/952.md

@@ -0,0 +1,2 @@
+- Fixed that scoped variables would overwrite parent scope [#952](https://github.com/smarty-php/smarty/issues/952)
+- Removed publicly accessible `$tpl->_var_stack` variable

src/Compile/Tag/FunctionClose.php

@@ -75,7 +75,7 @@ class FunctionClose extends Base {
 			$output .= "foreach (\$params as \$key => \$value) {\n\$_smarty_tpl->assign(\$key, \$value);\n}\n";
 			$output .= "\$params = var_export(\$params, true);\n";
 			$output .= "echo \"/*%%SmartyNocache:{$compiler->getTemplate()->getCompiled()->nocache_hash}%%*/<?php ";
-			$output .= "\\\$_smarty_tpl->getSmarty()->getRuntime('TplFunction')->saveTemplateVariables(\\\$_smarty_tpl, '{$_name}');\nforeach (\$params as \\\$key => \\\$value) {\n\\\$_smarty_tpl->assign(\\\$key, \\\$value);\n}\n?>";
+			$output .= "\\\$_smarty_tpl->pushStack();\nforeach (\$params as \\\$key => \\\$value) {\n\\\$_smarty_tpl->assign(\\\$key, \\\$value);\n}\n?>";
 			$output .= "/*/%%SmartyNocache:{$compiler->getTemplate()->getCompiled()->nocache_hash}%%*/\";?>";
 			$compiler->getParser()->current_buffer->append_subtree(
 				$compiler->getParser(),
@@ -86,7 +86,7 @@ class FunctionClose extends Base {
 			);
 			$compiler->getParser()->current_buffer->append_subtree($compiler->getParser(), $_functionCode);
 			$output = "<?php echo \"/*%%SmartyNocache:{$compiler->getTemplate()->getCompiled()->nocache_hash}%%*/<?php ";
-			$output .= "\\\$_smarty_tpl->getSmarty()->getRuntime('TplFunction')->restoreTemplateVariables(\\\$_smarty_tpl, '{$_name}');?>\n";
+			$output .= "\\\$_smarty_tpl->popStack();?>\n";
 			$output .= "/*/%%SmartyNocache:{$compiler->getTemplate()->getCompiled()->nocache_hash}%%*/\";\n?>";
 			$output .= "<?php echo str_replace('{$compiler->getTemplate()->getCompiled()->nocache_hash}', \$_smarty_tpl->getCompiled()->nocache_hash ?? '', ob_get_clean());\n";
 			$output .= "}\n}\n";

src/Data.php

@@ -47,6 +47,20 @@ class Data
      */
     public $config_vars = array();
 
+	/**
+	 * This variable will hold a stack of template variables.
+	 *
+	 * @var null|array
+	 */
+	private $_var_stack = [];
+
+	/**
+	 * This variable will hold a stack of config variables.
+	 *
+	 * @var null|array
+	 */
+	private $_config_stack = [];
+
 	/**
 	 * Default scope for new variables
 	 * @var int
@@ -493,4 +507,20 @@ class Data
 	public function setParent($parent): void {
 		$this->parent = $parent;
 	}
+
+	public function pushStack(): void {
+		$stackList = [];
+		foreach ($this->tpl_vars as $name => $variable) {
+			$stackList[$name] = clone $variable; // variables are stored in Variable objects
+		}
+		$this->_var_stack[] = $this->tpl_vars;
+		$this->tpl_vars = $stackList;
+
+		$this->_config_stack[] = $this->config_vars;
+	}
+
+	public function popStack(): void {
+		$this->tpl_vars = array_pop($this->_var_stack);
+		$this->config_vars = array_pop($this->_config_stack);
+	}
 }

src/Runtime/TplFunctionRuntime.php

@@ -26,31 +26,26 @@ class TplFunctionRuntime {
 	 */
 	public function callTemplateFunction(Template $tpl, $name, $params, $nocache) {
 		$funcParam = $tpl->tplFunctions[$name] ?? ($tpl->getSmarty()->tplFunctions[$name] ?? null);
-		if (isset($funcParam)) {
-			if (!$tpl->caching || ($tpl->caching && $nocache)) {
-				$function = $funcParam['call_name'];
+		if (!isset($funcParam)) {
+			throw new \Smarty\Exception("Unable to find template function '{$name}'");
+		}
+
+		if (!$tpl->caching || ($tpl->caching && $nocache)) {
+			$function = $funcParam['call_name'];
+		} else {
+			if (isset($funcParam['call_name_caching'])) {
+				$function = $funcParam['call_name_caching'];
 			} else {
-				if (isset($funcParam['call_name_caching'])) {
-					$function = $funcParam['call_name_caching'];
-				} else {
-					$function = $funcParam['call_name'];
-				}
-			}
-			if (function_exists($function)) {
-				$this->saveTemplateVariables($tpl, $name);
-				$function($tpl, $params);
-				$this->restoreTemplateVariables($tpl, $name);
-				return;
-			}
-			// try to load template function dynamically
-			if ($this->addTplFuncToCache($tpl, $name, $function)) {
-				$this->saveTemplateVariables($tpl, $name);
-				$function($tpl, $params);
-				$this->restoreTemplateVariables($tpl, $name);
-				return;
+				$function = $funcParam['call_name'];
 			}
 		}
-		throw new \Smarty\Exception("Unable to find template function '{$name}'");
+		if (!function_exists($function) && !$this->addTplFuncToCache($tpl, $name, $function)) {
+			throw new \Smarty\Exception("Unable to find template function '{$name}'");
+		}
+
+		$tpl->pushStack();
+		$function($tpl, $params);
+		$tpl->popStack();
 	}
 
 	/**
@@ -146,28 +141,4 @@ class TplFunctionRuntime {
 		return false;
 	}
 
-	/**
-	 * Save current template variables on stack
-	 *
-	 * @param \Smarty\Template $tpl
-	 * @param string $name stack name
-	 */
-	public function saveTemplateVariables(Template $tpl, $name) {
-		$tpl->_var_stack[] =
-			['tpl' => $tpl->tpl_vars, 'config' => $tpl->config_vars, 'name' => "_tplFunction_{$name}"];
-	}
-
-	/**
-	 * Restore saved variables into template objects
-	 *
-	 * @param \Smarty\Template $tpl
-	 * @param string $name stack name
-	 */
-	public function restoreTemplateVariables(Template $tpl, $name) {
-		if (isset($tpl->_var_stack)) {
-			$vars = array_pop($tpl->_var_stack);
-			$tpl->tpl_vars = $vars['tpl'];
-			$tpl->config_vars = $vars['config'];
-		}
-	}
 }

src/Template.php

@@ -645,8 +645,7 @@ class Template extends TemplateBase {
 			} else {
 
 				// After rendering a template, the tpl/config variables are reset, so the template can be re-used.
-				$savedTplVars = $this->tpl_vars;
-				$savedConfigVars = $this->config_vars;
+				$this->pushStack();
 
 				// Start output-buffering.
 				ob_start();
@@ -654,8 +653,7 @@ class Template extends TemplateBase {
 				$result = $this->render(false, $function);
 
 				// Restore the template to its previous state
-				$this->tpl_vars = $savedTplVars;
-				$this->config_vars = $savedConfigVars;
+				$this->popStack();
 			}
 
 			if (isset($errorHandler)) {

src/TemplateBase.php

@@ -59,13 +59,6 @@ abstract class TemplateBase extends Data {
 	 */
 	public $tplFunctions = [];
 
-	/**
-	 * When initialized to an (empty) array, this variable will hold a stack of template variables.
-	 *
-	 * @var null|array
-	 */
-	public $_var_stack = null;
-
 	/**
 	 * @var Debug
 	 */

tests/UnitTests/TemplateSource/X_Scopes/ScopeTest.php

@@ -325,4 +325,13 @@ class ScopeTest extends PHPUnit_Smarty
         $this->smarty->assign('scope', 'none');
         $r = $this->smarty->fetch('test_function_scope.tpl');
     }
+
+	public function testFunctionScopeIsLocalByDefault()
+	{
+		$this->assertEquals(
+			'a',
+			$this->smarty->fetch('string:{function name=test}{$var="b"}{/function}{$var="a"}{test}{$var}')
+		);
+	}
+
 }

tests/UnitTests/__shared/PHPunitplugins/function.checkvar.php

@@ -29,12 +29,6 @@ function smarty_function_checkvar($params, \Smarty\Template $template)
         if (in_array('template', $types) && $ptr instanceof Template) {
             $output .= "#{$ptr->getSource()->name}:\${$var} =";
             $output .= $ptr->hasVariable($var) ? preg_replace('/\s/', '', var_export($ptr->getValue($var), true)) : '>unassigned<';
-            $i = 0;
-            while (isset($ptr->_var_stack[ $i ])) {
-                $output .= "#{$ptr->_var_stack[ $i ]['name']} = ";
-                $output .= isset($ptr->_var_stack[ $i ][ 'tpl' ][$var]) ? preg_replace('/\s/', '', var_export($ptr->_var_stack[ $i ][ 'tpl' ][$var]->value, true)) : '>unassigned<';
-                $i ++;
-            }
             $ptr = $ptr->parent;
         } elseif (in_array('data', $types) && !($ptr instanceof Template || $ptr instanceof \Smarty\Smarty)) {
             $output .= "#data:\${$var} =";