bug #17360 [Maintenance] Remove enshrined/svg-sanitize package (mpysiak)

This PR was merged into the 1.14 branch.
labels: bc-break

Discussion
----------

| Q               | A
|-----------------|-----
| Branch?         | 1.14
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | yes
| Deprecations?   | no
| Related tickets | n/a
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.13 branch
 - Features and deprecations must be submitted against the 1.14 branch
 - Features, removing deprecations and BC breaks must be submitted against the 2.0 branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

23f949521a Remove enshrined/svg-sanitize dependency
74be2d9351 Add allowed types configuration and change validation
b847995f96 Add tests
7d6b4d6e5b Adjust behat tests
0e0847c200 Add phpstan ignores
f044558058 Format fix
cd512e412d Fix composerRequireChecker
fe5910e8ac Skip svg sanitize test if package is not installed
d6fa473f9b Fix behat test
c3d603c5bd Add deprecations and configuration unit tests
f7a75e2cdb Add info to UPGRADE file
48dcd94f0e Changes after CR

UPGRADE-1.14.md

@@ -31,6 +31,18 @@
 
    The previous namespaces are still usable, but are considered deprecated and may be removed in future versions of `Resource` packages, update them at your own convenience.
 
+2. Package `enshrined/svg-sanitize` has been removed from the project due to license incompatibility with Sylius. You can still use it in your project by adding it manually:
+
+    ```bash
+    composer require enshrined/svg-sanitize
+    ```
+   Also, uploading `svg` image support has been removed. If you want to use `svg` images in your project, add `image/svg+xml` to the list of allowed mime types in the `CoreBundle` configuration.
+
+    ```yaml
+    sylius_core:
+        allowed_image_mime_types: ['image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/svg+xml']
+    ```
+
 ### Deprecations
 
 1. Aliases for the following services have been introduced to standardize service IDs and will replace the incorrect IDs in Sylius 2.0:
@@ -718,9 +730,6 @@
 1. The following classes have been deprecated and will be removed in Sylius 2.0:
     - **AdminBundle**
     - `Sylius\Bundle\AdminBundle\Controller\NotificationController`
-
-1. The following services have been deprecated and will be removed in Sylius 2.0:
-    - **AdminBundle**
     - `sylius.controller.admin.notification`
     - `Sylius\Bundle\AdminBundle\Controller\Dashboard\StatisticsController`
     - `Sylius\Bundle\AdminBundle\Form\Extension\CatalogPromotionActionTypeExtension`
@@ -805,6 +814,7 @@
     - `Sylius\Bundle\CoreBundle\Form\Extension\LocaleTypeExtension`
     - `Sylius\Bundle\CoreBundle\Form\Type\Grid\Filter\EntitiesFilterType`
     - `Sylius\Component\Core\Grid\Filter\EntitiesFilter`
+    - `Sylius\Bundle\CoreBundle\Twig\FilterExtension`
     - **PayumBundle**
     - `Sylius\Bundle\PayumBundle\Action\Paypal\ExpressCheckout\ConvertPaymentAction`
     - `Sylius\Bundle\PayumBundle\Controller\PayumController`

composer-require-checker.json

@@ -24,6 +24,7 @@
     "Behat\\Testwork\\Tester\\Setup\\Teardown",
     "Doctrine\\DBAL\\Platforms\\MySqlPlatform",
     "Doctrine\\DBAL\\Platforms\\MySQLPlatform",
+    "enshrined\\svgSanitize\\Sanitizer",
     "Http\\Client\\HttpClient",
     "HWI\\Bundle\\OAuthBundle\\Connect\\AccountConnectorInterface",
     "HWI\\Bundle\\OAuthBundle\\OAuth\\Response\\UserResponseInterface",

composer.json

@@ -44,7 +44,6 @@
         "doctrine/orm": "^2.13",
         "doctrine/persistence": "^2.3 || ^3.0",
         "egulias/email-validator": "^3.1",
-        "enshrined/svg-sanitize": "^0.16",
         "fakerphp/faker": "^1.10",
         "friendsofphp/proxy-manager-lts": "^1.0.7",
         "friendsofsymfony/rest-bundle": "^3.0",

features/product/managing_products/adding_images_to_existing_product.feature

@@ -17,7 +17,16 @@ Feature: Adding images to an existing product
         Then I should be notified that it has been successfully uploaded
         And the product "Lamborghini Gallardo Model" should have an image with "banner" type
 
