Merge branch '3.x' into 4.x

* 3.x: Remove obsolete code Update CHANGELOG Sandbox ArrayAccess and do sandbox checks before isset() checks Fix sandbox handling for __toString() Fix mistake in docs for `keys` filter Update CHANGELOG [String] Add SpanishInflector support for singular and plural Add `find` to `Filters` docs Remove duplicate test case Revert "minor #4411 Add return type to getDebugInfo (ruudk)" Add `$this` return type to Template::unwrap Add link to TwigQI in docs "You might also be interested in" section Add return type to compiled macro
twigphp · Nov 6, 2024 · 429b13c · 429b13c
2 parents c949f3e + 32e99c2
commit 429b13c
Show file tree

Hide file tree

Showing 18 changed files with 222 additions and 39 deletions.
diff --git a/doc/filters/index.rst b/doc/filters/index.rst
@@ -18,6 +18,7 @@ Filters
     default
     escape
     filter
+    find
     first
     format
     format_currency

diff --git a/doc/filters/keys.rst b/doc/filters/keys.rst
@@ -6,10 +6,10 @@ when you want to iterate over the keys of a sequence or a mapping:
 
 .. code-block:: twig
 
-    {% for key in [1, 2, 3, 4]|keys %}
+    {% for key in ['a', 'b', 'c', 'd']|keys %}
         {{ key }}
     {% endfor %}
