From d5896239068e1063970f398a12ffb5a0707fb286 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Sat, 7 Jul 2012 22:43:52 +0100 Subject: [PATCH 1/7] [ticket/10970] Paths of the form {FOO}/a/{BAR}/b parsed correctly A new method to handle this type of path was added and compile_tag_include, compile_tag_include_php and compile_tag_include_js were modified to use it appropriately. Tests were added for these three macros also. PHPBB3-10970 --- phpBB/includes/template/filter.php | 70 +++++++++++++++---- tests/template/includephp_test.php | 12 ++++ tests/template/template_includejs_test.php | 6 +- tests/template/template_test.php | 7 ++ .../template/templates/include_variables.html | 1 + tests/template/templates/includejs.html | 4 +- .../templates/includephp_variables.html | 2 + .../template/templates/subdir/parent_only.js | 0 .../templates/subdir/subsubdir/parent_only.js | 0 tests/template/templates/subdir/variable.html | 1 + 10 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 tests/template/templates/include_variables.html create mode 100644 tests/template/templates/includephp_variables.html create mode 100644 tests/template/templates/subdir/parent_only.js create mode 100644 tests/template/templates/subdir/subsubdir/parent_only.js create mode 100644 tests/template/templates/subdir/variable.html diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index ad2e35de6a..a8985a771c 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -361,6 +361,52 @@ class phpbb_template_filter extends php_user_filter return $text_blocks; } + /** + * Parse paths of the form {FOO}/a/{BAR}/b + * Note: this method assumes at least one variable in the path, this should + * be checked before this method is called. + * + * @param string $path The path to parse + * @param string $include_type The type of template function to call + * @return string An appropriately formatted string to include in the + * template + */ + private function parse_dynamic_path($path, $include_type) + { + $segments = explode('/', $path); + $is_expr = true; + $str = array(); + $vars = array(); + + foreach ($segments as $segment) + { + if ($segment[0] === '{') + { + $var = $this->get_varref($segment, $tmp_is_expr); + $is_expr = $is_expr && $tmp_is_expr; + $vars[] = "isset($var)"; + $str[] = "$var . '/'"; + } + else + { + $str[] = "'$segment/'"; + } + } + + // Remove trailing slash from last element + $last = array_pop($str); + $str[] = str_replace('/', '', $last); + + if (!$is_expr) + { + return ' if (' . implode(' && ', $vars) . ") { \$_template->$include_type(" . implode(' . ', $str) . ', true); }'; + } + else + { + return ''; + } + } + /** * Compile variables * @@ -774,15 +820,9 @@ class phpbb_template_filter extends php_user_filter private function compile_tag_include($tag_args) { // Process dynamic includes - if ($tag_args[0] == '{') + if (strpos($tag_args, '{') !== false) { - $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 $this->parse_dynamic_path($tag_args, '_tpl_include'); } return "\$_template->_tpl_include('$tag_args');"; @@ -796,6 +836,11 @@ class phpbb_template_filter extends php_user_filter */ private function compile_tag_include_php($tag_args) { + if (strpos($tag_args, '{') !== false) + { + return $this->parse_dynamic_path($tag_args, '_php_include'); + } + return "\$_template->_php_include('$tag_args');"; } @@ -883,14 +928,9 @@ class phpbb_template_filter extends php_user_filter private function compile_tag_include_js($tag_args) { // Process dynamic includes - if ($tag_args[0] == '{') + if (strpos($tag_args, '{') !== false) { - $var = $this->get_varref($tag_args, $is_expr); - if (!$is_expr) - { - return " \$_template->_js_include($var, true);"; - } - return ''; + return $this->parse_dynamic_path($tag_args, '_js_include'); } // Locate file diff --git a/tests/template/includephp_test.php b/tests/template/includephp_test.php index 626735f15f..236621adec 100644 --- a/tests/template/includephp_test.php +++ b/tests/template/includephp_test.php @@ -23,6 +23,18 @@ class phpbb_template_includephp_test extends phpbb_template_template_test_case $this->assertEquals("Path is relative to board root.\ntesting included php", $this->display('test'), "Testing INCLUDEPHP"); } + public function test_includephp_variables() + { + $this->setup_engine(array('tpl_allow_php' => true)); + + $cache_file = $this->template->cachepath . 'includephp_variables.html.php'; + + $this->run_template('includephp_variables.html', array('TEMPLATES' => 'templates'), array(), array(), "Path includes variables.\ntesting included php", $cache_file); + + $this->template->set_filenames(array('test' => 'includephp_variables.html')); + $this->assertEquals("Path includes variables.\ntesting included php", $this->display('test'), "Testing INCLUDEPHP"); + } + public function test_includephp_absolute() { $path_to_php = dirname(__FILE__) . '/templates/_dummy_include.php.inc'; diff --git a/tests/template/template_includejs_test.php b/tests/template/template_includejs_test.php index a8f9a9037f..d8c7170e94 100644 --- a/tests/template/template_includejs_test.php +++ b/tests/template/template_includejs_test.php @@ -20,11 +20,13 @@ class phpbb_template_template_includejs_test extends phpbb_template_template_tes $scripts = array( '', '', - '' + '', + '', + '', ); // Run test $cache_file = $this->template->cachepath . 'includejs.html.php'; - $this->run_template('includejs.html', array('PARENT' => 'parent_only.js'), array(), array(), implode('', $scripts), $cache_file); + $this->run_template('includejs.html', array('PARENT' => 'parent_only.js', 'SUBDIR' => 'subdir'), array(), array(), implode('', $scripts), $cache_file); } } diff --git a/tests/template/template_test.php b/tests/template/template_test.php index f8677ed913..83995cb4ac 100644 --- a/tests/template/template_test.php +++ b/tests/template/template_test.php @@ -183,6 +183,13 @@ class phpbb_template_template_test extends phpbb_template_template_test_case array(), 'value', ), + array( + 'include_variables.html', + array('SUBDIR' => 'subdir', 'VARIABLE' => 'value'), + array(), + array(), + 'value', + ), array( 'loop_vars.html', array(), diff --git a/tests/template/templates/include_variables.html b/tests/template/templates/include_variables.html new file mode 100644 index 0000000000..8371a061b5 --- /dev/null +++ b/tests/template/templates/include_variables.html @@ -0,0 +1 @@ + diff --git a/tests/template/templates/includejs.html b/tests/template/templates/includejs.html index 8a2587d76b..dbcf6e04a8 100644 --- a/tests/template/templates/includejs.html +++ b/tests/template/templates/includejs.html @@ -2,4 +2,6 @@ -{SCRIPTS} \ No newline at end of file + + +{SCRIPTS} diff --git a/tests/template/templates/includephp_variables.html b/tests/template/templates/includephp_variables.html new file mode 100644 index 0000000000..6106efc86a --- /dev/null +++ b/tests/template/templates/includephp_variables.html @@ -0,0 +1,2 @@ +Path includes variables. + diff --git a/tests/template/templates/subdir/parent_only.js b/tests/template/templates/subdir/parent_only.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/template/templates/subdir/subsubdir/parent_only.js b/tests/template/templates/subdir/subsubdir/parent_only.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/template/templates/subdir/variable.html b/tests/template/templates/subdir/variable.html new file mode 100644 index 0000000000..f68f91597c --- /dev/null +++ b/tests/template/templates/subdir/variable.html @@ -0,0 +1 @@ +{VARIABLE} From d0cb5bb093d09a15f422f84e19d781a8260512a0 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Sun, 8 Jul 2012 15:12:08 +0100 Subject: [PATCH 2/7] [ticket/10970] Added support for forms such as {FOO}bar.{EXT} PHPBB3-10970 --- phpBB/includes/template/filter.php | 30 +++++++--------------- tests/template/template_includejs_test.php | 3 ++- tests/template/templates/includejs.html | 1 + 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index a8985a771c..568727be82 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -373,33 +373,21 @@ class phpbb_template_filter extends php_user_filter */ private function parse_dynamic_path($path, $include_type) { - $segments = explode('/', $path); - $is_expr = true; - $str = array(); - $vars = array(); + $matches = array(); + $replace = array(); - foreach ($segments as $segment) + preg_match_all('/{[^}]+}/', $path, $matches); + foreach ($matches[0] as $var_str) { - if ($segment[0] === '{') - { - $var = $this->get_varref($segment, $tmp_is_expr); - $is_expr = $is_expr && $tmp_is_expr; - $vars[] = "isset($var)"; - $str[] = "$var . '/'"; - } - else - { - $str[] = "'$segment/'"; - } + $var = $this->get_varref($var_str, $tmp_is_expr); + $is_expr = $is_expr && $tmp_is_expr; + $vars[] = "isset($var)"; + $replace[] = "' . $var . '"; } - // Remove trailing slash from last element - $last = array_pop($str); - $str[] = str_replace('/', '', $last); - if (!$is_expr) { - return ' if (' . implode(' && ', $vars) . ") { \$_template->$include_type(" . implode(' . ', $str) . ', true); }'; + return ' if (' . implode(' && ', $vars) . ") { \$_template->$include_type('" . str_replace($matches[0], $replace, $path) . "', true); }"; } else { diff --git a/tests/template/template_includejs_test.php b/tests/template/template_includejs_test.php index d8c7170e94..22b020208b 100644 --- a/tests/template/template_includejs_test.php +++ b/tests/template/template_includejs_test.php @@ -23,10 +23,11 @@ class phpbb_template_template_includejs_test extends phpbb_template_template_tes '', '', '', + '', ); // Run test $cache_file = $this->template->cachepath . 'includejs.html.php'; - $this->run_template('includejs.html', array('PARENT' => 'parent_only.js', 'SUBDIR' => 'subdir'), array(), array(), implode('', $scripts), $cache_file); + $this->run_template('includejs.html', array('PARENT' => 'parent_only.js', 'SUBDIR' => 'subdir', 'EXT' => 'js'), array(), array(), implode('', $scripts), $cache_file); } } diff --git a/tests/template/templates/includejs.html b/tests/template/templates/includejs.html index dbcf6e04a8..ef73700eeb 100644 --- a/tests/template/templates/includejs.html +++ b/tests/template/templates/includejs.html @@ -4,4 +4,5 @@ + {SCRIPTS} From 47c67b6d021c6488106bb0d327600b35e08bf94b Mon Sep 17 00:00:00 2001 From: Fyorl Date: Tue, 24 Jul 2012 11:54:46 +0100 Subject: [PATCH 3/7] [ticket/10970] Added missing initialisations and modified the regex Non-existent variables now become empty strings. PHPBB3-10970 --- phpBB/includes/template/filter.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index 568727be82..596b83cc75 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -375,10 +375,13 @@ class phpbb_template_filter extends php_user_filter { $matches = array(); $replace = array(); + $vars = array(); + $is_expr = true; - preg_match_all('/{[^}]+}/', $path, $matches); + preg_match_all('#\{((?:' . self::REGEX_NS . '\.)*)(\$)?(' . self::REGEX_VAR . ')\}#', $path, $matches); foreach ($matches[0] as $var_str) { + $tmp_is_expr = false; $var = $this->get_varref($var_str, $tmp_is_expr); $is_expr = $is_expr && $tmp_is_expr; $vars[] = "isset($var)"; @@ -387,7 +390,7 @@ class phpbb_template_filter extends php_user_filter if (!$is_expr) { - return ' if (' . implode(' && ', $vars) . ") { \$_template->$include_type('" . str_replace($matches[0], $replace, $path) . "', true); }"; + return " \$_template->$include_type('" . str_replace($matches[0], $replace, $path) . "', true);"; } else { From 003220dc7c781df38fde75f0007c3b3e48803162 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Tue, 24 Jul 2012 11:55:38 +0100 Subject: [PATCH 4/7] [ticket/10970] Fixed a problem with prepending root paths PHPBB3-10970 --- phpBB/includes/template/template.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 8ab3c44be3..861a5e28e7 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -503,7 +503,11 @@ class phpbb_template // Locate file if ($locate) { - $file = $this->locator->get_first_file_location(array($file), true, true); + $located = $this->locator->get_first_file_location(array($file), false, true); + if ($located) + { + $file = $located; + } } else if ($relative) { From 02ccf392fa11c7c2210f5ba1e8923e82e62a66cf Mon Sep 17 00:00:00 2001 From: Fyorl Date: Fri, 10 Aug 2012 13:42:38 +0100 Subject: [PATCH 5/7] [ticket/10970] Removed unused $vars PHPBB3-10970 --- phpBB/includes/template/filter.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index 596b83cc75..e4c3184565 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -375,7 +375,6 @@ class phpbb_template_filter extends php_user_filter { $matches = array(); $replace = array(); - $vars = array(); $is_expr = true; preg_match_all('#\{((?:' . self::REGEX_NS . '\.)*)(\$)?(' . self::REGEX_VAR . ')\}#', $path, $matches); @@ -384,7 +383,6 @@ class phpbb_template_filter extends php_user_filter $tmp_is_expr = false; $var = $this->get_varref($var_str, $tmp_is_expr); $is_expr = $is_expr && $tmp_is_expr; - $vars[] = "isset($var)"; $replace[] = "' . $var . '"; } From e9bdca7a8bc8c36b6d55806b1337e5d32409cb9d Mon Sep 17 00:00:00 2001 From: Fyorl Date: Sun, 19 Aug 2012 17:12:08 +0800 Subject: [PATCH 6/7] [ticket/10970] Added newline in docblock PHPBB3-10970 --- phpBB/includes/template/filter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index e4c3184565..471ca0c777 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -363,6 +363,7 @@ class phpbb_template_filter extends php_user_filter /** * Parse paths of the form {FOO}/a/{BAR}/b + * * Note: this method assumes at least one variable in the path, this should * be checked before this method is called. * From a05f354fdf903d00e85d8e4ad1d50f9ef906534d Mon Sep 17 00:00:00 2001 From: Fyorl Date: Sat, 10 Nov 2012 09:36:02 +0000 Subject: [PATCH 7/7] [ticket/10970] Added extra documentation to parse_dynamic_path. PHPBB3-10970 --- phpBB/includes/template/filter.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/template/filter.php b/phpBB/includes/template/filter.php index 471ca0c777..3e90190db7 100644 --- a/phpBB/includes/template/filter.php +++ b/phpBB/includes/template/filter.php @@ -370,7 +370,8 @@ class phpbb_template_filter extends php_user_filter * @param string $path The path to parse * @param string $include_type The type of template function to call * @return string An appropriately formatted string to include in the - * template + * template or an empty string if an expression like S_FIRST_ROW was + * incorrectly used */ private function parse_dynamic_path($path, $include_type) {