Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a strict_variables option to the Engine #325

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce a strict_variables option to the Engine #325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/Mustache/Compiler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Mustache_Compiler
private $entityFlags;
private $charset;
private $strictCallables;
private $strictVariables;

/**
* Compile a Mustache token parse tree into PHP source code.
Expand All @@ -36,11 +37,12 @@ class Mustache_Compiler
* @param bool $customEscape (default: false)
* @param string $charset (default: 'UTF-8')
* @param bool $strictCallables (default: false)
* @param bool $strictVariables (default: false)
* @param int $entityFlags (default: ENT_COMPAT)
*
* @return string Generated PHP source code
*/
public function compile($source, array $tree, $name, $customEscape = false, $charset = 'UTF-8', $strictCallables = false, $entityFlags = ENT_COMPAT)
public function compile($source, array $tree, $name, $customEscape = false, $charset = 'UTF-8', $strictCallables = false, $entityFlags = ENT_COMPAT, $strictVariables = false)
{
$this->pragmas = $this->defaultPragmas;
$this->sections = array();
Expand All @@ -51,6 +53,7 @@ public function compile($source, array $tree, $name, $customEscape = false, $cha
$this->entityFlags = $entityFlags;
$this->charset = $charset;
$this->strictCallables = $strictCallables;
$this->strictVariables = $strictVariables;

return $this->writeCode($tree, $name);
}
Expand Down Expand Up @@ -187,7 +190,7 @@ private function walk(array $tree, $level = 0)

class %s extends Mustache_Template
{
private $lambdaHelper;%s
private $lambdaHelper;%s%s

public function renderInternal(Mustache_Context $context, $indent = \'\')
{
Expand All @@ -204,7 +207,7 @@ public function renderInternal(Mustache_Context $context, $indent = \'\')
const KLASS_NO_LAMBDAS = '<?php

class %s extends Mustache_Template
{%s
{%s%s
public function renderInternal(Mustache_Context $context, $indent = \'\')
{
$buffer = \'\';
Expand All @@ -216,6 +219,8 @@ public function renderInternal(Mustache_Context $context, $indent = \'\')

const STRICT_CALLABLE = 'protected $strictCallables = true;';

const STRICT_VARIABLE = 'protected $strictVariables = true;';

/**
* Generate Mustache Template class PHP source.
*
Expand All @@ -232,8 +237,9 @@ private function writeCode($tree, $name)
$klass = empty($this->sections) && empty($this->blocks) ? self::KLASS_NO_LAMBDAS : self::KLASS;

$callable = $this->strictCallables ? $this->prepare(self::STRICT_CALLABLE) : '';
$variable = $this->strictVariables ? $this->prepare(self::STRICT_VARIABLE) : '';

return sprintf($this->prepare($klass, 0, false, true), $name, $callable, $code, $sections, $blocks);
return sprintf($this->prepare($klass, 0, false, true), $name, $callable, $variable, $code, $sections, $blocks);
}

const BLOCK_VAR = '
Expand Down Expand Up @@ -322,7 +328,11 @@ private function block($nodes)
}

const SECTION_CALL = '
$value = $context->%s(%s);%s
try {
$value = $context->%s(%s);%s
} catch (Mustache_Exception_UnknownVariableException $ex) {
$value = "";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why aren't sections (and inverted sections) strict?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is a bit old so I have forgotten a bit about it. I will try to deep dive in again next week but this is likely due to some previous comments:
#325 (comment)
#325 (comment)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha. i'll poke around a bit and see if it makes sense to me.

}
$buffer .= $this->section%s($context, $indent, $value);
';

Expand Down Expand Up @@ -396,7 +406,11 @@ private function section($nodes, $id, $filters, $start, $end, $otag, $ctag, $lev
}

const INVERTED_SECTION = '
$value = $context->%s(%s);%s
try {
$value = $context->%s(%s);%s
} catch (Mustache_Exception_UnknownVariableException $ex) {
$value = "";
}
if (empty($value)) {
%s
}
Expand Down
17 changes: 12 additions & 5 deletions src/Mustache/Context.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,22 @@
*/
class Mustache_Context
{
private $stack = array();
private $blockStack = array();
private $stack = array();
private $blockStack = array();
private $strictVariables;

/**
* Mustache rendering Context constructor.
*
* @param mixed $context Default rendering context (default: null)
* @param bool $strictVariables
*/
public function __construct($context = null)
public function __construct($context = null, $strictVariables = false)
{
if ($context !== null) {
$this->stack = array($context);
}
$this->strictVariables = $strictVariables;
}

/**
Expand Down Expand Up @@ -200,10 +203,11 @@ public function findInBlock($id)
*
* @see Mustache_Context::find
*
* @param string $id Variable name
* @param array $stack Context stack
* @param string $id Variable name
* @param array $stack Context stack
*
* @return mixed Variable value, or '' if not found
* @throws \Mustache_Exception_UnknownVariableException
*/
private function findVariableInStack($id, array $stack)
{
Expand Down Expand Up @@ -237,6 +241,9 @@ private function findVariableInStack($id, array $stack)
}
}

if ($this->strictVariables) {
throw new Mustache_Exception_UnknownVariableException($id);
}
return '';
}
}
10 changes: 9 additions & 1 deletion src/Mustache/Engine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Mustache_Engine
private $charset = 'UTF-8';
private $logger;
private $strictCallables = false;
private $strictVariables = false;
private $pragmas = array();
private $delimiters;

Expand Down Expand Up @@ -132,6 +133,9 @@ class Mustache_Engine
* // This currently defaults to false, but will default to true in v3.0.
* 'strict_callables' => true,
*
* // Treat unknown variables as a failure and throw an exception instead of silently ignoring them.
* 'strict_variables' => true,
*
* // Enable pragmas across all templates, regardless of the presence of pragma tags in the individual
* // templates.
* 'pragmas' => [Mustache_Engine::PRAGMA_FILTERS],
Expand Down Expand Up @@ -206,6 +210,10 @@ public function __construct(array $options = array())
$this->strictCallables = $options['strict_callables'];
}

if (isset($options['strict_variables'])) {
$this->strictVariables = $options['strict_variables'];
}

if (isset($options['delimiters'])) {
$this->delimiters = $options['delimiters'];
}
Expand Down Expand Up @@ -812,7 +820,7 @@ private function compile($source)
$compiler = $this->getCompiler();
$compiler->setPragmas($this->getPragmas());

return $compiler->compile($source, $tree, $name, isset($this->escape), $this->charset, $this->strictCallables, $this->entityFlags);
return $compiler->compile($source, $tree, $name, isset($this->escape), $this->charset, $this->strictCallables, $this->entityFlags, $this->strictVariables);
}

/**
Expand Down
41 changes: 41 additions & 0 deletions src/Mustache/Exception/UnknownVariableException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

/*
* This file is part of Mustache.php.
*
* (c) 2017 Enalean
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

class Mustache_Exception_UnknownVariableException extends UnexpectedValueException implements Mustache_Exception
{
/**
* @var string
*/
protected $variableName;

/**
* @param string $variableName
* @param Exception $previous
*/
public function __construct($variableName, Exception $previous = null)
{
$this->variableName = $variableName;
$message = sprintf('Unknown variable: %s', $variableName);
if (version_compare(PHP_VERSION, '5.3.0', '>=')) {
parent::__construct($message, 0, $previous);
} else {
parent::__construct($message); // @codeCoverageIgnore
}
}

/**
* @return string
*/
public function getVariableName()
{
return $this->variableName;
}
}
7 changes: 6 additions & 1 deletion src/Mustache/Template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ abstract class Mustache_Template
*/
protected $strictCallables = false;

/**
* @var bool
*/
protected $strictVariables = false;

/**
* Mustache Template constructor.
*
Expand Down Expand Up @@ -143,7 +148,7 @@ protected function isIterable($value)
*/
protected function prepareContextStack($context = null)
{
$stack = new Mustache_Context();
$stack = new Mustache_Context(null, $this->strictVariables);

$helpers = $this->mustache->getHelpers();
if (!$helpers->isEmpty()) {
Expand Down
2 changes: 1 addition & 1 deletion test/Mustache/Test/CompilerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function testCompile($source, array $tree, $name, $customEscaper, $entity
{
$compiler = new Mustache_Compiler();

$compiled = $compiler->compile($source, $tree, $name, $customEscaper, $charset, false, $entityFlags);
$compiled = $compiler->compile($source, $tree, $name, $customEscaper, $charset, false, $entityFlags, false);
foreach ($expected as $contains) {
$this->assertContains($contains, $compiled);
}
Expand Down
23 changes: 23 additions & 0 deletions test/Mustache/Test/ContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,29 @@ public function testAnchoredDotNotationThrowsExceptions()
$context->push(array('a' => 1));
$context->findAnchoredDot('a');
}

/**
* @expectedException Mustache_Exception_UnknownVariableException
*/
public function testUnknownVariableThrowsException()
{
$context = new Mustache_Context(null, true);
$context->push(array('a' => 1));
$context->find('b');
}

/**
* @expectedException Mustache_Exception_UnknownVariableException
*/
public function testAnchoredDotNotationUnknownVariableThrowsException()
{
$context = new Mustache_Context(null, true);
$a = array(
'a' => array('b' => 1),
);
$context->push($a);
$context->find('a.c');
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd like to see positive and negative tests. we can probably do it with a single method for "throws exception" and a single method for "doesn't throw exception" and a dataprovider for each.

for the examples below, assume context like this:

[
  'a' => ['b' => 1],
  'c' => 1,
  'd' => 0,
]

we need coverage for:

  • finding things that exist doesn't throw, with coverage for top level variables, variables in nested sections, dot notation, and top level variables from inside sections.
    e.g.: {{ a }}, {{# a }}{{ b }}{{/ a }}, {{ a.b }} and {{# c }}{{ a }}{{/ c }}, etc.

  • sections and inverted sections never throw.
    e.g.: {{# x }}{{/ x }}, {{^ x }}{{/ x }}, {{# a }}{{# x }}{{/ x }}{{/ a }}, {{# a.x }}{{/ a.x }}, {{# d.x }}{{/ d.x }}, etc.

  • missing variables inside falsey sections never throw.
    e.g.: {{^ a }}{{ x }}{{/ a }} and {{# d }}{{ x }}{{/ d }}, etc.

}

class Mustache_Test_TestDummy
Expand Down
44 changes: 44 additions & 0 deletions test/Mustache/Test/Exception/UnknownVariableExceptionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

/*
* This file is part of Mustache.php.
*
* (c) 2017 Enalean
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

class Mustache_Test_Exception_UnknownVariableExceptionTest extends PHPUnit_Framework_TestCase
{

public function testInstance()
{
$e = new Mustache_Exception_UnknownVariableException('alpha');
$this->assertTrue($e instanceof UnexpectedValueException);
$this->assertTrue($e instanceof Mustache_Exception);
}

public function testMessage()
{
$e = new Mustache_Exception_UnknownVariableException('beta');
$this->assertEquals('Unknown variable: beta', $e->getMessage());
}

public function testGetHelperName()
{
$e = new Mustache_Exception_UnknownVariableException('gamma');
$this->assertEquals('gamma', $e->getVariableName());
}

public function testPrevious()
{
if (version_compare(PHP_VERSION, '5.3.0', '<')) {
$this->markTestSkipped('Exception chaining requires at least PHP 5.3');
}

$previous = new Exception();
$e = new Mustache_Exception_UnknownVariableException('foo', $previous);
$this->assertSame($previous, $e->getPrevious());
}
}
Loading