-    {# outputs: 1 2 3 4 #}
+    {# outputs: 0 1 2 3 #}
 
     {% for key in {a: 'a_value', b: 'b_value'}|keys %}
         {{ key }}

diff --git a/doc/sandbox.rst b/doc/sandbox.rst
@@ -37,6 +37,15 @@ able to call the ``getTitle()`` and ``getBody()`` methods on ``Article``
 objects, and the ``title`` and ``body`` public properties. Everything else
 won't be allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception.
 
+.. note::
+
+    As of Twig 1.14.1 (and on Twig 3.11.2), if the ``Article`` class implements
+    the ``ArrayAccess`` interface, the templates will only be able to access
+    the ``title`` and ``body`` attributes.
+
+    Note that native array-like classes (like ``ArrayObject``) are always
+    allowed, you don't need to configure them.
+
 .. caution::
 
     The ``extends`` and ``use`` tags are always allowed in a sandboxed

diff --git a/doc/templates.rst b/doc/templates.rst
@@ -69,6 +69,10 @@ You might also be interested in:
 * `Twig Language Server`_: provides some language features like syntax
   highlighting, diagnostics, auto complete, ...
 
+* `TwigQI`_: an extension which analyzes your templates for common bugs during compilation
+
+* `TwigStan`_: a static analyzer for Twig templates powered by PHPStan
+
 Variables
 ---------
 
@@ -583,7 +587,7 @@ exist:
    * ``\x``: Hexadecimal escape sequence
    * ``\0`` to ``\377``: Octal escape sequences representing characters
    * ``\``: Backslash
-  
+
    When using single-quoted strings, the single quote character (``'``) needs to be escaped with a backslash (``\'``).
    When using double-quoted strings, the double quote character (``"``) needs to be escaped with a backslash (``\"``).
 
@@ -1110,6 +1114,8 @@ Twig can be extended. If you want to create your own extensions, read the
 .. _`regular expressions`:                  https://www.php.net/manual/en/pcre.pattern.php
 .. _`PHP-twig for atom`:                    https://github.com/reesef/php-twig
 .. _`TwigFiddle`:                           https://twigfiddle.com/
+.. _`TwigQI`:                               https://github.com/alisqi/TwigQI
+.. _`TwigStan`:                             https://github.com/twigstan/twigstan
 .. _`Twig pack`:                            https://marketplace.visualstudio.com/items?itemName=bajdzis.vscode-twig-pack
 .. _`Modern Twig`:                          https://marketplace.visualstudio.com/items?itemName=Stanislav.vscode-twig
 .. _`Twig Language Server`:                 https://github.com/kaermorchen/twig-language-server/tree/master/packages/language-server

diff --git a/extra/string-extra/StringExtension.php b/extra/string-extra/StringExtension.php
@@ -15,17 +15,20 @@
 use Symfony\Component\String\Inflector\EnglishInflector;
 use Symfony\Component\String\Inflector\FrenchInflector;
 use Symfony\Component\String\Inflector\InflectorInterface;
+use Symfony\Component\String\Inflector\SpanishInflector;
 use Symfony\Component\String\Slugger\AsciiSlugger;
 use Symfony\Component\String\Slugger\SluggerInterface;
 use Symfony\Component\String\UnicodeString;
+use Twig\Error\RuntimeError;
 use Twig\Extension\AbstractExtension;
 use Twig\TwigFilter;
 
 final class StringExtension extends AbstractExtension
 {
     private $slugger;
-    private $frenchInflector;
     private $englishInflector;
+    private $spanishInflector;
+    private $frenchInflector;
 
     public function __construct(?SluggerInterface $slugger = null)
     {
@@ -73,10 +76,15 @@ public function singular(string $value, string $locale = 'en', bool $all = false
     private function getInflector(string $locale): InflectorInterface
     {
         switch ($locale) {
-            case 'fr':
-                return $this->frenchInflector ?? $this->frenchInflector = new FrenchInflector();
             case 'en':
                 return $this->englishInflector ?? $this->englishInflector = new EnglishInflector();
+            case 'es':
+                if (!class_exists(SpanishInflector::class)) {
+                    throw new RuntimeError('SpanishInflector is not available.');
+                }
+                return $this->spanishInflector ?? $this->spanishInflector = new SpanishInflector();
+            case 'fr':
+                return $this->frenchInflector ?? $this->frenchInflector = new FrenchInflector();
             default:
                 throw new \InvalidArgumentException(\sprintf('Locale "%s" is not supported.', $locale));
         }

diff --git a/extra/string-extra/Tests/Fixtures/plural.test b/extra/string-extra/Tests/Fixtures/plural.test
@@ -5,6 +5,8 @@
 {{ 'partition'|plural('fr', all=true)|join(',') }}
 {{ 'person'|plural('fr') }}
 {{ 'person'|plural('en', all=true)|join(',') }}
+{{ 'avión'|plural('es') }}
+{{ 'avión'|plural('es', all=true)|join(',') }}
 
 --DATA--
 return []
@@ -13,3 +15,5 @@ partitions
 partitions
 persons
 persons,people
+aviones
+aviones
diff --git a/extra/string-extra/Tests/Fixtures/singular.test b/extra/string-extra/Tests/Fixtures/singular.test
@@ -7,6 +7,8 @@
 {{ 'persons'|singular('en', all=true)|join(',') }}
 {{ 'people'|singular('en') }}
 {{ 'people'|singular('en', all=true)|join(',') }}
+{{ 'personas'|singular('es') }}
+{{ 'personas'|singular('es', all=true)|join(',') }}
 
 --DATA--
 return []
@@ -17,3 +19,5 @@ person
 person
 person
 person
+persona
+persona
diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php
@@ -71,6 +71,8 @@
 use Twig\Node\Expression\Variable\ContextVariable;
 use Twig\Node\Node;
 use Twig\Parser;
+use Twig\Sandbox\SecurityNotAllowedMethodError;
+use Twig\Sandbox\SecurityNotAllowedPropertyError;
 use Twig\Source;
 use Twig\Template;
 use Twig\TemplateWrapper;
@@ -101,6 +103,20 @@ final class CoreExtension extends AbstractExtension
 {
     private const DEFAULT_TRIM_CHARS = " \t\n\r\0\x0B";
 
+    public const ARRAY_LIKE_CLASSES = [
+        'ArrayIterator',
+        'ArrayObject',
+        'CachingIterator',
+        'RecursiveArrayIterator',
+        'RecursiveCachingIterator',
+        'SplDoublyLinkedList',
+        'SplFixedArray',
+        'SplObjectStorage',
+        'SplQueue',
+        'SplStack',
+        'WeakMap',
+    ];
+
     private array $dateFormats = ['F j, Y H:i', '%d days'];
     private array $numberFormat = [0, '.', ','];
     private ?\DateTimeZone $timezone = null;
@@ -1556,10 +1572,20 @@ public static function batch($items, $size, $fill = null, $preserveKeys = true):
      */
     public static function getAttribute(Environment $env, Source $source, $object, $item, array $arguments = [], $type = Template::ANY_CALL, $isDefinedTest = false, $ignoreStrictCheck = false, $sandboxed = false, int $lineno = -1)
     {
+        $propertyNotAllowedError = null;
+
         // array
         if (Template::METHOD_CALL !== $type) {
             $arrayItem = \is_bool($item) || \is_float($item) ? (int) $item : $item;
 
+            if ($sandboxed && $object instanceof \ArrayAccess && !\in_array($object::class, self::ARRAY_LIKE_CLASSES, true)) {
+                try {
+                    $env->getExtension(SandboxExtension::class)->checkPropertyAllowed($object, $arrayItem, $lineno, $source);
+                } catch (SecurityNotAllowedPropertyError $propertyNotAllowedError) {
+                    goto methodCheck;
+                }
+            }
+
             if (((\is_array($object) || $object instanceof \ArrayObject) && (isset($object[$arrayItem]) || \array_key_exists($arrayItem, (array) $object)))
                 || ($object instanceof \ArrayAccess && isset($object[$arrayItem]))
             ) {
@@ -1631,15 +1657,19 @@ public static function getAttribute(Environment $env, Source $source, $object, $
 
         // object property
         if (Template::METHOD_CALL !== $type) {
+            if ($sandboxed) {
+                try {
+                    $env->getExtension(SandboxExtension::class)->checkPropertyAllowed($object, $item, $lineno, $source);
+                } catch (SecurityNotAllowedPropertyError $propertyNotAllowedError) {
+                    goto methodCheck;
+                }
+            }
+
             if (isset($object->$item) || \array_key_exists((string) $item, (array) $object)) {
                 if ($isDefinedTest) {
                     return true;
                 }
 
-                if ($sandboxed) {
-                    $env->getExtension(SandboxExtension::class)->checkPropertyAllowed($object, $item, $lineno, $source);
-                }
-
                 return isset($object->$item) ? $object->$item : ((array) $object)[(string) $item];
             }
 
@@ -1656,6 +1686,8 @@ public static function getAttribute(Environment $env, Source $source, $object, $
             }
         }
 
+        methodCheck:
+
         static $cache = [];
 
         $class = $object::class;
@@ -1714,19 +1746,35 @@ public static function getAttribute(Environment $env, Source $source, $object, $
                 return false;
             }
 
+            if ($propertyNotAllowedError) {
+                throw $propertyNotAllowedError;
+            }
+
             if ($ignoreStrictCheck || !$env->isStrictVariables()) {
                 return;
             }
 
             throw new RuntimeError(\sprintf('Neither the property "%1$s" nor one of the methods "%1$s()", "get%1$s()"/"is%1$s()"/"has%1$s()" or "__call()" exist and have public access in class "%2$s".', $item, $class), $lineno, $source);
         }
 
-        if ($isDefinedTest) {
-            return true;
+        if ($sandboxed) {
+            try {
+                $env->getExtension(SandboxExtension::class)->checkMethodAllowed($object, $method, $lineno, $source);
+            } catch (SecurityNotAllowedMethodError $e) {
+                if ($isDefinedTest) {
+                    return false;
+                }
+
+                if ($propertyNotAllowedError) {
+                    throw $propertyNotAllowedError;
+                }
+
+                throw $e;
+            }
         }
 
-        if ($sandboxed) {
-            $env->getExtension(SandboxExtension::class)->checkMethodAllowed($object, $method, $lineno, $source);
+        if ($isDefinedTest) {
+            return true;
         }
 
         // Some objects throw exceptions when they have __call, and the method we try

diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php
@@ -113,6 +113,14 @@ public function checkPropertyAllowed($obj, $property, int $lineno = -1, ?Source
 
     public function ensureToStringAllowed($obj, int $lineno = -1, ?Source $source = null)
     {
+        if (\is_array($obj)) {
+            foreach ($obj as $v) {
+                $this->ensureToStringAllowed($v, $lineno, $source);
+            }
+
+            return $obj;
+        }
+
         if ($this->isSandboxed($source) && $obj instanceof \Stringable) {
             try {
                 $this->policy->checkMethodAllowed($obj, '__toString');

diff --git a/src/Node/Expression/GetAttrExpression.php b/src/Node/Expression/GetAttrExpression.php
@@ -32,6 +32,7 @@ public function __construct(AbstractExpression $node, AbstractExpression $attrib
     public function compile(Compiler $compiler): void
     {
         $env = $compiler->getEnvironment();
+        $arrayAccessSandbox = false;
 
         // optimize array calls
         if (
@@ -45,17 +46,35 @@ public function compile(Compiler $compiler): void
                 ->raw('(('.$var.' = ')
                 ->subcompile($this->getNode('node'))
                 ->raw(') && is_array(')
-                ->raw($var)
+                ->raw($var);
+
+            if (!$env->hasExtension(SandboxExtension::class)) {
+                $compiler
+                    ->raw(') || ')
+                    ->raw($var)
+                    ->raw(' instanceof ArrayAccess ? (')
+                    ->raw($var)
+                    ->raw('[')
+                    ->subcompile($this->getNode('attribute'))
+                    ->raw('] ?? null) : null)')
+                ;
+
+                return;
+            }
+
+            $arrayAccessSandbox = true;
+
+            $compiler
                 ->raw(') || ')
                 ->raw($var)
-                ->raw(' instanceof ArrayAccess ? (')
+                ->raw(' instanceof ArrayAccess && in_array(')
+                ->raw($var.'::class')
+                ->raw(', CoreExtension::ARRAY_LIKE_CLASSES, true) ? (')
                 ->raw($var)
                 ->raw('[')
                 ->subcompile($this->getNode('attribute'))
-                ->raw('] ?? null) : null)')
+                ->raw('] ?? null) : ')
             ;
-
-            return;
         }
 
         $compiler->raw('CoreExtension::getAttribute($this->env, $this->source, ');
@@ -94,5 +113,9 @@ public function compile(Compiler $compiler): void
             ->raw(', lineno: ')->repr($this->getNode('node')->getTemplateLine())
             ->raw(')')
         ;
+
+        if ($arrayAccessSandbox) {
+            $compiler->raw(')');
+        }
     }
 }
diff --git a/src/Node/MacroNode.php b/src/Node/MacroNode.php
@@ -14,6 +14,7 @@
 use Twig\Attribute\YieldReady;
 use Twig\Compiler;
 use Twig\Error\SyntaxError;
+use Twig\Markup;
 use Twig\Node\Expression\ArrayExpression;
 use Twig\Node\Expression\Variable\LocalVariable;
 
@@ -58,7 +59,7 @@ public function compile(Compiler $compiler): void
 
         $compiler
             ->raw('...$varargs')
-            ->raw(")\n")
+            ->raw("): string|Markup\n")
             ->write("{\n")
             ->indent()
             ->write("\$macros = \$this->macros;\n")

diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php
@@ -436,7 +436,6 @@ protected function compileDebugInfo(Compiler $compiler)
         $compiler
             ->write("/**\n")
             ->write(" * @codeCoverageIgnore\n")
-            ->write(" * @return array<int, int>\n")
             ->write(" */\n")
             ->write("public function getDebugInfo(): array\n", "{\n")
             ->indent()

diff --git a/src/NodeVisitor/SandboxNodeVisitor.php b/src/NodeVisitor/SandboxNodeVisitor.php
@@ -15,12 +15,14 @@
 use Twig\Node\CheckSecurityCallNode;
 use Twig\Node\CheckSecurityNode;
 use Twig\Node\CheckToStringNode;
+use Twig\Node\Expression\ArrayExpression;
 use Twig\Node\Expression\Binary\ConcatBinary;
 use Twig\Node\Expression\Binary\RangeBinary;
 use Twig\Node\Expression\FilterExpression;
 use Twig\Node\Expression\FunctionExpression;
 use Twig\Node\Expression\GetAttrExpression;
 use Twig\Node\Expression\Variable\ContextVariable;
+use Twig\Node\Expression\Unary\SpreadUnary;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
 use Twig\Node\Nodes;
@@ -121,7 +123,18 @@ private function wrapNode(Node $node, string $name): void
     {
         $expr = $node->getNode($name);
         if (($expr instanceof ContextVariable || $expr instanceof GetAttrExpression) && !$expr->isGenerator()) {
-            $node->setNode($name, new CheckToStringNode($expr));
+            // Simplify in 4.0 as the spread attribute has been removed there
+            $new = new CheckToStringNode($expr);
+            if ($expr->hasAttribute('spread')) {
+                $new->setAttribute('spread', $expr->getAttribute('spread'));
+            }
+            $node->setNode($name, $new);
+        } elseif ($expr instanceof SpreadUnary) {
+            $this->wrapNode($expr, 'node');
+        } elseif ($expr instanceof ArrayExpression) {
+            foreach ($expr as $name => $_) {
+                $this->wrapNode($expr, $name);
+            }
         }
     }