Merge IBX-1392: Implemented image filenames sanitization (#52)

src/bundle/Core/Command/NormalizeImagesPathsCommand.php

@@ -11,6 +11,9 @@
 use Doctrine\DBAL\Driver\Connection;
 use Ibexa\Core\FieldType\Image\ImageStorage\Gateway as ImageStorageGateway;
 use Ibexa\Core\IO\FilePathNormalizerInterface;
+use Ibexa\Core\IO\IOServiceInterface;
+use Ibexa\Core\IO\Values\BinaryFile;
+use Ibexa\Core\IO\Values\BinaryFileCreateStruct;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -40,16 +43,21 @@ final class NormalizeImagesPathsCommand extends Command
     /** @var \Doctrine\DBAL\Driver\Connection */
     private $connection;
 
+    /** @var \Ibexa\Core\IO\IOServiceInterface */
+    private $ioService;
+
     public function __construct(
         ImageStorageGateway $imageGateway,
         FilePathNormalizerInterface $filePathNormalizer,
-        Connection $connection
+        Connection $connection,
+        IOServiceInterface $ioService
     ) {
         parent::__construct();
 
         $this->imageGateway = $imageGateway;
         $this->filePathNormalizer = $filePathNormalizer;
         $this->connection = $connection;
+        $this->ioService = $ioService;
     }
 
     protected function configure()
@@ -163,6 +171,30 @@ private function updateImagePath(int $fieldId, string $oldPath, string $newPath)
                 $this->imageGateway->updateImagePath($fieldId, $oldPath, $newPath);
             }
         }
+
+        $this->moveFile($oldFileName, $newFilename, $oldPath);
+    }
+
+    private function moveFile(string $oldFileName, string $newFileName, string $oldPath): void
+    {
+        $oldBinaryFile = $this->ioService->loadBinaryFileByUri(\DIRECTORY_SEPARATOR . $oldPath);
+        $newId = str_replace($oldFileName, $newFileName, $oldBinaryFile->id);
+        $inputStream = $this->ioService->getFileInputStream($oldBinaryFile);
+
+        $binaryCreateStruct = new BinaryFileCreateStruct(
+            [
+                'id' => $newId,
+                'size' => $oldBinaryFile->size,
+                'inputStream' => $inputStream,
+                'mimeType' => $this->ioService->getMimeType($oldBinaryFile->id),
+            ]
+        );
+
+        $newBinaryFile = $this->ioService->createBinaryFile($binaryCreateStruct);
+
+        if ($newBinaryFile instanceof BinaryFile) {
+            $this->ioService->deleteBinaryFile($oldBinaryFile);
+        }
     }
 }
 

src/bundle/Core/Resources/config/commands.yml

@@ -51,6 +51,7 @@ services:
         arguments:
             $connection: '@ezpublish.persistence.connection'
             $imageGateway: '@ezpublish.fieldType.ezimage.storage_gateway'
+            $ioService: '@ezpublish.fieldType.ezimage.io_service'
         tags:
             - { name: console.command }
 

src/lib/IO/FilePathNormalizer/Flysystem.php

@@ -9,12 +9,32 @@
 namespace Ibexa\Core\IO\FilePathNormalizer;
 
 use Ibexa\Core\IO\FilePathNormalizerInterface;
+use Ibexa\Core\Persistence\Legacy\Content\UrlAlias\SlugConverter;
 use League\Flysystem\Util;
 
 final class Flysystem implements FilePathNormalizerInterface
 {
+    private const HASH_PATTERN = '/^[0-9a-f]{12}-/';
+
+    /** @var \Ibexa\Core\Persistence\Legacy\Content\UrlAlias\SlugConverter */
+    private $slugConverter;
+
+    public function __construct(SlugConverter $slugConverter)
+    {
+        $this->slugConverter = $slugConverter;
+    }
+
     public function normalizePath(string $filePath): string
     {
+        $fileName = pathinfo($filePath, PATHINFO_BASENAME);
+        $directory = pathinfo($filePath, PATHINFO_DIRNAME);
+
+        $fileName = $this->slugConverter->convert($fileName);
+
+        $hash = preg_match(self::HASH_PATTERN, $fileName) ? '' : bin2hex(random_bytes(6)) . '-';
+
+        $filePath = $directory . \DIRECTORY_SEPARATOR . $hash . $fileName;
+
         return Util::normalizePath($filePath);
     }
 }

src/lib/Resources/settings/io.yml

@@ -75,5 +75,8 @@ services:
             - ~
             - "@ezpublish.core.io.image_fieldtype.legacy_url_decorator"
 
-    Ibexa\Core\IO\FilePathNormalizer\Flysystem: ~
+    Ibexa\Core\IO\FilePathNormalizer\Flysystem:
+        arguments:
+            $slugConverter: '@ezpublish.persistence.slug_converter'
+
     Ibexa\Core\IO\FilePathNormalizerInterface: '@Ibexa\Core\IO\FilePathNormalizer\Flysystem'

tests/integration/Core/Repository/FieldType/ImageIntegrationTest.php

@@ -182,12 +182,10 @@ public function getFieldName()
      *
      * Asserts that the data provided by {@link getValidCreationFieldData()}
      * was stored and loaded correctly.
-     *
-     * @param \Ibexa\Contracts\Core\Repository\Values\Content\Field $field
      */
