Merge remote-tracking branch 'EXreaction/ticket/11943' into develop

* EXreaction/ticket/11943:
  [ticket/11943] New line at EOF for define_error.html
  [ticket/11943] Forgot template file for test
  [ticket/11943] Throw an exception if DEFINE is setup improperly
  [ticket/11943] Split fix_inline_variable_tokens into 3 steps
  [ticket/11943] Require stricter DEFINE statements for templates
  [ticket/11943] Do not quote the value when it is exactly true, false, or null
  [ticket/11943] Add test for DEFINE $VAR = false

phpBB/phpbb/template/twig/lexer.php

@@ -68,6 +68,12 @@ class lexer extends \Twig_Lexer
 		);
 
 		// Fix tokens that may have inline variables (e.g. <!-- DEFINE $TEST = '{FOO}')
+		$code = $this->strip_surrounding_quotes(array(
+			'INCLUDE',
+			'INCLUDEPHP',
+			'INCLUDEJS',
+			'INCLUDECSS',
+		), $code);
 		$code = $this->fix_inline_variable_tokens(array(
 			'DEFINE \$[a-zA-Z0-9_]+ =',
 			'INCLUDE',
@@ -75,6 +81,12 @@ class lexer extends \Twig_Lexer
 			'INCLUDEJS',
 			'INCLUDECSS',
 		), $code);
+		$code = $this->add_surrounding_quotes(array(
+			'INCLUDE',
+			'INCLUDEPHP',
+			'INCLUDEJS',
+			'INCLUDECSS',
+		), $code);
 
 		// Fix our BEGIN statements
 		$code = $this->fix_begin_tokens($code);
@@ -107,10 +119,30 @@ class lexer extends \Twig_Lexer
 		return parent::tokenize($code, $filename);
 	}
 
+	/**
+	* Strip surrounding quotes
+	*
+	* First step to fix tokens that may have inline variables
+	* E.g. <!-- INCLUDE '{TEST}.html' to <!-- INCLUDE {TEST}.html
+	*
+	* @param array $tokens array of tokens to search for (imploded to a regular expression)
+	* @param string $code
+	* @return string
+	*/
+	protected function strip_surrounding_quotes($tokens, $code)
+	{
+		// Remove matching quotes at the beginning/end if a statement;
+		// E.g. 'asdf'"' -> asdf'"
+		// E.g. "asdf'"" -> asdf'"
+		// E.g. 'asdf'" -> 'asdf'"
+		return preg_replace('#<!-- (' . implode('|', $tokens) . ') (([\'"])?(.*?)\1) -->#', '<!-- $1 $2 -->', $code);
+	}
+
 	/**
 	* Fix tokens that may have inline variables
 	*
-	* E.g. <!-- INCLUDE {TEST}.html
+	* Second step to fix tokens that may have inline variables
+	* E.g. <!-- INCLUDE '{TEST}.html' to <!-- INCLUDE ' ~ {TEST} ~ '.html
 	*
 	* @param array $tokens array of tokens to search for (imploded to a regular expression)
 	* @param string $code
@@ -120,22 +152,30 @@ class lexer extends \Twig_Lexer
 	{
 		$callback = function($matches)
 		{
-			// Remove matching quotes at the beginning/end if a statement;
-			// E.g. 'asdf'"' -> asdf'"
-			// E.g. "asdf'"" -> asdf'"
-			// E.g. 'asdf'" -> 'asdf'"
-			$matches[2] = preg_replace('#^([\'"])?(.*?)\1$#', '$2', $matches[2]);
-
 			// Replace template variables with start/end to parse variables (' ~ TEST ~ '.html)
 			$matches[2] = preg_replace('#{([a-zA-Z0-9_\.$]+)}#', "'~ \$1 ~'", $matches[2]);
 
-			// Surround the matches in single quotes ('' ~ TEST ~ '.html')
+			return "<!-- {$matches[1]} {$matches[2]} -->";
-			return "<!-- {$matches[1]} '{$matches[2]}' -->";
 		};
 
 		return preg_replace_callback('#<!-- (' . implode('|', $tokens) . ') (.+?) -->#', $callback, $code);
 	}
 
+	/**
+	* Add surrounding quotes
+	*
+	* Last step to fix tokens that may have inline variables
+	* E.g. <!-- INCLUDE '{TEST}.html' to <!-- INCLUDE '' ~ {TEST} ~ '.html'
+	*
+	* @param array $tokens array of tokens to search for (imploded to a regular expression)
+	* @param string $code
+	* @return string
+	*/
+	protected function add_surrounding_quotes($tokens, $code)
+	{
+		return preg_replace('#<!-- (' . implode('|', $tokens) . ') (.+?) -->#', '<!-- $1 \'$2\' -->', $code);
+	}
+
 	/**
 	* Fix begin tokens (convert our BEGIN to Twig for)
 	*

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. <!-- DEFINE $VAR = foo -->
+				throw new \Twig_Error_Syntax('Invalid DEFINE', $token->getLine(), $this->parser->getFilename());
+			}
+
 			$stream->expect(\Twig_Token::BLOCK_END_TYPE);
 		} else {
 			$capture = true;

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',
@@ -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(), '');
+	}
 }

tests/template/templates/define.html

@@ -29,3 +29,9 @@ $VALUE == 'abc'
 <!-- DEFINE $VALUE = '' -->
 [{$VALUE}]
 <!-- DEFINE $TEST -->foobar<!-- ENDDEFINE -->|{$TEST}|
+<!-- DEFINE $VAR = false -->
+<!-- IF $VAR -->
+true
+<!-- ELSE -->
+false
+<!-- ENDIF -->

tests/template/templates/define_error.html

@@ -0,0 +1,2 @@
+<!-- DEFINE $VAR = foo -->
+{$VAR}