-    @ui @mink:chromedriver @api
+    @api @ui @mink:chromedriver
+    Scenario: Trying to add svg image to an existing product
+        Given the store has a product "Lamborghini Gallardo Model"
+        When I want to modify this product
+        And I attach the "sylius.svg" image with "banner" type to this product
+        And I save my changes to the images
+        Then I should be notified that svg file is not allowed
+        And this product should not have any images
+
+    @api @ui @mink:chromedriver
     Scenario: Adding multiple images to an existing product
         Given the store has a product "Lamborghini Gallardo Model"
         When I want to modify this product

phpstan-baseline.neon

@@ -7179,3 +7179,14 @@ parameters:
 			message: "#^Method Sylius\\\\Component\\\\User\\\\Security\\\\Checker\\\\TokenUniquenessChecker\\:\\:__construct\\(\\) has parameter \\$repository with generic interface Sylius\\\\Component\\\\Resource\\\\Repository\\\\RepositoryInterface but does not specify its types\\: T$#"
 			count: 1
 			path: src/Sylius/Component/User/Security/Checker/TokenUniquenessChecker.php
+
+		-
+			message: "#^Call to method sanitize\\(\\) on an unknown class enshrined\\\\svgSanitize\\\\Sanitizer.$#"
+			count: 1
+			path: src/Sylius/Component/Core/Uploader/ImageUploader.php
+
+		-
+			message: "#^Property Sylius\\\\Component\\\\Core\\\\Uploader\\\\ImageUploader\\:\\:\\$sanitizer has unknown class enshrined\\\\svgSanitize\\\\Sanitizer as its type.$#"
+			count: 1
+			path: src/Sylius/Component/Core/Uploader/ImageUploader.php
+

src/Sylius/Behat/Context/Api/Admin/ManagingProductImagesContext.php

@@ -220,6 +220,17 @@ final class ManagingProductImagesContext implements Context
         );
     }
 
+    /**
+     * @Then I should be notified that svg file is not allowed
+     */
+    public function iShouldBeNotifiedThatSvgFileIsNotAllowed(): void
+    {
+        Assert::contains(
+            $this->responseChecker->getError($this->client->getLastResponse()),
+            'The file is not a valid image.',
+        );
+    }
+
     private function createProductImage(
         string $path,
         ProductInterface $product,

src/Sylius/Behat/Context/Ui/Admin/ManagingProductsContext.php

@@ -1081,6 +1081,14 @@ final class ManagingProductsContext implements Context
         );
     }
 
+    /**
+     * @Then I should be notified that svg file is not allowed
+     */
+    public function iShouldBeNotifiedThatSvgTypeIsNotAllowed(): void
+    {
+       $this->assertValidationMessage('images', 'The file is not a valid image. Allowed types are image/jpeg, image/png, image/gif, image/webp.');
+    }
+
     /**
      * @Then I should be notified that simple product code has to be unique
      */

src/Sylius/Behat/Resources/fixtures/sylius.svg