-    public function assertFieldDataLoadedCorrect(Field $field)
+    public function assertFieldDataLoadedCorrect(Field $field): void
     {
-        $this->assertInstanceOf(
+        self::assertInstanceOf(
             ImageValue::class,
             $field->value
         );
@@ -198,12 +196,15 @@ public function assertFieldDataLoadedCorrect(Field $field)
         // Will be nullified by external storage
         $expectedData['inputUri'] = null;
 
+        // Will be changed by external storage as fileName will be decorated with a hash
+        $expectedData['fileName'] = $field->value->fileName;
+
         $this->assertPropertiesCorrect(
             $expectedData,
             $field->value
         );
 
-        $this->assertTrue(
+        self::assertTrue(
             $this->uriExistsOnIO($field->value->uri),
             "Asserting that {$field->value->uri} exists."
         );
@@ -247,7 +248,7 @@ public function getValidUpdateFieldData()
      */
     public function assertUpdatedFieldDataLoadedCorrect(Field $field)
     {
-        $this->assertInstanceOf(
+        self::assertInstanceOf(
             ImageValue::class,
             $field->value
         );
@@ -258,14 +259,17 @@ public function assertUpdatedFieldDataLoadedCorrect(Field $field)
         // Will change during storage
         $expectedData['inputUri'] = null;
 
+        // Will change during storage as fileName will be decorated with a hash
+        $expectedData['fileName'] = $field->value->fileName;
+
         $expectedData['uri'] = $field->value->uri;
 
         $this->assertPropertiesCorrect(
             $expectedData,
             $field->value
         );
 
-        $this->assertTrue(
+        self::assertTrue(
             $this->uriExistsOnIO($field->value->uri),
             "Asserting that file {$field->value->uri} exists"
         );
@@ -573,8 +577,8 @@ protected function getValidSearchValueOne()
     {
         return new ImageValue(
             [
-                'fileName' => 'cafe-terrace-at-night.jpg',
-                'inputUri' => ($path = __DIR__ . '/_fixtures/image.jpg'),
+                'fileName' => '1234eeee1234-cafe-terrace-at-night.jpg',
+                'inputUri' => ($path = __DIR__ . '/_fixtures/1234eeee1234-image.jpg'),
                 'alternativeText' => 'café terrace at night, also known as the cafe terrace on the place du forum',
                 'fileSize' => filesize($path),
             ]
@@ -585,8 +589,8 @@ protected function getValidSearchValueTwo()
     {
         return new ImageValue(
             [
-                'fileName' => 'thatched-cottages-at-cordeville.png',
-                'inputUri' => ($path = __DIR__ . '/_fixtures/image.png'),
+                'fileName' => '2222eeee1111-thatched-cottages-at-cordeville.png',
+                'inputUri' => ($path = __DIR__ . '/_fixtures/2222eeee1111-image.png'),
                 'alternativeText' => 'chaumes de cordeville à auvers-sur-oise',
                 'fileSize' => filesize($path),
             ]

tests/lib/IO/FilePathNormalizer/FlysystemTest.php

@@ -0,0 +1,87 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Tests\Core\IO\FilePathNormalizer;
+
+use Ibexa\Core\IO\FilePathNormalizer\Flysystem;
+use Ibexa\Core\Persistence\Legacy\Content\UrlAlias\SlugConverter;
+use PHPUnit\Framework\TestCase;
+
+final class FlysystemTest extends TestCase
+{
+    /** @var \Ibexa\Core\IO\FilePathNormalizer\Flysystem */
+    private $filePathNormalizer;
+
+    /** @var \Ibexa\Core\Persistence\Legacy\Content\UrlAlias\SlugConverter|\PHPUnit\Framework\MockObject\MockObject */
+    private $slugConverter;
+
+    public function setUp(): void
+    {
+        $this->slugConverter = $this->createMock(SlugConverter::class);
+        $this->filePathNormalizer = new Flysystem($this->slugConverter);
+    }
+
+    /**
+     * @dataProvider providerForTestNormalizePath
+     */
+    public function testNormalizePath(
+        string $originalPath,
+        string $fileName,
+        string $sluggedFileName,
+        string $regex
+    ): void {
+        $this->slugConverter
+            ->expects(self::once())
+            ->method('convert')
+            ->with($fileName)
+            ->willReturn($sluggedFileName);
+
+        $normalizedPath = $this->filePathNormalizer->normalizePath($originalPath);
+
+        self::assertStringEndsWith($sluggedFileName, $normalizedPath);
+        self::assertRegExp($regex, $normalizedPath);
+    }
+
+    public function providerForTestNormalizePath(): array
+    {
+        $defaultPattern = '/\/[0-9a-f]{12}-';
+
+        return [
+            'No special chars' => [
+                '4/3/2/234/1/image.jpg',
+                'image.jpg',
+                'image.jpg',
+                $defaultPattern . 'image.jpg/',
+            ],
+            'Spaces in the filename' => [
+                '4/3/2/234/1/image with spaces.jpg',
+                'image with spaces.jpg',
+                'image-with-spaces.jpg',
+                $defaultPattern . 'image-with-spaces.jpg/',
+            ],
+            'Encoded spaces in the name' => [
+                '4/3/2/234/1/image%20+no+spaces.jpg',
+                'image%20+no+spaces.jpg',
+                'image-20-nospaces.jpg',
+                $defaultPattern . 'image-20-nospaces.jpg/',
+            ],
+            'Special chars in the name' => [
+                '4/3/2/234/1/image%20+no+spaces?.jpg',
+                'image%20+no+spaces?.jpg',
+                'image-20-nospaces.jpg',
+                $defaultPattern . 'image-20-nospaces.jpg/',
+            ],
+            'Already hashed name' => [
+                '4/3/2/234/1/14ff44718877-hashed.jpg',
+                '14ff44718877-hashed.jpg',
+                '14ff44718877-hashed.jpg',
+                '/^4\/3\/2\/234\/1\/14ff44718877-hashed.jpg$/',
+            ],
+        ];
+    }
+}