[ticket/10322] Separate template varref resolution from output generation

Most template variables can now have their PHP variable name resolved instead of only compiling directly. This allows for the use of block vars in INCLUDE statements. This does not work for language variables since they require multiple checks. Added tests for the new types of allowed INCLUDEs. PHPBB3-10322
author: Patrick Webster <noxwizard@phpbb.com> 2011-09-16 01:46:42 -0500
committer: Patrick Webster <noxwizard@phpbb.com> 2011-09-16 01:46:42 -0500
commit: b5a79009cebe926058b745621158a105a45648a1 (patch)
tree: 61dfbfc01a97e0dee0dd409acbb7f25f9722fc7b
parent: b5ecb2f7a84fc53185572e2a603e8c49fdc85c7a (diff)
download: forums-b5a79009cebe926058b745621158a105a45648a1.tar
forums-b5a79009cebe926058b745621158a105a45648a1.tar.gz
forums-b5a79009cebe926058b745621158a105a45648a1.tar.bz2
forums-b5a79009cebe926058b745621158a105a45648a1.tar.xz
forums-b5a79009cebe926058b745621158a105a45648a1.zip
8 files changed, 75 insertions, 41 deletions
diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php
index 47394cf9ff..f24c3f4d09 100644
--- a/phpBB/includes/template/filter.php
+++ b/phpBB/includes/template/filter.php
@@ -273,26 +273,7 @@ class phpbb_template_filter extends php_user_filter
 			break;
 
 			case 'INCLUDE':
-				// Dynamic includes
-				// Cheap match rather than a full blown regexp, we already know
-				// the format of the input so just use string manipulation.
-				$temp = $matches[2];
-				if ($temp[0] == '{')
-				{
-					$file = false;
-
-					if ($temp[1] == '$')
-					{
-						$var = substr($temp, 2, -1);
-						$temp = "\$_tpldata['DEFINE']['.']['$var']";
-					}
-					else
-					{
-						$var = substr($temp, 1, -1);
-						$temp = "\$_rootref['$var']";
-					}
-				}
-				return '<?php ' . $this->compile_tag_include($temp) . ' ?>';
+				return '<?php ' . $this->compile_tag_include($matches[2]) . ' ?>';
 			break;
 
 			case 'INCLUDEPHP':
@@ -326,12 +307,13 @@ class phpbb_template_filter extends php_user_filter
 	}
 
 	/**
-	* Compile variables
+	* Convert template variables into PHP varrefs
 	*
 	* @param string $text_blocks Variable reference in source template
-	* @return string compiled template code
+	* @param bool $is_expr Returns whether the source was an expression type variable (i.e. S_FIRST_ROW)
+	* @return string PHP variable name
 	*/
-	private function compile_var_tags(&$text_blocks)
+	private function get_varref($text_blocks, &$is_expr)
 	{
 		// change template varrefs into PHP varrefs
 		$varrefs = array();
@@ -343,17 +325,38 @@ class phpbb_template_filter extends php_user_filter
 		{
 			$namespace = $var_val[1];
 			$varname = $var_val[3];
-			$new = $this->generate_block_varref($namespace, $varname, true, $var_val[2]);
+			$new = $this->generate_block_varref($namespace, $varname, $is_expr, $var_val[2]);
 
 			$text_blocks = str_replace($var_val[0], $new, $text_blocks);
 		}
 
-		// Handle special language tags L_ and LA_
-		$this->compile_language_tags($text_blocks);
+		// Language variables cannot be reduced to a single varref, so they must be skipped
+		// These two replacements would break language variables, so we can only run them on non-language types
+		if (strpos($text_blocks, '{L_') === false && strpos($text_blocks, '{LA_') === false)
+		{
+			// This will handle the remaining root-level varrefs
+			$text_blocks = preg_replace('#\{(' . self::REGEX_VAR . ')\}#', "\$_rootref['\\1']", $text_blocks);
+			$text_blocks = preg_replace('#\{\$(' . self::REGEX_VAR . ')\}#', "\$_tpldata['DEFINE']['.']['\\1']", $text_blocks);
+		}
 
-		// This will handle the remaining root-level varrefs
-		$text_blocks = preg_replace('#\{(' . self::REGEX_VAR . ')\}#', "<?php echo (isset(\$_rootref['\\1'])) ? \$_rootref['\\1'] : ''; /**/?>", $text_blocks);
-		$text_blocks = preg_replace('#\{\$(' . self::REGEX_VAR . ')\}#', "<?php echo (isset(\$_tpldata['DEFINE']['.']['\\1'])) ? \$_tpldata['DEFINE']['.']['\\1'] : ''; /**/?>", $text_blocks);
+		return $text_blocks;
+	}
+
+	/**
+	* Compile variables
+	*
+	* @param string $text_blocks Variable reference in source template
+	* @return string compiled template code
+	*/
+	private function compile_var_tags(&$text_blocks)
+	{
+		$text_blocks = $this->get_varref($text_blocks, $is_expr);
+		$lang_replaced = $this->compile_language_tags($text_blocks);
+
+		if(!$lang_replaced)
+		{
+			$text_blocks = '<?php echo ' . ($is_expr ? "$text_blocks" : "(isset($text_blocks)) ? $text_blocks : ''") . '; /**/?>';
+		}
 
 		return $text_blocks;
 	}