@@ -0,0 +1 @@
+<svg id="Warstwa_1" data-name="Warstwa 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1012.77 417.76"><defs><style>.cls-1{fill:#fff;}.cls-2{fill:#30ba9d;}.cls-3{fill:#1a9f83;}</style></defs><title>sylius-logo</title><path class="cls-1" d="M393.56,255.07s14.68,17.33,36.09,17.33c13.46,0,24.67-8.15,24.67-21.41,0-30.17-69.53-25.48-69.53-70.34,0-21.62,19-38.54,46.08-38.54,15.3,0,41,6.94,41,26.51v11.82H453.1v-6.72c0-7.55-10.81-13.05-22.23-13.05-15.49,0-25.28,8.56-25.28,19.36,0,29.16,69.33,22.64,69.33,69.94,0,22-17.13,41-45.67,41a64.37,64.37,0,0,1-47.72-21.4Z"/><path class="cls-1" d="M507.53,314.2c8,0,14.27-5.71,18.15-14.88l4.69-11.22-35.28-81.76c-1.42-3.27-3.26-4.08-6.52-4.08h-2.45V185.34h11.62c8.36,0,11.42,2.24,14.68,10.4l23.45,58.1a110.31,110.31,0,0,1,3.67,12.44h.4a93.93,93.93,0,0,1,3.47-12.44l21.41-58.1c3.06-8.16,6.73-10.4,15.09-10.4h11.83v16.92h-2.66c-3.25,0-5.09.81-6.52,4.08L541.78,307.06c-6.32,15.9-18.76,24.27-33.23,24.27a34.19,34.19,0,0,1-24.26-10.2l8.15-14.27C492.44,307.06,498.76,314.2,507.53,314.2Z"/><path class="cls-1" d="M611.5,166c0-3.06-1.63-4.49-4.48-4.49h-8.36V144.56h19.57c9.17,0,13,3.87,13,13.05V267.1c0,3.06,1.63,4.49,4.49,4.49h8.35v16.92H624.55c-9.17,0-13-3.88-13-13.05Z"/><path class="cls-1" d="M673.23,206.75c0-3.06-1.63-4.49-4.49-4.49h-8.36V185.34h19.38c9.17,0,13,3.87,13,13V267.1c0,3.06,1.63,4.49,4.49,4.49h8.36v16.92H686.28c-9.18,0-13-3.88-13-13.05Zm.41-62.19h17.73v20.59H673.64Z"/><path class="cls-1" d="M730.7,206.75c0-3.06-1.63-4.49-4.49-4.49h-8.36V185.34h19.37c9.38,0,13.25,3.87,13.25,12.64v48.93c0,14.28,2.86,25.69,19.17,25.69,21.21,0,33.64-18.55,33.64-38.94V185.34h19.78V267.1c0,3.06,1.63,4.49,4.49,4.49h8.35v16.92h-19c-8.76,0-13.05-4.08-13.05-11.42v-3.87c0-3.27.21-5.91.21-5.91h-.41C799.41,277.5,786.16,291,765.77,291c-23.25,0-35.07-12.23-35.07-39.55Z"/><path class="cls-1" d="M858.91,259.56s12.23,15.29,31.2,15.29c9.58,0,17.94-4.29,17.94-13.26,0-18.55-56.27-16.51-56.27-49.74,0-19.78,17.32-29,38.33-29,12.44,0,33.84,4.28,33.84,20v9.78H906.21v-5.1c0-6.12-9.17-8.77-15.49-8.77-11.62,0-19.17,4.09-19.17,12.24,0,19.57,56.28,15.29,56.28,49.75,0,18.35-16.31,30.17-37.93,30.17-27.52,0-41.18-18.14-41.18-18.14Z"/><polygon class="cls-2" points="119.37 230.23 168.96 329.41 260.49 329.56 282.64 298.38 119.37 230.23"/><polygon class="cls-3" points="84.95 241.12 151.62 332.64 260.49 332.82 282.64 298.58 84.95 241.12"/><polygon class="cls-3" points="164.89 129.63 197.34 154.8 226.68 98.5 216.71 92.12 164.89 129.63"/><polygon class="cls-2" points="260.49 332.82 164.89 129.63 185.34 114.83 296.58 277.01 260.49 332.82"/><polygon class="cls-2" points="207.51 98.78 269.78 149.08 226.08 84.95 207.51 98.78"/></svg>

src/Sylius/Bundle/ApiBundle/composer.json

@@ -26,7 +26,6 @@
         "php": "^8.1",
         "doctrine/dbal": "^3.0",
         "api-platform/core": "^2.7.10",
-        "enshrined/svg-sanitize": "^0.16",
         "lexik/jwt-authentication-bundle": "^2.11",
         "sylius/core-bundle": "^1.13",
         "symfony/messenger": "^5.4.21 || ^6.4"

src/Sylius/Bundle/CoreBundle/DependencyInjection/Compiler/LiipImageFiltersPass.php

@@ -19,8 +19,16 @@ use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 
+trigger_deprecation(
+    'sylius/sylius',
+    '1.14',
+    'The "%s" class is deprecated and will be removed in Sylius 2.0.',
+    LiipImageFiltersPass::class,
+);
 /**
  * @internal
+ *
+ * @deprecated
  */
 final class LiipImageFiltersPass implements CompilerPassInterface
 {

src/Sylius/Bundle/CoreBundle/DependencyInjection/Configuration.php

@@ -146,6 +146,10 @@ final class Configuration implements ConfigurationInterface
                         ->end()
                     ->end()
                 ->end()
+                ->arrayNode('allowed_images_mime_types')
+                    ->prototype('scalar')->end()
+                    ->defaultValue(['image/jpeg', 'image/png', 'image/gif', 'image/webp'])
+                ->end()
             ->end()
         ;
 

src/Sylius/Bundle/CoreBundle/DependencyInjection/SyliusCoreExtension.php

@@ -75,6 +75,7 @@ final class SyliusCoreExtension extends AbstractResourceExtension implements Pre
         $container->setParameter('sylius_core.price_history.batch_size', $config['price_history']['batch_size']);
         $container->setParameter('sylius_core.orders_statistics.intervals_map', $config['orders_statistics']['intervals_map'] ?? []);
         $container->setParameter('sylius_core.max_int_value', $config['max_int_value']);
+        $container->setParameter('sylius_core.allowed_images_mime_types', $config['allowed_images_mime_types']);
 
         /** @var string $env */
         $env = $container->getParameter('kernel.environment');

src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml

@@ -133,6 +133,12 @@
             <argument>%sylius_core.max_int_value%</argument>
             <tag name="validator.constraint_validator" alias="sylius_max_integer" />
         </service>
+
         <service id="sylius.validator.max_integer" alias="Sylius\Bundle\CoreBundle\Validator\Constraints\MaxIntegerValidator" />
+
+        <service id="sylius.validator.allowed_image_mime_types" class="Sylius\Bundle\CoreBundle\Validator\Constraints\AllowedImageMimeTypesValidator">
+            <argument>%sylius_core.allowed_images_mime_types%</argument>
+            <tag name="validator.constraint_validator" alias="sylius_image_allowed_mime_types_validator" />
+        </service>
     </services>
 </container>

src/Sylius/Bundle/CoreBundle/Resources/config/validation/AvatarImage.xml

@@ -20,6 +20,9 @@
                 <option name="uploadIniSizeErrorMessage">sylius.avatar_image.file.upload_ini_size</option>
                 <option name="groups">sylius</option>
             </constraint>
+            <constraint name="Sylius\Bundle\CoreBundle\Validator\Constraints\AllowedImageMimeTypes">
+                <option name="groups">sylius</option>
+            </constraint>
         </property>
     </class>
 </constraint-mapping>

src/Sylius/Bundle/CoreBundle/Resources/config/validation/ProductImage.xml

@@ -24,6 +24,9 @@
                 <option name="uploadIniSizeErrorMessage">sylius.product_image.file.upload_ini_size</option>
                 <option name="groups">sylius</option>
             </constraint>
+            <constraint name="Sylius\Bundle\CoreBundle\Validator\Constraints\AllowedImageMimeTypes">
+                <option name="groups">sylius</option>
+            </constraint>
         </property>
     </class>
 </constraint-mapping>

src/Sylius/Bundle/CoreBundle/Resources/config/validation/TaxonImage.xml

@@ -20,6 +20,9 @@
                 <option name="uploadIniSizeErrorMessage">sylius.taxon_image.file.upload_ini_size</option>
                 <option name="groups">sylius</option>
             </constraint>
+            <constraint name="Sylius\Bundle\CoreBundle\Validator\Constraints\AllowedImageMimeTypes">
+                <option name="groups">sylius</option>
+            </constraint>
         </property>
     </class>
 </constraint-mapping>

src/Sylius/Bundle/CoreBundle/Resources/translations/validators.en.yml

@@ -115,6 +115,9 @@ sylius:
             not_exist: Product with code {{ code }} does not exist.
     product_attribute:
         invalid: Position must be an integer.
+    image:
+        file:
+            allowed_mime_types: The file is not a valid image. Allowed types are %types%.
     product_image:
         file:
             max_size: The image is too big - {{ size }}{{ suffix }}. Maximum allowed size is {{ limit }}{{ suffix }}.

src/Sylius/Bundle/CoreBundle/Tests/DependencyInjection/ConfigurationTest.php

@@ -262,6 +262,26 @@ final class ConfigurationTest extends TestCase
         );
     }
 
