refacto: code cleaning

.github/workflows/code-quality.yaml

@@ -40,6 +40,7 @@ jobs:
         outputs:
             lint: ${{ steps.filter.outputs.lint }}
             analyse: ${{ steps.filter.outputs.analyse }}
+            tests: ${{ steps.filter.outputs.tests }}
 
         steps:
             -   name: Checkout
@@ -51,8 +52,13 @@ jobs:
                     filters: |
                         lint:
                           - 'src/**'
+                          - 'tests/**'
                         analyse:
                           - 'src/**'
+                          - 'tests/**'
+                        tests:
+                          - 'src/**'
+                          - 'tests/**'
 
     lint:
         runs-on: ubuntu-latest
@@ -120,3 +126,34 @@ jobs:
                     php_version: "8.2"
                     version: 2
                     command: audit
+
+    test:
+        runs-on: ubuntu-latest
+
+        needs: [composer,changes]
+
+        if: ${{ needs.changes.outputs.tests == 'true' }}
+
+        steps:
+            -   name: Checkout
+                uses: actions/checkout@v4
+
+            -   name: Setup PHP with PCOV
+                uses: shivammathur/setup-php@v2
+                with:
+                    php-version: "8.2"
+                    extensions: gd, imagick, pcov
+                    coverage: pcov
+
+            -   name: Cache composer dependencies
+                uses: actions/cache@v4
+                with:
+                    key: composer-${{ hashFiles('composer.json') }}-${{ hashFiles('composer.lock') }}
+                    restore-keys: composer-
+                    path: vendor
+
+            -   name: Run PHPUnit with coverage
+                run: make test
+
+            -   name: Check coverage threshold
+                run: make coverage

Makefile

@@ -1,6 +1,6 @@
 -include docker/.env
 
-.SILENT: cache shell analyse
+.SILENT: cache shell analyse test coverage
 .DEFAULT_GOAL := help
 
 help:
@@ -44,7 +44,7 @@ cache:
 ## Code quality
 ##---------------------------------------------------------------------------
 
-check: lint analyse security
+check: lint analyse security test coverage
 
 lint: ## Execute PHPCS
 lint: cache
@@ -61,3 +61,11 @@ analyse: cache
 security: ## Check CVE for vendor dependencies
 security:
 	docker/exec composer audit
+
+test: ## Execute PHPUnit tests
+test: cache
+	docker/exec vendor/bin/phpunit --coverage-xml .cache/coverage/xml
+
+coverage: ## Check coverage threshold (95%)
+coverage:
+	docker/exec vendor/bin/coverage-checker .cache/coverage/xml/index.xml 75

composer.json