@@ -362,21 +365,28 @@ class phpbb_template_filter extends php_user_filter
 	* Handles special language tags L_ and LA_
 	*
 	* @param string $text_blocks Variable reference in source template
+	* @return bool Whether a replacement occurred or not
 	*/
 	private function compile_language_tags(&$text_blocks)
 	{
+		$replacements = 0;
+
 		// transform vars prefixed by L_ into their language variable pendant if nothing is set within the tpldata array
 		if (strpos($text_blocks, '{L_') !== false)
 		{
-			$text_blocks = preg_replace('#\{L_(' . self::REGEX_VAR . ')\}#', "<?php echo ((isset(\$_rootref['L_\\1'])) ? \$_rootref['L_\\1'] : ((isset(\$_lang['\\1'])) ? \$_lang['\\1'] : '{ \\1 }')); /**/?>", $text_blocks);
+			$text_blocks = preg_replace('#\{L_(' . self::REGEX_VAR . ')\}#', "<?php echo ((isset(\$_rootref['L_\\1'])) ? \$_rootref['L_\\1'] : ((isset(\$_lang['\\1'])) ? \$_lang['\\1'] : '{ \\1 }')); /**/?>", $text_blocks, -1, $replacements);
+			return (bool) $replacements;
 		}
 
 		// Handle addslashed language variables prefixed with LA_
 		// If a template variable already exist, it will be used in favor of it...
 		if (strpos($text_blocks, '{LA_') !== false)
 		{
-			$text_blocks = preg_replace('#\{LA_(' . self::REGEX_VAR . '+)\}#', "<?php echo ((isset(\$_rootref['LA_\\1'])) ? \$_rootref['LA_\\1'] : ((isset(\$_rootref['L_\\1'])) ? addslashes(\$_rootref['L_\\1']) : ((isset(\$_lang['\\1'])) ? addslashes(\$_lang['\\1']) : '{ \\1 }'))); /**/?>", $text_blocks);
+			$text_blocks = preg_replace('#\{LA_(' . self::REGEX_VAR . '+)\}#', "<?php echo ((isset(\$_rootref['LA_\\1'])) ? \$_rootref['LA_\\1'] : ((isset(\$_rootref['L_\\1'])) ? addslashes(\$_rootref['L_\\1']) : ((isset(\$_lang['\\1'])) ? addslashes(\$_lang['\\1']) : '{ \\1 }'))); /**/?>", $text_blocks, -1, $replacements);
+			return (bool) $replacements;
 		}
+
+		return false;
 	}
 
 	/**
@@ -745,9 +755,15 @@ class phpbb_template_filter extends php_user_filter
 	private function compile_tag_include($tag_args)
 	{
 		// Process dynamic includes
-		if ($tag_args[0] == '$')
+		if ($tag_args[0] == '{')
 		{
-			return "if (isset($tag_args)) { \$_template->_tpl_include($tag_args); }";
+			$var = $this->get_varref($tag_args, $is_expr);
+
+			// Make sure someone didn't try to include S_FIRST_ROW or similar
+			if (!$is_expr)
+			{
+				return "if (isset($var)) { \$_template->_tpl_include($var); }";
+			}
 		}
 
 		return "\$_template->_tpl_include('$tag_args');";
@@ -847,17 +863,16 @@ class phpbb_template_filter extends php_user_filter
 	*
 	* @param string $namespace Namespace to access (expects a trailing "." on the namespace)
 	* @param string $varname Variable name to use
-	* @param bool $echo If true return an echo statement, otherwise a reference to the internal variable
+	* @param bool $expr Returns whether the source was an expression type
 	* @param bool $defop If true this is a variable created with the DEFINE construct, otherwise template variable
 	* @return string Code to access variable or echo it if $echo is true
 	*/