+    /** @test */
+    public function it_sets_default_allowed_images_mime_types(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [[]],
+            ['allowed_images_mime_types' => ['image/jpeg', 'image/png', 'image/gif', 'image/webp']],
+            'allowed_images_mime_types',
+        );
+    }
+
+    /** @test */
+    public function it_allows_setting_custom_allowed_images_mime_types(): void
+    {
+        $this->assertProcessedConfigurationEquals(
+            [['allowed_images_mime_types' => ['image/svg+xml', 'image/bmp']]],
+            ['allowed_images_mime_types' => ['image/svg+xml', 'image/bmp']],
+            'allowed_images_mime_types',
+        );
+    }
+
     protected function getConfiguration(): ConfigurationInterface
     {
         return new Configuration();

src/Sylius/Bundle/CoreBundle/Twig/FilterExtension.php

@@ -17,6 +17,16 @@ use Liip\ImagineBundle\Imagine\Cache\CacheManager;
 use Liip\ImagineBundle\Templating\FilterExtension as BaseFilterExtension;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
+trigger_deprecation(
+    'sylius/sylius',
+    '1.14',
+    'The "%s" class is deprecated and will be removed in Sylius 2.0.',
+    FilterExtension::class,
+);
+
+/**
+ * @deprecated
+ */
 final class FilterExtension extends BaseFilterExtension
 {
     public function __construct(private string $imagesPath, CacheManager $cache)

src/Sylius/Bundle/CoreBundle/Validator/Constraints/AllowedImageMimeTypes.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Sylius Sp. z o.o.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\CoreBundle\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraint;
+
+final class AllowedImageMimeTypes extends Constraint
+{
+    public string $message = 'sylius.image.file.allowed_mime_types';
+
+    public function validatedBy(): string
+    {
+        return 'sylius_image_allowed_mime_types_validator';
+    }
+
+    public function getTargets(): string
+    {
+        return self::PROPERTY_CONSTRAINT;
+    }
+}

src/Sylius/Bundle/CoreBundle/Validator/Constraints/AllowedImageMimeTypesValidator.php

@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\CoreBundle\Validator\Constraints;
+
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Webmozart\Assert\Assert;
+
+class AllowedImageMimeTypesValidator extends ConstraintValidator
+{
+    /**
+     * @param array<string> $allowedMimeTypes
+     */
+    public function __construct(private readonly array $allowedMimeTypes)
+    {
+    }
+
+    public function validate(mixed $value, Constraint $constraint): void
+    {
+        if (null === $value) {
+            return;
+        }
+
+        Assert::isInstanceOf($value, UploadedFile::class);
+        Assert::isInstanceOf($constraint, AllowedImageMimeTypes::class);
+
+        if (!in_array($value->getMimeType(), $this->allowedMimeTypes, true)) {
+            $this->context->buildViolation($constraint->message, ['%types%' => implode(', ', $this->allowedMimeTypes)])->addViolation();
+        }
+    }
+}

src/Sylius/Component/Core/Uploader/ImageUploader.php

@@ -13,7 +13,6 @@ declare(strict_types=1);
 
 namespace Sylius\Component\Core\Uploader;
 
-use enshrined\svgSanitize\Sanitizer;
 use Gaufrette\FilesystemInterface;
 use Sylius\Component\Core\Filesystem\Adapter\FilesystemAdapterInterface;
 use Sylius\Component\Core\Filesystem\Adapter\GaufretteFilesystemAdapter;
@@ -30,8 +29,8 @@ class ImageUploader implements ImageUploaderInterface
 
     private const MIME_SVG = 'image/svg';
 
-    /** @var Sanitizer */
-    protected $sanitizer;
+    /** @var \enshrined\svgSanitize\Sanitizer|null */
+    protected ?object $sanitizer = null;
 
     public function __construct(
         /** @var FilesystemAdapterInterface $filesystem */
@@ -59,7 +58,10 @@ class ImageUploader implements ImageUploaderInterface
         }
 
         $this->imagePathGenerator = $imagePathGenerator ?? new UploadedImagePathGenerator();
-        $this->sanitizer = new Sanitizer();
+
+        if (class_exists(\enshrined\svgSanitize\Sanitizer::class)) {
+            $this->sanitizer = new \enshrined\svgSanitize\Sanitizer();
+        }
     }
 
     public function upload(ImageInterface $image): void
@@ -101,7 +103,7 @@ class ImageUploader implements ImageUploaderInterface
 
     protected function sanitizeContent(string $fileContent, string $mimeType): string
     {
-        if (self::MIME_SVG_XML === $mimeType || self::MIME_SVG === $mimeType) {
+        if ((self::MIME_SVG_XML === $mimeType || self::MIME_SVG === $mimeType) && $this->sanitizer !== null) {
             $fileContent = $this->sanitizer->sanitize($fileContent);
         }
 

src/Sylius/Component/Core/composer.json

@@ -27,7 +27,6 @@
     ],
     "require": {
         "php": "^8.1",
-        "enshrined/svg-sanitize": "^0.16",
         "guzzlehttp/guzzle": "^6.5 || ^7.0",
         "knplabs/gaufrette": "^0.10 || ^0.11",
         "league/flysystem": "^2.4",

tests/Functional/ImageUploaderTest.php

@@ -28,6 +28,10 @@ final class ImageUploaderTest extends WebTestCase
     /** @test */
     public function it_sanitizes_file_content_if_it_is_svg_mime_type(): void
     {
+        if (!class_exists(\enshrined\svgSanitize\Sanitizer::class)) {
+            $this->markTestSkipped('This test requires "enshrined/svg-sanitize" package to be installed.');
+        }
+
         self::$client = static::createClient();
 
         $imageUploader = self::$kernel->getContainer()->get('sylius.image_uploader');