Skip to content

Commit

Permalink
2.3 (#364)
Browse files Browse the repository at this point in the history
* [TASK] Use the Trusty build environment on Travis (#314)

* [TASK] Remove test coverage of HHVM from travis.yml (#323)

Resolves: #322

* [BUGFIX] Fix getLayoutPathAndFilename behavior in TemplatePaths (#309)

Class TemplatePaths will now correctly return the class variable layoutPathAndFilename if it was set before.

Close: #309

* [BUGFIX] Use sanizted identifier in TemplateCompiler::has() (#321)

Instances are stored with the sanitzied identifier into a runtime cache.
The `TemplateCompiler::has()` must also use the sanitzed version of the
identifier to check if the instance exists in the runtime cache.

Resolves #320

* require and include are statements (#316)

* [BUGFIX] Prevent re-loading cached classes that already exist (#315)

This patch prevents a problem where it is made up to the individual cache implementation whether or not to re-load a class file when a class is already defined. Instead, making the compiler only fetch the class from cache if it is not already loaded (by checking class_exists without allowing autoloading!) prevents re-loading classes with “class already declared” errors to follow.

* [TASK] Throw ViewHelper exception in f:count on uncountable subject (#296)

* [TASK] Use (float) and (int) (#313)

* [BUGFIX] Do not attempt to escape non-string or -compatible values (#285)

Avoids calling htmlspecialchars() on incompatible values. Changes compiling of the escaping node to generate a small closure which checks for string or string-compatible value before escaping.

* [BUGFIX] Handle adding namespaces to ignored namespaces (#283)

Corrects the following two misbehaviors:

1. A second call to add a namespace with a `null` value causes an error; expected: silently keep ignoring namespace.
2. A second call to add a namespace that was previously ignored causes an error; expected: converts ignored namespace to active.

Fixes: #282

* [TASK] Fix typo in unknown namespace exception message (#326)

* [BUGFIX] Make casting of numbers the same in arguments and arrays (#333)

Problem briefly described: numbers passed in tag attributes vs. numbers passed in inline syntax (which internally uses the array syntax parsing) handles numeric values in two different ways:

* In tags, a NumericNode is created if is_numeric is true
* In arrays, numbers are cast with (float) and matched by regexp

NumericNode also casts the value but does so by using an add zero trick which makes PHP do the casting based on string value, and the input is always a string in the parser. This means that the two different ways of passing a number will produce two different
types of variables given natural numbers as input. Passing floats still causes the same type.

The solution is to apply the same method of casting in both cases.

* [BUGFIX] Make ViewHelperResolver internal cache non-static (#328)

Using a static class property for the cache could have bad side effects for setups which have multiple contexts for Fluid and may change the namespaces between contexts.

Converting the cache to a non-static property cleans it properly when a new ViewHelperResolver is created.

* [BUGFIX] Remove incorrect throws annotation (#355)

Exception class doesn't exist,

* [TASK] Add PHP 7.2 to travis (#363)
  • Loading branch information
opi99 authored and lolli42 committed Feb 5, 2018
1 parent a409981 commit 1a7947d
Show file tree
Hide file tree
Showing 18 changed files with 97 additions and 31 deletions.
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
dist: trusty

language: php

branches:
Expand All @@ -17,7 +19,7 @@ php:
- 5.6
- 7
- 7.1
- hhvm
- 7.2

before_install:
- composer self-update
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Cache/SimpleFileCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function get($name)
}
$file = $this->getCachedFilePathAndFilename($name);
if (file_exists($file) && !class_exists($name)) {
include_once($file);
include_once $file;
return true;
}
return false;
Expand Down
7 changes: 6 additions & 1 deletion src/Core/Compiler/NodeConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ public function convert(NodeInterface $node)
protected function convertEscapingNode(EscapingNode $node)
{
$configuration = $this->convert($node->getNode());
$configuration['execution'] = sprintf('htmlspecialchars(%s, ENT_QUOTES)', $configuration['execution']);
$configuration['execution'] = sprintf(
'call_user_func_array( function ($var) { ' .
'return (is_string($var) || (is_object($var) && method_exists($var, \'__toString\')) ' .
'? htmlspecialchars((string) $var, ENT_QUOTES) : $var); }, [%s])',
$configuration['execution']
);
return $configuration;
}

Expand Down
13 changes: 9 additions & 4 deletions src/Core/Compiler/TemplateCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,15 @@ public function isDisabled()
*/
public function has($identifier)
{
$identifier = $this->sanitizeIdentifier($identifier);

if (isset($this->syntaxTreeInstanceCache[$identifier])) {
return true;
}
if (!$this->renderingContext->isCacheEnabled()) {
return false;
}
$identifier = $this->sanitizeIdentifier($identifier);
return !empty($identifier) && $this->renderingContext->getCache()->get($identifier);
return !empty($identifier) && (class_exists($identifier, false) || $this->renderingContext->getCache()->get($identifier));
}

/**
Expand All @@ -161,10 +162,13 @@ public function get($identifier)
$identifier = $this->sanitizeIdentifier($identifier);

if (!isset($this->syntaxTreeInstanceCache[$identifier])) {
$this->renderingContext->getCache()->get($identifier);
if (!class_exists($identifier, false)) {
$this->renderingContext->getCache()->get($identifier);
}
$this->syntaxTreeInstanceCache[$identifier] = new $identifier();
}


return $this->syntaxTreeInstanceCache[$identifier];
}

Expand All @@ -185,6 +189,8 @@ public function reset()
*/
public function store($identifier, ParsingState $parsingState)
{
$identifier = $this->sanitizeIdentifier($identifier);

if ($this->isDisabled()) {
$cache = $this->renderingContext->getCache();
if ($cache) {
Expand All @@ -196,7 +202,6 @@ public function store($identifier, ParsingState $parsingState)
}

$this->currentlyProcessingState = $parsingState;
$identifier = $this->sanitizeIdentifier($identifier);
$this->nodeConverter->setVariableCounter(0);
$generatedRenderFunctions = $this->generateSectionCodeFromParsingState($parsingState);

Expand Down
4 changes: 2 additions & 2 deletions src/Core/Parser/BooleanParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,9 @@ protected function evaluateTerm($x, $context)
return $x;
}
if (strpos($x, '.') !== false) {
return floatval($x);
return (float)$x;
} else {
return intval($x);
return (int)$x;
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/Core/Parser/SyntaxTree/EscapingNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ public function __construct(NodeInterface $node)
*/
public function evaluate(RenderingContextInterface $renderingContext)
{
return htmlspecialchars($this->node->evaluate($renderingContext), ENT_QUOTES);
$evaluated = $this->node->evaluate($renderingContext);
if (is_string($evaluated) || (is_object($evaluated) && method_exists($evaluated, '__toString'))) {
return htmlspecialchars((string) $evaluated, ENT_QUOTES);
}
return $evaluated;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/Core/Parser/TemplateParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,8 @@ protected function recursiveArrayHandler($arrayText)
if (!empty($singleMatch['VariableIdentifier'])) {
$arrayToBuild[$arrayKey] = new ObjectAccessorNode($singleMatch['VariableIdentifier']);
} elseif (array_key_exists('Number', $singleMatch) && (!empty($singleMatch['Number']) || $singleMatch['Number'] === '0')) {
$arrayToBuild[$arrayKey] = floatval($singleMatch['Number']);
// Note: this method of casting picks "int" when value is a natural number and "float" if any decimals are found. See also NumericNode.
$arrayToBuild[$arrayKey] = $singleMatch['Number'] + 0;
} elseif ((array_key_exists('QuotedString', $singleMatch) && !empty($singleMatch['QuotedString']))) {
$argumentString = $this->unquoteString($singleMatch['QuotedString']);
$arrayToBuild[$arrayKey] = $this->buildArgumentObjectTree($argumentString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public function throwExceptionsForUnhandledNamespaces($templateSource)
foreach ($splitTemplate as $templateElement) {
if (preg_match(Patterns::$SCAN_PATTERN_TEMPLATE_VIEWHELPERTAG, $templateElement, $matchedVariables) > 0) {
if (!$viewHelperResolver->isNamespaceValidOrIgnored($matchedVariables['NamespaceIdentifier'])) {
throw new UnknownNamespaceException('Unkown Namespace: ' . htmlspecialchars($matchedVariables[0]));
throw new UnknownNamespaceException('Unknown Namespace: ' . htmlspecialchars($matchedVariables[0]));
}
continue;
} elseif (preg_match(Patterns::$SCAN_PATTERN_TEMPLATE_CLOSINGVIEWHELPERTAG, $templateElement, $matchedVariables) > 0) {
Expand All @@ -184,7 +184,7 @@ public function throwExceptionsForUnhandledNamespaces($templateSource)
if (is_array($shorthandViewHelpers) === true) {
foreach ($shorthandViewHelpers as $shorthandViewHelper) {
if (!$viewHelperResolver->isNamespaceValidOrIgnored($shorthandViewHelper['NamespaceIdentifier'])) {
throw new UnknownNamespaceException('Unkown Namespace: ' . $shorthandViewHelper['NamespaceIdentifier']);
throw new UnknownNamespaceException('Unknown Namespace: ' . $shorthandViewHelper['NamespaceIdentifier']);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/Core/Variables/VariableProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public function getAll();
* @param string $identifier Identifier of the variable to add
* @param mixed $value The variable's value
* @return void
* @throws Exception\InvalidVariableException
* @api
*/
public function add($identifier, $value);
Expand Down
10 changes: 5 additions & 5 deletions src/Core/ViewHelper/ViewHelperResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ViewHelperResolver
/**
* @var array
*/
protected static $resolvedViewHelperClassNames = [];
protected $resolvedViewHelperClassNames = [];

/**
* Namespaces requested by the template being rendered,
Expand Down Expand Up @@ -85,7 +85,7 @@ public function getNamespaces()
*/
public function addNamespace($identifier, $phpNamespace)
{
if (!array_key_exists($identifier, $this->namespaces)) {
if (!array_key_exists($identifier, $this->namespaces) || $this->namespaces[$identifier] === null) {
$this->namespaces[$identifier] = $phpNamespace === null ? null : (array) $phpNamespace;
} elseif (is_array($phpNamespace)) {
$this->namespaces[$identifier] = array_unique(array_merge($this->namespaces[$identifier], $phpNamespace));
Expand Down Expand Up @@ -244,7 +244,7 @@ public function isNamespaceIgnored($namespaceIdentifier)
*/
public function resolveViewHelperClassName($namespaceIdentifier, $methodIdentifier)
{
if (!isset(static::$resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier])) {
if (!isset($this->resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier])) {
$resolvedViewHelperClassName = $this->resolveViewHelperName($namespaceIdentifier, $methodIdentifier);
$actualViewHelperClassName = implode('\\', array_map('ucfirst', explode('.', $resolvedViewHelperClassName)));
if (false === class_exists($actualViewHelperClassName) || $actualViewHelperClassName === false) {
Expand All @@ -256,9 +256,9 @@ public function resolveViewHelperClassName($namespaceIdentifier, $methodIdentifi
$resolvedViewHelperClassName
), 1407060572);
}
static::$resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier] = $actualViewHelperClassName;
$this->resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier] = $actualViewHelperClassName;
}
return static::$resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier];
return $this->resolvedViewHelperClassNames[$namespaceIdentifier][$methodIdentifier];
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/View/TemplatePaths.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public function setTemplatePathAndFilename($templatePathAndFilename)
*/
public function setLayoutPathAndFilename($layoutPathAndFilename)
{
$this->layoutPathAndFilename = $layoutPathAndFilename;
$this->layoutPathAndFilename = (string) $this->sanitizePath($layoutPathAndFilename);
}

/**
Expand Down Expand Up @@ -632,6 +632,9 @@ protected function createIdentifierForFile($pathAndFilename, $prefix)
*/
public function getLayoutPathAndFilename($layoutName = 'Default')
{
if ($this->layoutPathAndFilename !== null) {
return $this->layoutPathAndFilename;
}
$format = $this->getFormat();
$layoutName = ucfirst($layoutName);
$layoutKey = $layoutName . '.' . $format;
Expand Down
14 changes: 12 additions & 2 deletions src/ViewHelpers/CountViewHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
*/
class CountViewHelper extends AbstractViewHelper
{

use CompileWithContentArgumentAndRenderStatic;

/**
Expand Down Expand Up @@ -64,6 +63,17 @@ public function initializeArguments()
*/
public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext)
{
return count($renderChildrenClosure());
$countable = $renderChildrenClosure();
if ($countable === null) {
return 0;
} elseif (!$countable instanceof \Countable && !is_array($countable)) {
throw new ViewHelper\Exception(
sprintf(
'Subject given to f:count() is not countable (type: %s)',
is_object($countable) ? get_class($countable) : gettype($countable)
)
);
}
return count($countable);
}
}
2 changes: 1 addition & 1 deletion tests/Unit/Core/Compiler/NodeConverterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public function getConvertTestValues()
],
[
new EscapingNode(new TextNode('foo')),
'htmlspecialchars(\'foo\', ENT_QUOTES)'
'call_user_func_array( function ($var) { return (is_string($var) || (is_object($var) && method_exists($var, \'__toString\')) ? htmlspecialchars((string) $var, ENT_QUOTES) : $var); }, [\'foo\'])'
],
[
new ViewHelperNode(
Expand Down
8 changes: 4 additions & 4 deletions tests/Unit/Core/Compiler/TemplateCompilerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public function testSetRenderingContext()
public function testHasReturnsFalseWithoutCache()
{
$instance = $this->getMock(TemplateCompiler::class, ['sanitizeIdentifier']);
$renderingContext = new RenderingContextFixture();
$renderingContext = $this->getMock(RenderingContextFixture::class, ['getCache']);
$renderingContext->cacheDisabled = true;
$renderingContext->expects($this->never())->method('getCache');
$instance->setRenderingContext($renderingContext);
$instance->expects($this->never())->method('sanitizeIdentifier');
$result = $instance->has('test');
$this->assertFalse($result);
}
Expand Down Expand Up @@ -126,9 +126,9 @@ public function testStoreReturnsEarlyIfDisabled()
{
$renderingContext = new RenderingContextFixture();
$renderingContext->cacheDisabled = true;
$instance = $this->getMock(TemplateCompiler::class, ['sanitizeIdentifier']);
$instance = $this->getMock(TemplateCompiler::class, ['generateSectionCodeFromParsingState']);
$instance->setRenderingContext($renderingContext);
$instance->expects($this->never())->method('sanitizeIdentifier');
$instance->expects($this->never())->method('generateSectionCodeFromParsingState');
$instance->store('foobar', new ParsingState());
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Core/Parser/TemplateParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public function templatesToSplit()
public function splitTemplateAtDynamicTagsReturnsCorrectlySplitTemplate($templateName)
{
$template = file_get_contents(__DIR__ . '/Fixtures/' . $templateName . '.html', FILE_TEXT);
$expectedResult = require(__DIR__ . '/Fixtures/' . $templateName . '-split.php');
$expectedResult = require __DIR__ . '/Fixtures/' . $templateName . '-split.php';
$templateParser = $this->getAccessibleMock(TemplateParser::class, ['dummy']);
$this->assertSame($expectedResult, $templateParser->_call('splitTemplateAtDynamicTags', $template), 'Filed for ' . $templateName);
}
Expand Down
36 changes: 34 additions & 2 deletions tests/Unit/Core/ViewHelper/ViewHelperResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ public function testResolveViewHelperClassNameSupportsMultipleNamespaces()
public function testResolveViewHelperClassNameTrimsBackslashSuffixFromNamespace()
{
$resolver = $this->getAccessibleMock(ViewHelperResolver::class, ['dummy']);
$resolver->_set('namespaces', ['f' => ['FluidTYPO3\\Fluid\\ViewHelpers\\']]);
$resolver->_set('namespaces', ['f' => ['TYPO3Fluid\\Fluid\\ViewHelpers\\']]);
$result = $resolver->_call('resolveViewHelperName', 'f', 'render');
$this->assertEquals('FluidTYPO3\\Fluid\\ViewHelpers\\RenderViewHelper', $result);
$this->assertEquals('TYPO3Fluid\\Fluid\\ViewHelpers\\RenderViewHelper', $result);
}

/**
Expand Down Expand Up @@ -134,6 +134,38 @@ public function testAddNamespaceWithArray()
], 'namespaces', $resolver);
}

/**
* @test
*/
public function testAddNamespaceWithNull()
{
$resolver = $this->getMock(ViewHelperResolver::class, ['dummy']);
$resolver->addNamespace('ignored', null);
$this->assertAttributeEquals(['f' => ['TYPO3Fluid\\Fluid\\ViewHelpers'], 'ignored' => null], 'namespaces', $resolver);
}

/**
* @test
*/
public function testAddSecondNamespaceWithNullWithExistingNullStillIgnoresNamespace()
{
$resolver = $this->getMock(ViewHelperResolver::class, ['dummy']);
$resolver->addNamespace('ignored', null);
$resolver->addNamespace('ignored', null);
$this->assertAttributeEquals(['f' => ['TYPO3Fluid\\Fluid\\ViewHelpers'], 'ignored' => null], 'namespaces', $resolver);
}

/**
* @test
*/
public function testAddSecondNamespaceWithExistingNullConvertsToNotIgnoredNamespace()
{
$resolver = $this->getMock(ViewHelperResolver::class, ['dummy']);
$resolver->addNamespace('ignored', null);
$resolver->addNamespace('ignored', ['Foo\\Bar']);
$this->assertAttributeEquals(['f' => ['TYPO3Fluid\\Fluid\\ViewHelpers'], 'ignored' => ['Foo\\Bar']], 'namespaces', $resolver);
}

/**
* @test
*/
Expand Down
1 change: 1 addition & 0 deletions tests/Unit/View/TemplatePathsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public function setsLayoutPathAndFilename()
$instance->expects($this->any())->method('sanitizePath')->willReturnArgument(0);
$instance->setLayoutPathAndFilename('foobar');
$this->assertAttributeEquals('foobar', 'layoutPathAndFilename', $instance);
$this->assertEquals('foobar', $instance->getLayoutPathAndFilename());
}

/**
Expand Down
6 changes: 5 additions & 1 deletion tests/Unit/ViewHelpers/CountViewHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* See LICENSE.txt that was shipped with this package.
*/

use TYPO3Fluid\Fluid\Core\ViewHelper\Exception;
use TYPO3Fluid\Fluid\Tests\Unit\Core\Rendering\RenderingContextFixture;
use TYPO3Fluid\Fluid\ViewHelpers\CountViewHelper;

/**
Expand Down Expand Up @@ -94,12 +96,14 @@ public function renderReturnsZeroIfGivenSubjectIsNullAndRenderChildrenReturnsNul

/**
* @test
* @expectedException \InvalidArgumentException
*/
public function renderThrowsExceptionIfGivenSubjectIsNotCountable()
{
$this->viewHelper->expects($this->never())->method('renderChildren');
$this->viewHelper->setRenderingContext(new RenderingContextFixture());
$object = new \stdClass();
$this->viewHelper->setArguments(['subject' => $object]);
$this->setExpectedException(\InvalidArgumentException::class);
$this->viewHelper->initializeArgumentsAndRender();
}
}

0 comments on commit 1a7947d

Please sign in to comment.