-	private function generate_block_varref($namespace, $varname, $echo = true, $defop = false)
+	private function generate_block_varref($namespace, $varname, &$expr, $defop = false)
 	{
 		// Strip the trailing period.
 		$namespace = substr($namespace, 0, -1);
 
 		$expr = true;
-		$isset = false;
 
 		// S_ROW_COUNT is deceptive, it returns the current row number now the number of rows
 		// hence S_ROW_COUNT is deprecated in favour of S_ROW_NUM
@@ -893,11 +908,9 @@ class phpbb_template_filter extends php_user_filter
 				$varref .= "['$varname']";
 
 				$expr = false;
-				$isset = true;
 			break;
 		}
 		// @todo Test the !$expr more
-		$varref = ($echo) ? '<?php echo ' . ($isset ? "isset($varref) ? $varref : ''" : $varref) . '; /**/?>' : (($expr || isset($varref)) ? $varref : '');
 
 		return $varref;
 	}
diff --git a/tests/template/template_test.php b/tests/template/template_test.php
index a200148a29..28eba05217 100644
--- a/tests/template/template_test.php
+++ b/tests/template/template_test.php
@@ -163,11 +163,25 @@ class phpbb_template_template_test extends phpbb_template_template_test_case
 				'value',
 			),
 			array(
+				'include_define.html',
+				array('VARIABLE' => 'value'),
+				array(),
+				array(),
+				'value',
+			),
+			array(
+				'include_loop.html',
+				array(),
+				array('loop' => array(array('NESTED_FILE' => 'include_loop1.html')), 'loop.inner' => array(array('NESTED_FILE' => 'include_loop1.html'), array('NESTED_FILE' => 'include_loop2.html'), array('NESTED_FILE' => 'include_loop3.html'))),
+				array(),
+				"1\n_1\n_02\n_3",
+			),
+			array(
 				'include_variable.html',
 				array('FILE' => 'variable.html', 'VARIABLE' => 'value'),
 				array(),
 				array(),
-				"value\nvalue",
+				'value',
 			),
 			array(
 				'loop_vars.html',
diff --git a/tests/template/templates/include_define.html b/tests/template/templates/include_define.html
new file mode 100644
index 0000000000..2419c8cba1
--- /dev/null
+++ b/tests/template/templates/include_define.html
@@ -0,0 +1,2 @@
+<!-- DEFINE $DEF = 'variable.html' -->
+<!-- INCLUDE {$DEF} -->
diff --git a/tests/template/templates/include_loop.html b/tests/template/templates/include_loop.html
new file mode 100644
index 0000000000..d5c3d9bc82
--- /dev/null
+++ b/tests/template/templates/include_loop.html
@@ -0,0 +1,4 @@
+<!-- BEGIN loop -->
+<!-- INCLUDE {loop.NESTED_FILE} -->
+<!-- BEGIN inner -->_<!-- INCLUDE {inner.NESTED_FILE} --><!-- END inner -->
+<!-- END loop -->
diff --git a/tests/template/templates/include_loop1.html b/tests/template/templates/include_loop1.html
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/template/templates/include_loop1.html
@@ -0,0 +1 @@
+1
diff --git a/tests/template/templates/include_loop2.html b/tests/template/templates/include_loop2.html
new file mode 100644
index 0000000000..9e22bcb8e3
--- /dev/null
+++ b/tests/template/templates/include_loop2.html
@@ -0,0 +1 @@
+02
diff --git a/tests/template/templates/include_loop3.html b/tests/template/templates/include_loop3.html
new file mode 100644
index 0000000000..00750edc07
--- /dev/null
+++ b/tests/template/templates/include_loop3.html
@@ -0,0 +1 @@
+3
diff --git a/tests/template/templates/include_variable.html b/tests/template/templates/include_variable.html
index a126372195..b907e0b44f 100644
--- a/tests/template/templates/include_variable.html
+++ b/tests/template/templates/include_variable.html
@@ -1,3 +1 @@
 <!-- INCLUDE {FILE} -->
-<!-- DEFINE $DEF = 'variable.html' -->
-<!-- INCLUDE {$DEF} -->
-\ No newline at end of file
author	Patrick Webster <noxwizard@phpbb.com>	2011-09-16 01:46:42 -0500
committer	Patrick Webster <noxwizard@phpbb.com>	2011-09-16 01:46:42 -0500
commit	b5a79009cebe926058b745621158a105a45648a1 (patch)
tree	61dfbfc01a97e0dee0dd409acbb7f25f9722fc7b
parent	b5ecb2f7a84fc53185572e2a603e8c49fdc85c7a (diff)
download	forums-b5a79009cebe926058b745621158a105a45648a1.tar forums-b5a79009cebe926058b745621158a105a45648a1.tar.gz forums-b5a79009cebe926058b745621158a105a45648a1.tar.bz2 forums-b5a79009cebe926058b745621158a105a45648a1.tar.xz forums-b5a79009cebe926058b745621158a105a45648a1.zip