@@ -37,11 +37,19 @@
     },
     "require-dev": {
         "babeuloula/phpcs": "^1.5",
+        "babeuloula/phpunit-coverage-checker": "^1.0",
         "intervention/image": "^3.9",
         "phpstan/phpstan": "^1.12",
         "phpstan/phpstan-deprecation-rules": "^1.2",
+        "phpstan/phpstan-phpunit": "^1.0",
         "phpstan/phpstan-strict-rules": "^1.6",
-        "phpstan/phpstan-symfony": "^1.4"
+        "phpstan/phpstan-symfony": "^1.4",
+        "phpunit/phpunit": "^11.0"
+    },
+    "autoload-dev": {
+        "psr-4": {
+            "BaBeuloula\\CdnPhpBundle\\Tests\\": "tests/"
+        }
     },
     "suggest": {
         "intervention/image": "Needed to use InterventionImageFallback."

docker/php/Dockerfile

@@ -1,6 +1,6 @@
 FROM php:8.2-cli
 
-ARG DOCKER_PHP_EXT=2.6.0
+ARG DOCKER_PHP_EXT=2.10.12
 ADD https://github.com/mlocati/docker-php-extension-installer/releases/download/${DOCKER_PHP_EXT}/install-php-extensions /usr/local/bin/
 RUN chmod +x /usr/local/bin/install-php-extensions
 
@@ -25,6 +25,9 @@ RUN \
     rm -rf /var/lib/apt/lists/* && \
     truncate -s 0 /var/log/*log
 
+# Install PHP extensions
+RUN install-php-extensions gd imagick pcov
+
 # Install composer
 RUN \
     curl -sl https://getcomposer.org/composer-2.phar -o /usr/local/bin/composer && \

phpcs.xml

@@ -15,5 +15,6 @@
     </rule>
 
     <file>src/</file>
+    <file>tests/</file>
 
 </ruleset>

phpstan.neon

@@ -2,6 +2,7 @@ includes:
     - vendor/phpstan/phpstan-symfony/extension.neon
     - vendor/phpstan/phpstan-symfony/rules.neon
     - vendor/phpstan/phpstan-deprecation-rules/rules.neon
+    - vendor/phpstan/phpstan-phpunit/extension.neon
 
 parameters:
     tmpDir: .cache/phpstan
@@ -10,6 +11,7 @@ parameters:
 
     paths:
         - src
+        - tests
 
     parallel:
         jobSize: 20

phpunit.xml.dist

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
+         bootstrap="vendor/autoload.php"
+         cacheDirectory=".cache/phpunit"
+         colors="true"
+         failOnWarning="true"
+         failOnRisky="true">
+
+    <testsuites>
+        <testsuite name="CdnPhpBundle Test Suite">
+            <directory>tests/</directory>
+        </testsuite>
+    </testsuites>
+
+    <source>
+        <include>
+            <directory>src/</directory>
+        </include>
+    </source>
+
+    <coverage>
+        <report>
+            <xml outputDirectory=".cache/coverage/xml"/>
+        </report>
+    </coverage>
+
+</phpunit>

src/AbstractHandler.php

@@ -31,7 +31,6 @@ public function parseHeaders(Request $request): array
     {
         $requestedHeaders = [
             'accept-language',
-            'accept-encoding',
             'accept',
             'user-agent',
         ];

src/CdnPhpBundle.php

@@ -27,7 +27,7 @@
         /** @var ArrayNodeDefinition $treeBuilder */
         $treeBuilder = $definition->rootNode();
 
         $treeBuilder
             ->children()
                 ->arrayNode('proxy')->isRequired()
                     ->children()
@@ -80,6 +80,7 @@
             ->get(ProxyExtension::class)
                 ->arg('$routeName', $config['twig']['route_name'])
                 ->arg('$routeParameter', $config['twig']['route_parameter'])
+                ->arg('$encryptedParameters', $config['proxy']['encrypted_parameters'])
         ;
     }
 }

src/FallbackHandler/InterventionImageFallbackHandler.php

