From c609f25bae6aee2b7fab8dfeb8b3a58e2c2f396f Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Mon, 4 Nov 2013 12:15:02 -0600 Subject: [PATCH 1/7] [ticket/11943] Add test for DEFINE $VAR = false PHPBB3-11943 --- tests/template/template_test.php | 2 +- tests/template/templates/define.html | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/template/template_test.php b/tests/template/template_test.php index 39eb08ab79..0ff469779d 100644 --- a/tests/template/template_test.php +++ b/tests/template/template_test.php @@ -158,7 +158,7 @@ class phpbb_template_template_test extends phpbb_template_template_test_case array(), array('test_loop' => array(array(), array(), array(), array(), array(), array(), array()), 'test' => array(array()), 'test.deep' => array(array()), 'test.deep.defines' => array(array())), array(), - "xyz\nabc\n\$VALUE == 'abc'\n(\$VALUE == 'abc')\n((\$VALUE == 'abc'))\n!\$DOESNT_EXIST\n(!\$DOESNT_EXIST)\nabc\nbar\nbar\nabc\ntest!@#$%^&*()_-=+{}[]:;\",<.>/?[]|foobar|", + "xyz\nabc\n\$VALUE == 'abc'\n(\$VALUE == 'abc')\n((\$VALUE == 'abc'))\n!\$DOESNT_EXIST\n(!\$DOESNT_EXIST)\nabc\nbar\nbar\nabc\ntest!@#$%^&*()_-=+{}[]:;\",<.>/?[]|foobar|false", ), array( 'define_advanced.html', diff --git a/tests/template/templates/define.html b/tests/template/templates/define.html index e7ce7f7def..276d2ebb99 100644 --- a/tests/template/templates/define.html +++ b/tests/template/templates/define.html @@ -29,3 +29,9 @@ $VALUE == 'abc' [{$VALUE}] foobar|{$TEST}| + + +true + +false + From b49d3a1851330d64009c8050132e50b093172559 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Mon, 4 Nov 2013 12:21:12 -0600 Subject: [PATCH 2/7] [ticket/11943] Do not quote the value when it is exactly true, false, or null Quoting these can change the meaning of the value (e.g. 'false' == true) PHPBB3-11943 --- phpBB/phpbb/template/twig/lexer.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/phpBB/phpbb/template/twig/lexer.php b/phpBB/phpbb/template/twig/lexer.php index be53b3eb5b..8c52fa65b2 100644 --- a/phpBB/phpbb/template/twig/lexer.php +++ b/phpBB/phpbb/template/twig/lexer.php @@ -129,6 +129,14 @@ class lexer extends \Twig_Lexer // Replace template variables with start/end to parse variables (' ~ TEST ~ '.html) $matches[2] = preg_replace('#{([a-zA-Z0-9_\.$]+)}#', "'~ \$1 ~'", $matches[2]); + // If the second item is exactly one of a few key words, + // do not quote it as it changes the meaning + // http://tracker.phpbb.com/browse/PHPBB3-11943 + if (in_array($matches[2], array('false', 'true', 'null'))) + { + return ""; + } + // Surround the matches in single quotes ('' ~ TEST ~ '.html') return ""; }; From da332aa0a5cbeabbcce5551ee955c701fc2a1d73 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 8 Nov 2013 19:55:16 -0600 Subject: [PATCH 3/7] [ticket/11943] Require stricter DEFINE statements for templates PHPBB3-11943 --- phpBB/phpbb/template/twig/lexer.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/phpBB/phpbb/template/twig/lexer.php b/phpBB/phpbb/template/twig/lexer.php index 8c52fa65b2..efd6a0bd84 100644 --- a/phpBB/phpbb/template/twig/lexer.php +++ b/phpBB/phpbb/template/twig/lexer.php @@ -69,7 +69,7 @@ class lexer extends \Twig_Lexer // Fix tokens that may have inline variables (e.g. "; - } - // Surround the matches in single quotes ('' ~ TEST ~ '.html') return ""; }; From 6370970f13d58f617379da64efb1f88a522f3f03 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 8 Nov 2013 22:30:58 -0600 Subject: [PATCH 4/7] [ticket/11943] Split fix_inline_variable_tokens into 3 steps DEFINE shouldn't add/remove surrounding quotes, but must have the inline variable tokens fixed PHPBB3-11943 --- phpBB/phpbb/template/twig/lexer.php | 60 ++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/phpBB/phpbb/template/twig/lexer.php b/phpBB/phpbb/template/twig/lexer.php index efd6a0bd84..f4efc58540 100644 --- a/phpBB/phpbb/template/twig/lexer.php +++ b/phpBB/phpbb/template/twig/lexer.php @@ -68,8 +68,20 @@ class lexer extends \Twig_Lexer ); // Fix tokens that may have inline variables (e.g. #', '', $code); + } + /** * Fix tokens that may have inline variables * - * E.g. "; + return ""; }; return preg_replace_callback('##', $callback, $code); } + /** + * Add surrounding quotes + * + * Last step to fix tokens that may have inline variables + * E.g. #', '', $code); + } + /** * Fix begin tokens (convert our BEGIN to Twig for) * From 2e5117a71eb64c734e5738235c44ef92818ca33b Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Sat, 9 Nov 2013 11:14:55 -0600 Subject: [PATCH 5/7] [ticket/11943] Throw an exception if DEFINE is setup improperly PHPBB3-11943 --- phpBB/phpbb/template/twig/tokenparser/defineparser.php | 7 +++++++ tests/template/template_test.php | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/phpBB/phpbb/template/twig/tokenparser/defineparser.php b/phpBB/phpbb/template/twig/tokenparser/defineparser.php index 21add0c17c..8484f2e81a 100644 --- a/phpBB/phpbb/template/twig/tokenparser/defineparser.php +++ b/phpBB/phpbb/template/twig/tokenparser/defineparser.php @@ -30,6 +30,13 @@ class defineparser extends \Twig_TokenParser $stream->next(); $value = $this->parser->getExpressionParser()->parseExpression(); + if ($value instanceof \Twig_Node_Expression_Name) + { + // This would happen if someone improperly formed their DEFINE syntax + // e.g. + throw new \Twig_Error_Syntax('Invalid DEFINE', $token->getLine(), $this->parser->getFilename()); + } + $stream->expect(\Twig_Token::BLOCK_END_TYPE); } else { $capture = true; diff --git a/tests/template/template_test.php b/tests/template/template_test.php index 0ff469779d..6e9b7d3ee9 100644 --- a/tests/template/template_test.php +++ b/tests/template/template_test.php @@ -561,4 +561,12 @@ EOT $expect = 'outer - 0[outer|4]outer - 1[outer|4]middle - 0[middle|1]outer - 2 - test[outer|4]middle - 0[middle|2]middle - 1[middle|2]outer - 3[outer|4]middle - 0[middle|3]middle - 1[middle|3]middle - 2[middle|3]'; $this->assertEquals($expect, str_replace(array("\n", "\r", "\t"), '', $this->display('test')), 'Ensuring S_NUM_ROWS is correct after modification'); } + + /** + * @expectedException Twig_Error_Syntax + */ + public function test_define_error() + { + $this->run_template('define_error.html', array(), array(), array(), ''); + } } From 64ed46e682564600c8b8688e7d4e49620eb79bb3 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Sat, 9 Nov 2013 11:39:35 -0600 Subject: [PATCH 6/7] [ticket/11943] Forgot template file for test PHPBB3-11943 --- tests/template/templates/define_error.html | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tests/template/templates/define_error.html diff --git a/tests/template/templates/define_error.html b/tests/template/templates/define_error.html new file mode 100644 index 0000000000..a272071736 --- /dev/null +++ b/tests/template/templates/define_error.html @@ -0,0 +1,2 @@ + +{$VAR} \ No newline at end of file From 47e364ae68a92dbf2107c56a2165b219e4312b50 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Mon, 11 Nov 2013 10:59:17 -0600 Subject: [PATCH 7/7] [ticket/11943] New line at EOF for define_error.html PHPBB3-11943 --- tests/template/templates/define_error.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/template/templates/define_error.html b/tests/template/templates/define_error.html index a272071736..72ab1ba033 100644 --- a/tests/template/templates/define_error.html +++ b/tests/template/templates/define_error.html @@ -1,2 +1,2 @@ -{$VAR} \ No newline at end of file +{$VAR}