[tainting] Fix the precedence of the CachedTemplatesMapping (#89)

Allow alternatives template name notation Isolate template naming in a CachedTemplatesRegistry Allow `render` calls with no second arguments Allow twig template name old notation alternatives
psalm · Nov 10, 2020 · f75effe · f75effe
1 parent 01b5dcb
commit f75effe
Show file tree

Hide file tree

Showing 8 changed files with 329 additions and 47 deletions.
diff --git a/src/Test/CodeceptionModule.php b/src/Test/CodeceptionModule.php
@@ -5,7 +5,9 @@
 namespace Psalm\SymfonyPsalmPlugin\Test;
 
 use Codeception\Module as BaseModule;
+use Codeception\TestInterface;
 use InvalidArgumentException;
+use Psalm\SymfonyPsalmPlugin\Twig\CachedTemplatesMapping;
 use Twig\Cache\FilesystemCache;
 use Twig\Environment;
 use Twig\Extension\AbstractExtension;
@@ -24,21 +26,39 @@ class CodeceptionModule extends BaseModule
 
     public const TWIG_TEMPLATES_DIR = 'templates';
 
+    /**
+     * @var FilesystemCache|null
+     */
+    private $twigCache;
+
+    /**
+     * @var string|null
+     */
+    private $lastCachePath;
+
+    public function _after(TestInterface $test): void
+    {
+        $this->twigCache = $this->lastCachePath = null;
+    }
+
     /**
      * @Given I have the following :templateName template :code
      */
     public function haveTheFollowingTemplate(string $templateName, string $code): void
     {
         $rootDirectory = rtrim($this->config['default_dir'], DIRECTORY_SEPARATOR);
-        $templateRootDirectory = $rootDirectory.DIRECTORY_SEPARATOR.self::TWIG_TEMPLATES_DIR;
-        if (!file_exists($templateRootDirectory)) {
-            mkdir($templateRootDirectory);
+        $templatePath = (
+            $rootDirectory.DIRECTORY_SEPARATOR.
+            self::TWIG_TEMPLATES_DIR.DIRECTORY_SEPARATOR.
+            $templateName
+        );
+
+        $templateDirectory = dirname($templatePath);
+        if (!file_exists($templateDirectory)) {
+            mkdir($templateDirectory, 0755, true);
         }
 
-        file_put_contents(
-            $templateRootDirectory.DIRECTORY_SEPARATOR.$templateName,
-            $code
-        );
+        file_put_contents($templatePath, $code);
     }
 
     /**
@@ -52,25 +72,57 @@ public function haveTheTemplateCompiled(string $templateName, string $cacheDirec
             mkdir($cacheDirectory, 0755, true);
         }
 
-        $twigEnvironment = self::getEnvironment($rootDirectory, $cacheDirectory);
-        $twigEnvironment->load($templateName);
+        $this->loadTemplate($templateName, $rootDirectory, $cacheDirectory);
+    }
+
+    /**
+     * @Given the last compiled template got his alias changed to :newAlias
+     */
+    public function changeTheLastTemplateAlias(string $newAlias): void
+    {
+        if (null === $this->lastCachePath) {
+            throw new \RuntimeException('You have to compile a template first.');
+        }
+
+        $cacheContent = file_get_contents($this->lastCachePath);
+
+        if (!preg_match('/'.CachedTemplatesMapping::CACHED_TEMPLATE_HEADER_PATTERN.'/m', $cacheContent, $cacheHeadParts)) {
+            throw new \RuntimeException('The cache file is somehow malformed.');
+        }
+
+        file_put_contents($this->lastCachePath, str_replace(
+            $cacheHeadParts[0],
+            str_replace($cacheHeadParts['name'], $newAlias, $cacheHeadParts[0]),
+            $cacheContent
+        ));
     }
 
-    private static function getEnvironment(string $rootDirectory, string $cacheDirectory): Environment
+    private function loadTemplate(string $templateName, string $rootDirectory, string $cacheDirectory): void
+    {
+        if (null === $this->twigCache) {
+            if (!is_dir($cacheDirectory)) {
+                throw new InvalidArgumentException(sprintf('The %s twig cache directory does not exist or is not readable.', $cacheDirectory));
+            }
+            $this->twigCache = new FilesystemCache($cacheDirectory);
+        }
+
+        $twigEnvironment = self::getEnvironment($rootDirectory, $this->twigCache);
+        $template = $twigEnvironment->load($templateName);
+
+        /** @psalm-suppress InternalMethod */
+        $this->lastCachePath = $this->twigCache->generateKey($templateName, get_class($template->unwrap()));
+    }
+
+    private static function getEnvironment(string $rootDirectory, FilesystemCache $twigCache): Environment
     {
         if (!file_exists($rootDirectory.DIRECTORY_SEPARATOR.self::TWIG_TEMPLATES_DIR)) {
             mkdir($rootDirectory.DIRECTORY_SEPARATOR.self::TWIG_TEMPLATES_DIR);
         }
 
         $loader = new FilesystemLoader(self::TWIG_TEMPLATES_DIR, $rootDirectory);
 
-        if (!is_dir($cacheDirectory)) {
-            throw new InvalidArgumentException(sprintf('The %s twig cache directory does not exist or is not readable.', $cacheDirectory));
-        }
-        $cache = new FilesystemCache($cacheDirectory);
-
         $twigEnvironment = new Environment($loader, [
-            'cache' => $cache,
+            'cache' => $twigCache,
             'auto_reload' => true,
             'debug' => true,
             'optimizations' => 0,

diff --git a/src/Twig/CachedTemplateNotFoundException.php b/src/Twig/CachedTemplateNotFoundException.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\SymfonyPsalmPlugin\Twig;
+
+use Exception;
+
+class CachedTemplateNotFoundException extends Exception
+{
+    public function __construct()
+    {
+        parent::__construct('No cache found for template with name(s) :');
+    }
+
+    public function addTriedName(string $possibleName): void
+    {
+        $this->message .= ' '.$possibleName;
+    }
+}
diff --git a/src/Twig/CachedTemplatesMapping.php b/src/Twig/CachedTemplatesMapping.php
@@ -5,68 +5,68 @@
 namespace Psalm\SymfonyPsalmPlugin\Twig;
 
 use Psalm\Codebase;
-use Psalm\Context;
-use Psalm\Plugin\Hook\AfterFileAnalysisInterface;
-use Psalm\StatementsSource;
-use Psalm\Storage\FileStorage;
+use Psalm\Plugin\Hook\AfterCodebasePopulatedInterface;
 use RuntimeException;
 
 /**
  * This class is used to store a mapping of all analyzed twig template cache files with their corresponding actual templates.
  */
-class CachedTemplatesMapping implements AfterFileAnalysisInterface
+class CachedTemplatesMapping implements AfterCodebasePopulatedInterface
 {
     /**
      * @var string
      */
-    private const CACHED_TEMPLATE_HEADER_PATTERN = 'use Twig\\\\Template;\n\n\/\* (@?.+\.twig) \*\/\nclass __TwigTemplate';
+    public const CACHED_TEMPLATE_HEADER_PATTERN =
+        'use Twig\\\\Template;\n\n'.
+        '\/\* (?<name>@?.+\.twig) \*\/\n'.
+        'class (?<class>__TwigTemplate_[a-z0-9]{64}) extends (\\\\Twig\\\\)?Template';
 
     /**
      * @var string|null
      */
-    public static $cache_path;
+    private static $cachePath;
 
     /**
-     * @var array<string, string>
+     * @var CachedTemplatesRegistry|null
      */
-    private static $mapping = [];
+    private static $cacheRegistry;
 
-    public static function afterAnalyzeFile(StatementsSource $statements_source, Context $file_context, FileStorage $file_storage, Codebase $codebase): void
+    public static function afterCodebasePopulated(Codebase $codebase)
     {
-        if (null === self::$cache_path || 0 !== strpos($file_storage->file_path, self::$cache_path)) {
+        if (null === self::$cachePath) {
             return;
         }
 
-        $rawSource = file_get_contents($file_storage->file_path);
-        if (!preg_match('/'.self::CACHED_TEMPLATE_HEADER_PATTERN.'/m', $rawSource, $matchingParts)) {
-            return;
-        }
+        self::$cacheRegistry = new CachedTemplatesRegistry();
+        $cacheFiles = $codebase->file_provider->getFilesInDir(self::$cachePath, ['php']);
 
-        /** @var string|null $cacheClassName */
-        [$cacheClassName] = array_values($file_storage->classlikes_in_file);
-        if (null === $cacheClassName) {
-            return;
-        }
+        foreach ($cacheFiles as $file) {
+            $rawSource = $codebase->file_provider->getContents($file);
 
-        self::registerNewCache($cacheClassName, $matchingParts[1]);
-    }
+            if (!preg_match('/'.self::CACHED_TEMPLATE_HEADER_PATTERN.'/m', $rawSource, $matchingParts)) {
+                continue;
+            }
+            $templateName = $matchingParts['name'];
+            $cacheClassName = $matchingParts['class'];
 
-    public static function setCachePath(string $cache_path): void
-    {
-        static::$cache_path = $cache_path;
+            self::$cacheRegistry->addTemplate($cacheClassName, $templateName);
+        }
     }
 
-    private static function registerNewCache(string $cacheClassName, string $templateName): void
+    public static function setCachePath(string $cachePath): void
     {
-        static::$mapping[$templateName] = $cacheClassName;
+        self::$cachePath = $cachePath;
     }
 
+    /**
+     * @throws CachedTemplateNotFoundException
+     */
     public static function getCacheClassName(string $templateName): string
     {
-        if (!array_key_exists($templateName, static::$mapping)) {
-            throw new RuntimeException(sprintf('The template %s was not found.', $templateName));
+        if (null === self::$cacheRegistry) {
+            throw new RuntimeException(sprintf('Can not load template %s, because no cache registry is provided.', $templateName));
         }
 
-        return static::$mapping[$templateName];
+        return self::$cacheRegistry->getCacheClassName($templateName);
     }
 }
diff --git a/src/Twig/CachedTemplatesRegistry.php b/src/Twig/CachedTemplatesRegistry.php
@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\SymfonyPsalmPlugin\Twig;
+
+use Generator;
+
+class CachedTemplatesRegistry
+{
+    /**
+     * @var array<string, string>
+     */
+    private $mapping = [];
+
+    public function addTemplate(string $cacheClassName, string $templateName): void
+    {
+        $this->mapping[$templateName] = $cacheClassName;
+    }
+
+    /**
+     * @throws CachedTemplateNotFoundException
+     */
+    public function getCacheClassName(string $templateName): string
+    {
+        $probableException = new CachedTemplateNotFoundException();
+
+        foreach (self::generateNames($templateName) as $possibleName) {
+            if (array_key_exists($possibleName, $this->mapping)) {
+                return $this->mapping[$possibleName];
+            }
+            $probableException->addTriedName($possibleName);
+        }
+
+        throw $probableException;
+    }
+
+    /**
+     * @return Generator<string>
+     */
+    private static function generateNames(string $baseName): Generator
+    {
+        yield $baseName;
+
+        /** @var string|null $oldNotation */
+        $oldNotation = null;
+
+        $alternativeNotation = preg_replace('/^@([^\/]+)\/?(.+)?\/([^\/]+\.twig)/', '$1Bundle:$2:$3', $baseName);
+        if ($alternativeNotation !== $baseName) {
+            yield $alternativeNotation;
+            $oldNotation = $alternativeNotation;
+        }
+
+        $alternativeNotation = preg_replace('/^(.+)Bundle:(.+)?:(.+\.twig)$/', '@$1/$2/$3', $baseName);
+        if ($alternativeNotation !== $baseName) {
+            yield str_replace('//', '/', $alternativeNotation);
+            $oldNotation = $baseName;
+        }
+
+        if (null !== $oldNotation) {
+            list($bundleName, $rest) = explode(':', $oldNotation, 2);
+            list($revTemplateName, $revRest) = explode(':', strrev($rest), 2);
+            $pathParts = explode('/', strrev($revRest));
+            $pathParts = array_merge($pathParts, explode('/', strrev($revTemplateName)));
+            for ($i = 0; $i <= count($pathParts); ++$i) {
+                yield $bundleName.':'.
+                    implode('/', array_slice($pathParts, 0, $i)).':'.
+                    implode('/', array_slice($pathParts, $i));
+            }
+        }
+    }
+}
diff --git a/src/Twig/CachedTemplatesTainter.php b/src/Twig/CachedTemplatesTainter.php
@@ -56,7 +56,7 @@ public static function getMethodReturnType(
             new Identifier(
                 'doDisplay'
             ),
-            [$call_args[1]]
+            isset($call_args[1]) ? [$call_args[1]] : []
         );
 
         $firstArgument = $call_args[0]->value;

diff --git a/tests/acceptance/acceptance/TwigTaintingWithAnalyzer.feature b/tests/acceptance/acceptance/TwigTaintingWithAnalyzer.feature
@@ -36,6 +36,20 @@ Feature: Twig tainting with analyzer
       function twig() {}
       """
 
+  Scenario: The twig rendering has no parameters
+    Given I have the following code
+      """
+      twig()->render('index.html.twig');
+      """
+    And I have the following "index.html.twig" template
+      """
+      <h1>
+        Nothing.
+      </h1>
+      """
+    When I run Psalm with taint analysis
+    And I see no errors
+
   Scenario: One parameter of the twig rendering is tainted but autoescaping is on
     Given I have the following code
       """