@@ -43,24 +43,32 @@ public function response(string $file, ?Options $options = null, array $headers
     {
         $file = $this->normalizeFile($file);
 
-        try {
-            $image = $this->imageManager->read($this->assetsPath . $file);
-        } catch (DecoderException $e) {
-            throw new FileNotFoundException($file, previous: $e);
-        }
+        $formatKey = match (true) {
+            $this->supportsAvif($headers) => 'avif',
+            $this->supportsWebp($headers) => 'webp',
+            default => 'original',
+        };
 
-        $cacheKey = sha1($file . '?' . $options?->buildQuery());
+        $safeFile = (string) preg_replace('/[^a-zA-Z0-9_\-\.]/', '_', $file);
+        $optionsPart = null !== $options ? '_' . sha1($options->buildQuery(false)) : '';
+        $cacheKey = $safeFile . $optionsPart . '.' . $formatKey;
 
         /** @var array{content: string, mimetype: string} $encodedImage */
         $encodedImage = $this->cache->get(
             $cacheKey,
-            function (ItemInterface $item) use ($options, $image, $headers): array {
+            function (ItemInterface $item) use ($options, $file, $headers): array {
+                try {
+                    $image = $this->imageManager->read($this->assetsPath . $file);
+                } catch (DecoderException $e) {
+                    throw new FileNotFoundException($file, previous: $e);
+                }
+
                 if (null !== $options?->width && null !== $options->height) {
-                    $image->cover((int) $options->width, (int) $options->height);
+                    $image->cover($options->width, $options->height);
                 } elseif (null !== $options?->width) {
-                    $image->scale(width: (int) $options->width);
+                    $image->scale(width: $options->width);
                 } elseif (null !== $options?->height) {
-                    $image->scale(height: (int) $options->height);
+                    $image->scale(height: $options->height);
                 }
 
                 $item->expiresAfter($this->cacheLifetime);
@@ -81,10 +89,7 @@ function (ItemInterface $item) use ($options, $image, $headers): array {
         return new Response(
             $encodedImage['content'],
             Response::HTTP_OK,
-            array_merge_recursive(
-                $headers,
-                ['Content-Type' => $encodedImage['mimetype']],
-            ),
+            array_replace($headers, ['Content-Type' => $encodedImage['mimetype']]),
         );
     }
 

src/Options.php

@@ -26,8 +26,8 @@ final class Options
     private const DEFAULT_SIGNATURE = null;
 
     public function __construct(
-        public readonly null|int|string $width = self::DEFAULT_WIDTH,
-        public readonly null|int|string $height = self::DEFAULT_HEIGHT,
+        public readonly ?int $width = self::DEFAULT_WIDTH,
+        public readonly ?int $height = self::DEFAULT_HEIGHT,
         public readonly ?string $watermarkUrl = self::DEFAULT_WATERMARK_URL,
         public readonly string $watermarkGravity = self::DEFAULT_WATERMARK_POSITION,
         public readonly int $watermarkScale = self::DEFAULT_WATERMARK_SCALE,
@@ -86,9 +86,12 @@ public function setSignature(string $signature): self
     /** @param array<int|string, mixed> $options */
     public static function fromArray(array $options): self
     {
+        $width = $options['width'] ?? $options['w'] ?? null;
+        $height = $options['height'] ?? $options['h'] ?? null;
+
         return new self(
-            $options['width'] ?? $options['w'] ?? self::DEFAULT_WIDTH,
-            $options['height'] ?? $options['h'] ?? self::DEFAULT_HEIGHT,
+            null !== $width ? (int) $width : self::DEFAULT_WIDTH,
+            null !== $height ? (int) $height : self::DEFAULT_HEIGHT,
             $options['watermarkUrl'] ?? $options['wu'] ?? $options['wat_url'] ?? self::DEFAULT_WATERMARK_URL,
             $options['watermarkPosition'] ?? $options['wp'] ?? $options['wat_position'] ?? self::DEFAULT_WATERMARK_POSITION,
             (int) ($options['watermarkScale'] ?? $options['ws'] ?? $options['wat_scale'] ?? self::DEFAULT_WATERMARK_SCALE),

src/Proxy.php

@@ -107,6 +107,6 @@ public function response(string $file, ?Options $options = null, array $headers
 
     private function exists(string $file): bool
     {
-        return $this->filesystem->exists($this->assetsPath . $this->normalizeFile($file));
+        return $this->filesystem->exists($this->assetsPath . $file);
     }
 }

src/Signer.php

@@ -13,8 +13,6 @@
 
 namespace BaBeuloula\CdnPhpBundle;
 
-use Defuse\Crypto\Crypto;
-
 final class Signer
 {
     public function __construct(
@@ -40,12 +38,12 @@ public function sign(Options $options): string
 
     public function isValid(Options $options): bool
     {
-        return $this->calcSignature($options) === $options->signature;
+        return hash_equals($this->calcSignature($options), (string) $options->signature);
     }
 
     public function calcSignature(Options $options): string
     {
-        return sha1($options->buildQuery(false) . $this->getSecretKey());
+        return hash_hmac('sha256', $options->buildQuery(false), $this->getSecretKey());
     }
 
     public function isCdnSigningEnabled(): bool

src/Twig/Extension/ProxyExtension.php

@@ -26,7 +26,8 @@ public function __construct(
         private readonly RouterInterface $router,
         private readonly string $routeName,
         private readonly string $routeParameter,
-        private readonly Signer $encrypter,
+        private readonly Signer $signer,
+        private readonly bool $encryptedParameters = false,
     ) {
     }
 
@@ -39,13 +40,12 @@ public function getFunctions(): array
     }
 
     /** @param array<string, mixed> $options */
-    public function cdnPhp(string $file, array $options = [], bool $enableEncrypter = true): string
+    public function cdnPhp(string $file, array $options = [], ?bool $enableEncrypter = null): string
     {
         $options = Options::fromArray($options);
-        $queryParams = (true === $enableEncrypter)
-            ? '?' . $this->encrypter->sign($options)
-            : '?' . $options->buildQuery()
-        ;
+        $useEncrypter = $enableEncrypter ?? $this->encryptedParameters;
+        $query = true === $useEncrypter ? $this->signer->sign($options) : $options->buildQuery();
+        $queryParams = '' !== $query ? '?' . $query : '';
 
         return $this->router->generate(
             $this->routeName,